diff --git a/CHANGELOG.md b/CHANGELOG.md index 49e057850..c7fd247d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,12 @@ [SimplyDanny](https://github.com/SimplyDanny) [#5623](https://github.com/realm/SwiftLint/issues/5623) +* Fix `baseline compare` incorrectly reporting some violations + as new, and also now sorts the violations from `baseline compare` + deterministically. + [Martin Redington](https://github.com/mildm8nnered) + [#5606](https://github.com/realm/SwiftLint/issues/5606) + ## 0.55.1: Universal Washing Powder #### Breaking diff --git a/Source/SwiftLintCore/Models/Baseline.swift b/Source/SwiftLintCore/Models/Baseline.swift index 630537caa..0df4326df 100644 --- a/Source/SwiftLintCore/Models/Baseline.swift +++ b/Source/SwiftLintCore/Models/Baseline.swift @@ -4,7 +4,7 @@ private typealias BaselineViolations = [BaselineViolation] private typealias ViolationsPerFile = [String: BaselineViolations] private typealias ViolationsPerRule = [String: BaselineViolations] -private struct BaselineViolation: Codable, Hashable { +private struct BaselineViolation: Codable, Hashable, Comparable { let violation: StyleViolation let text: String var key: String { text + violation.reason } @@ -20,13 +20,23 @@ private struct BaselineViolation: Codable, Hashable { ) self.text = text } + + static func == (lhs: Self, rhs: Self) -> Bool { + lhs.violation == rhs.violation && lhs.text == rhs.text + } + + static func < (lhs: Self, rhs: Self) -> Bool { + lhs.violation.location == rhs.violation.location + ? lhs.violation.ruleIdentifier < rhs.violation.ruleIdentifier + : lhs.violation.location < rhs.violation.location + } } /// A set of violations that can be used to filter newly detected violations. public struct Baseline: Equatable { private let baseline: ViolationsPerFile private var sortedBaselineViolations: BaselineViolations { - baseline.sorted(by: { $0.key < $1.key }).flatMap(\.value) + baseline.flatMap(\.value).sorted() } /// The stored violations. @@ -71,6 +81,16 @@ public struct Baseline: Equatable { } let relativePathViolations = BaselineViolations(violations) + let violationsWithAbsolutePaths = filter( + relativePathViolations: relativePathViolations, + baselineViolations: baselineViolations + ) + return violations.filter { violationsWithAbsolutePaths.contains($0) } + } + + private func filter( + relativePathViolations: BaselineViolations, baselineViolations: BaselineViolations + ) -> Set { if relativePathViolations == baselineViolations { return [] } @@ -106,8 +126,7 @@ public struct Baseline: Equatable { } } - let violationsWithAbsolutePaths = Set(filteredViolations.violationsWithAbsolutePaths) - return violations.filter { violationsWithAbsolutePaths.contains($0) } + return Set(filteredViolations.violationsWithAbsolutePaths) } /// Returns the violations that are present in another `Baseline`, but not in this one. @@ -116,8 +135,13 @@ public struct Baseline: Equatable { /// /// - parameter otherBaseline: The other `Baseline`. public func compare(_ otherBaseline: Baseline) -> [StyleViolation] { - otherBaseline.baseline.flatMap { - filter($1.violationsWithAbsolutePaths) + otherBaseline.baseline.flatMap { relativePath, otherBaselineViolations -> Set in + if let baselineViolations = baseline[relativePath] { + return filter(relativePathViolations: otherBaselineViolations, baselineViolations: baselineViolations) + } + return Set(otherBaselineViolations.violationsWithAbsolutePaths) + }.sorted { + $0.location == $1.location ? $0.ruleIdentifier < $1.ruleIdentifier : $0.location < $1.location } } } diff --git a/Tests/SwiftLintFrameworkTests/BaselineTests.swift b/Tests/SwiftLintFrameworkTests/BaselineTests.swift index 8d3805030..5f45b66c0 100644 --- a/Tests/SwiftLintFrameworkTests/BaselineTests.swift +++ b/Tests/SwiftLintFrameworkTests/BaselineTests.swift @@ -139,13 +139,16 @@ final class BaselineTests: XCTestCase { func testCompare() throws { try withExampleFileCreated { sourceFilePath in - let violations = Self.violations(for: sourceFilePath) - let oldViolations = Array(violations.dropFirst()) - let newViolations = Array(violations.dropLast()) - let oldBaseline = Baseline(violations: oldViolations) - let newBaseline = Baseline(violations: newViolations) - XCTAssertEqual(oldBaseline.compare(newBaseline), [violations.first]) - XCTAssertEqual(newBaseline.compare(oldBaseline), [violations.last]) + let ruleDescriptions = Self.ruleDescriptions + Self.ruleDescriptions + let violations = ruleDescriptions.violations(for: sourceFilePath) + let numberofViolationsToDrop = 3 + let oldBaseline = Baseline(violations: Array(violations.dropFirst(numberofViolationsToDrop)).reversed()) + let newViolations = Array( + try violations.lineShifted(by: 2, path: sourceFilePath).dropLast(numberofViolationsToDrop) + ) + let newBaseline = Baseline(violations: newViolations.reversed()) + XCTAssertEqual(oldBaseline.compare(newBaseline), Array(newViolations.prefix(numberofViolationsToDrop))) + XCTAssertEqual(newBaseline.compare(oldBaseline), Array(violations.suffix(numberofViolationsToDrop))) } }