Skip to content

Commit

Permalink
Add an experimental baseline feature (#5475)
Browse files Browse the repository at this point in the history
  • Loading branch information
mildm8nnered committed May 1, 2024
1 parent 99a990d commit 96db41c
Show file tree
Hide file tree
Showing 11 changed files with 551 additions and 9 deletions.
8 changes: 7 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,13 @@

#### Experimental

* None.
* Add two new options to the `lint` and `analyze` commands: `--write-baseline`
to save a baseline to disk, and `--baseline` to read a saved baseline and
use it to filter out detected pre-existing violations. A new `baseline`
command uses the reporters to print the violations in a baseline.
[Martin Redington](https://github.com/mildm8nnered)
[#5475](https://github.com/realm/SwiftLint/pull/5475)
[#3421](https://github.com/realm/SwiftLint/pull/3421)

#### Enhancements

Expand Down
182 changes: 182 additions & 0 deletions Source/SwiftLintCore/Models/Baseline.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
import Foundation

private typealias BaselineViolations = [BaselineViolation]
private typealias ViolationsPerFile = [String: BaselineViolations]
private typealias ViolationsPerRule = [String: BaselineViolations]

private struct BaselineViolation: Codable, Hashable {
let violation: StyleViolation
let text: String
var key: String { text + violation.reason + violation.severity.rawValue }

init(violation: StyleViolation, text: String) {
let location = violation.location
self.violation = violation.with(location: Location(
// Within the baseline, we use relative paths, so that
// comparisons are independent of the absolute path
file: location.relativeFile,
line: location.line,
character: location.character)
)
self.text = text
}
}

/// 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)
}

/// The stored violations.
public var violations: [StyleViolation] {
sortedBaselineViolations.violationsWithAbsolutePaths
}

/// Creates a `Baseline` from a saved file.
///
/// - parameter fromPath: The path to read from.
public init(fromPath path: String) throws {
let data = try Data(contentsOf: URL(fileURLWithPath: path))
baseline = try JSONDecoder().decode(BaselineViolations.self, from: data).groupedByFile()
}

/// Creates a `Baseline` from a list of violations.
///
/// - parameter violations: The violations for the baseline.
public init(violations: [StyleViolation]) {
baseline = BaselineViolations(violations).groupedByFile()
}

/// Writes a `Baseline` to disk in JSON format.
///
/// - parameter toPath: The path to write to.
public func write(toPath path: String) throws {
let data = try JSONEncoder().encode(sortedBaselineViolations)
try data.write(to: URL(fileURLWithPath: path))
}

/// Filters out violations that are present in the `Baseline`.
///
/// Assumes that all violations are from the same file.
///
/// - parameter violations: The violations to filter.
/// - Returns: The new violations.
public func filter(_ violations: [StyleViolation]) -> [StyleViolation] {
guard let firstViolation = violations.first,
let baselineViolations = baseline[firstViolation.location.relativeFile ?? ""],
baselineViolations.isNotEmpty else {
return violations
}

let relativePathViolations = BaselineViolations(violations)
if relativePathViolations == baselineViolations {
return []
}

let violationsByRuleIdentifier = relativePathViolations.groupedByRuleIdentifier(
filteredBy: baselineViolations
)
let baselineViolationsByRuleIdentifier = baselineViolations.groupedByRuleIdentifier(
filteredBy: relativePathViolations
)

var filteredViolations: Set<BaselineViolation> = []

for (ruleIdentifier, ruleViolations) in violationsByRuleIdentifier {
guard
let baselineViolations = baselineViolationsByRuleIdentifier[ruleIdentifier],
baselineViolations.isNotEmpty else {
filteredViolations.formUnion(ruleViolations)
continue
}

let groupedRuleViolations = Dictionary(grouping: ruleViolations, by: \.key)
let groupedBaselineViolations = Dictionary(grouping: baselineViolations, by: \.key)

for (key, ruleViolations) in groupedRuleViolations {
guard let baselineViolations = groupedBaselineViolations[key] else {
filteredViolations.formUnion(ruleViolations)
continue
}
if ruleViolations.count > baselineViolations.count {
filteredViolations.formUnion(ruleViolations)
}
}
}

let violationsWithAbsolutePaths = Set(filteredViolations.violationsWithAbsolutePaths)
return violations.filter { violationsWithAbsolutePaths.contains($0) }
}

/// Returns the violations that are present in another `Baseline`, but not in this one.
///
/// The violations are filtered using the same algorithm as the `filter` method above.
///
/// - parameter otherBaseline: The other `Baseline`.
public func compare(_ otherBaseline: Baseline) -> [StyleViolation] {
otherBaseline.baseline.flatMap {
filter($1.violationsWithAbsolutePaths)
}
}
}

private struct LineCache {
private var lines: [String: [String]] = [:]

mutating func text(at location: Location) -> String {
let line = (location.line ?? 0) - 1
if line > 0, let file = location.file, let content = cached(file: file), line < content.count {
return content[line]
}
return ""
}

private mutating func cached(file: String) -> [String]? {
if let fileLines = lines[file] {
return fileLines
}
if let fileLines = SwiftLintFile(path: file)?.lines.map(\.content) {
lines[file] = fileLines
return fileLines
}
return nil
}
}

private extension Sequence where Element == BaselineViolation {
init(_ violations: [StyleViolation]) where Self == BaselineViolations {
var lineCache = LineCache()
self = violations.map { $0.baselineViolation(text: lineCache.text(at: $0.location)) }
}

var violationsWithAbsolutePaths: [StyleViolation] {
map { $0.violation.withAbsolutePath }
}

func groupedByFile() -> ViolationsPerFile {
Dictionary(grouping: self) { $0.violation.location.relativeFile ?? "" }
}

func groupedByRuleIdentifier(filteredBy existingViolations: [BaselineViolation] = []) -> ViolationsPerRule {
Dictionary(grouping: Set(self).subtracting(existingViolations), by: \.violation.ruleIdentifier)
}
}

private extension StyleViolation {
var withAbsolutePath: StyleViolation {
let absolutePath: String? =
if let relativePath = location.file {
FileManager.default.currentDirectoryPath + "/" + relativePath
} else {
nil
}
let newLocation = Location(file: absolutePath, line: location.line, character: location.character)
return with(location: newLocation)
}

func baselineViolation(text: String = "") -> BaselineViolation {
BaselineViolation(violation: self, text: text)
}
}
9 changes: 7 additions & 2 deletions Source/SwiftLintCore/Models/Issue.swift
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ public enum Issue: LocalizedError, Equatable {
/// An error that occurred when parsing YAML.
case yamlParsing(String)

/// The baseline file at `path` is not readable or cannot be opened.
case baselineNotReadable(path: String)

/// Flag to enable warnings for deprecations being printed to the console. Printing is enabled by default.
public static var printDeprecationWarnings = true

Expand Down Expand Up @@ -172,6 +175,8 @@ public enum Issue: LocalizedError, Equatable {
"""
case let .initialFileNotFound(path):
return "Could not read file at path '\(path)'."
case let .fileNotFound(path):
return "File at path '\(path)' not found."
case let .fileNotReadable(path, id):
return "Cannot open or read file at path '\(path ?? "...")' within '\(id)' rule."
case let .fileNotWritable(path):
Expand All @@ -187,8 +192,8 @@ public enum Issue: LocalizedError, Equatable {
return "Cannot get cursor info from file at path '\(path ?? "...")' within '\(id)' rule."
case let .yamlParsing(message):
return "Cannot parse YAML file: \(message)"
case let .fileNotFound(path):
return "File at path '\(path)' not found."
case let .baselineNotReadable(path):
return "Cannot open or read the baseline file at path '\(path)'."
}
}
}
Expand Down
6 changes: 5 additions & 1 deletion Source/SwiftLintCore/Models/StyleViolation.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/// A value describing an instance of Swift source code that is considered invalid by a SwiftLint rule.
public struct StyleViolation: CustomStringConvertible, Equatable, Codable {
public struct StyleViolation: CustomStringConvertible, Codable, Hashable {
/// The identifier of the rule that generated this violation.
public let ruleIdentifier: String

Expand Down Expand Up @@ -66,4 +66,8 @@ public struct StyleViolation: CustomStringConvertible, Equatable, Codable {
new.location = location
return new
}

public func hash(into hasher: inout Hasher) {
hasher.combine(description)
}
}
2 changes: 2 additions & 0 deletions Source/swiftlint/Commands/Analyze.swift
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ extension SwiftLint {
useScriptInputFiles: common.useScriptInputFiles,
benchmark: common.benchmark,
reporter: common.reporter,
baseline: common.baseline,
writeBaseline: common.writeBaseline,
quiet: quiet,
output: common.output,
progress: common.progress,
Expand Down
88 changes: 88 additions & 0 deletions Source/swiftlint/Commands/Baseline.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import ArgumentParser
import Foundation
import SwiftLintFramework

extension SwiftLint {
struct Baseline: ParsableCommand {
static let configuration = CommandConfiguration(
abstract: "Operations on existing baselines",
subcommands: [Report.self, Compare.self],
defaultSubcommand: Report.self
)
}

private struct BaselineOptions: ParsableArguments {
@Argument(help: "The path to the baseline file.")
var baseline: String
}

private struct ReportingOptions: ParsableArguments {
@Option(
help: """
The reporter used to report violations. The 'summary' reporter can be useful to \
provide an overview.
"""
)
var reporter: String?
@Option(help: "The file where violations should be saved. Prints to stdout by default.")
var output: String?
}

private struct Report: ParsableCommand {
static let configuration = CommandConfiguration(abstract: "Reports the violations in a baseline.")

@OptionGroup
var options: BaselineOptions
@OptionGroup
var reportingOptions: ReportingOptions

func run() throws {
let savedBaseline = try SwiftLintCore.Baseline(fromPath: options.baseline)
try report(savedBaseline.violations, using: reportingOptions.reporter, to: reportingOptions.output)
ExitHelper.successfullyExit()
}
}

private struct Compare: ParsableCommand {
static let configuration = CommandConfiguration(
abstract: "Reports the violations that are present in another baseline " +
"but not in the original baseline."
)

@OptionGroup
var options: BaselineOptions
@Option(
help: """
The path to a second baseline to compare against the baseline. Violations in \
the second baseline that are not present in the original baseline will be reported.
"""
)
var otherBaseline: String
@OptionGroup
var reportingOptions: ReportingOptions

func run() throws {
let baseline = try SwiftLintCore.Baseline(fromPath: options.baseline)
let otherBaseline = try SwiftLintCore.Baseline(fromPath: otherBaseline)
try report(baseline.compare(otherBaseline), using: reportingOptions.reporter, to: reportingOptions.output)
ExitHelper.successfullyExit()
}
}
}

private func report(_ violations: [StyleViolation], using reporterIdentifier: String?, to output: String?) throws {
let reporter = reporterFrom(identifier: reporterIdentifier)
let report = reporter.generateReport(violations)
if report.isNotEmpty {
if let output {
let data = Data((report + "\n").utf8)
do {
try data.write(to: URL(fileURLWithPath: output))
} catch {
Issue.fileNotWritable(path: output).print()
}
} else {
queuedPrint(report)
}
}
}
2 changes: 2 additions & 0 deletions Source/swiftlint/Commands/Lint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ extension SwiftLint {
useScriptInputFiles: common.useScriptInputFiles,
benchmark: common.benchmark,
reporter: common.reporter,
baseline: common.baseline,
writeBaseline: common.writeBaseline,
quiet: quiet,
output: common.output,
progress: common.progress,
Expand Down
1 change: 1 addition & 0 deletions Source/swiftlint/Commands/SwiftLint.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ struct SwiftLint: AsyncParsableCommand {
Docs.self,
GenerateDocs.self,
Lint.self,
Baseline.self,
Reporters.self,
Rules.self,
Version.self
Expand Down
4 changes: 4 additions & 0 deletions Source/swiftlint/Helpers/LintOrAnalyzeArguments.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ struct LintOrAnalyzeArguments: ParsableArguments {
var benchmark = false
@Option(help: "The reporter used to log errors and warnings.")
var reporter: String?
@Option(help: "The path to a baseline file, which will be used to filter out detected violations.")
var baseline: String?
@Option(help: "The path to save detected violations to as a new baseline.")
var writeBaseline: String?
@Flag(help: "Use the in-process version of SourceKit.")
var inProcessSourcekit = false
@Option(help: "The file where violations should be saved. Prints to stdout by default.")
Expand Down

0 comments on commit 96db41c

Please sign in to comment.