Baseline implementation

[mildm8nnered]

Inspired by #3421 and its predecessors.

Adds a new baseline capability to SwiftLint, in the form of two new options to the lint and analyze commands.

This may be helpful for people who have large numbers of pre-existing violations in their codebase, that they cannot easily fix, and do not want to suppress individually.

This should allow rules to be enabled, and existing violations be ignored, but any new violations detected.

Usage:

  --baseline <baseline>   The path to a baseline file, which will be used to filter out detected violations.

  --write-baseline <write-baseline>
                          The path to save detected violations to as a new baseline.

For example:

swiftlint --baseline /path/to/baselinefile --write-baseline /path/to/baselinefile

Will read the existing baseline from baselinefile, and also write the full set of detected violations back to the same file.

If the baseline cannot be read, that is a fatal error, unless you've also specified the same file with --write-baseline (so that you can use that invocation, and still have it work first time around).

In practice, in CI, I expect that you would probably keep a current baseline from your main branch, and then compare feature branches against that baseline.

Baseline Comparisons

A baseline is a dictionary, keyed on relative file path, whose values are the array of StyleViolation's in each file, and additionally the text of the line of source code that triggered the violation. Storing the text means that violations can still be matched, even if they have been moved around in the source file, changing their line numbers.

On disk, the baseline is actually stored as a sorted array, so that identical baselines will have identical on-disk representations.

When filtering detected violations according to the baseline, we first compare for exact equality between the detected and baseline violations, including line numbers.

If the sets of violations are not exactly equal, then we remove any violations that are exactly identical (again including line numbers).

We then group the violations by rule, and then again by the original source code text. For each detected violation, if the baseline does not contain a matching violation of the same rule, with the same reason, and text, then the detected violation is reported. If the detected violations contain multiple violations of the same rule, with the same reason and text, then all those violations are reported, even if the baseline contains a match, as we cannot tell which of the violations map to which of the baseline violations.

It may be the case that the user fixed one violation of a rule, but introduced a completely new violation of that rule, with the same text, elsewhere, or fixed two existing violations, but added a new one. In the current implementation, as long as the number of violations of that rule (additionally matching on the reason and text) did not increase, we will ignore them.

Threshold Violations

Currently threshold violations are not filtered or stored in the baseline, and are calculated against the filtered violations.

Inspecting a Baseline

There is also a new command - baseline

$ swiftlint  help baseline 
OVERVIEW: Reports the violations in a baseline or the violations that are present in another baseline, but not in the original.

USAGE: swiftlint baseline <subcommand>

OPTIONS:
  --version               Show the version.
  -h, --help              Show help information.

SUBCOMMANDS:
  report (default)        Reports the violations in a baseline.
  compare                 Reports the violations that are present in another baseline but not in the original baseline.

  See 'swiftlint help baseline <subcommand>' for detailed help.
$ 
$ 
$ swiftlint  help baseline report
OVERVIEW: Reports the violations in a baseline.

USAGE: swiftlint baseline report <baseline> [--reporter <reporter>] [--output <output>]

ARGUMENTS:
  <baseline>              The path to the baseline file.

OPTIONS:
  --reporter <reporter>   The reporter used to report violations. The 'summary' reporter can be useful to provide an overview.
  --output <output>       The file where violations should be saved. Prints to stdout by default.
  --version               Show the version.
  -h, --help              Show help information.

$
$ swiftlint  help baseline compare
OVERVIEW: Reports the violations that are present in another baseline but not in the original baseline.

USAGE: swiftlint baseline compare <baseline> --other-baseline <other-baseline> [--reporter <reporter>] [--output <output>]

ARGUMENTS:
  <baseline>              The path to the baseline file.

OPTIONS:
  --other-baseline <other-baseline>
                          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.
  --reporter <reporter>   The reporter used to report violations. The 'summary' reporter can be useful to provide an overview.
  --output <output>       The file where violations should be saved. Prints to stdout by default.
  --version               Show the version.
  -h, --help              Show help information.

This uses the existing reporters to report the violations contained in the baseline (--reporter summary can be useful with the following commands, to get an overview without having to wade through individual violations).

For example, to inspect a baseline:

swiftlint baseline report Baseline.json

To see what new violations have been introduced, you can run:

swiftlint baseline compare Baseline.json --other-baseline NewBaseline.json

This will produce the same output as:

swiftlint lint --baseline Baseline.json

To see what violations have been fixed, simply reverse the baselines

swiftlint baseline compare NewBaseline.json --other-baseline Baseline.json

[SwiftLintBot]

	1 Warning
⚠️	Big PR

	17 Messages
📖	Linting Aerial with this PR took 1.24s vs 1.24s on main (0% slower)
📖	Linting Alamofire with this PR took 1.8s vs 1.81s on main (0% faster)
📖	Linting Brave with this PR took 10.42s vs 10.43s on main (0% faster)
📖	Linting DuckDuckGo with this PR took 5.33s vs 5.35s on main (0% faster)
📖	Linting Firefox with this PR took 13.42s vs 13.41s on main (0% slower)
📖	Linting Kickstarter with this PR took 12.66s vs 12.67s on main (0% faster)
📖	Linting Moya with this PR took 0.7s vs 0.7s on main (0% slower)
📖	Linting NetNewsWire with this PR took 3.84s vs 3.83s on main (0% slower)
📖	Linting Nimble with this PR took 1.03s vs 1.03s on main (0% slower)
📖	Linting PocketCasts with this PR took 10.33s vs 10.33s on main (0% slower)
📖	Linting Quick with this PR took 0.46s vs 0.46s on main (0% slower)
📖	Linting Realm with this PR took 6.34s vs 6.34s on main (0% slower)
📖	Linting Sourcery with this PR took 3.15s vs 3.17s on main (0% faster)
📖	Linting Swift with this PR took 6.34s vs 6.32s on main (0% slower)
📖	Linting VLC with this PR took 1.69s vs 1.68s on main (0% slower)
📖	Linting Wire with this PR took 23.68s vs 23.54s on main (0% slower)
📖	Linting WordPress with this PR took 15.69s vs 15.69s on main (0% slower)

Generated by 🚫 Danger

[mildm8nnered]

@SimplyDanny and @jpsim - would love to get some feedback on this ...

[SimplyDanny]

Very good idea to store and compare the violating line for more context. To increase precision even further, could this theoretically be extended to the violating line itself plus the previous and the subsequent line? Not sure what the benefit of this would be though ...

[mildm8nnered]

Very good idea to store and compare the violating line for more context. To increase precision even further, could this theoretically be extended to the violating line itself plus the previous and the subsequent line? Not sure what the benefit of this would be though ...

So we could definitely store more context, and that would definitely improve accuracy in some situations.

But having more context would mean that a change in the preceding or following line would then show up as "new", at least on the basis of exact equality. The same is true for the line the violation is on of course - someone could change whitespace there, but still leave the violation, and that would fail our equality test. We could try to filter intelligently for preceding and following line changes, but I think the line the violation is on is going to work well for most cases (one can always construct a pathological set of violations that will fool whichever algorithm you use).

If this does turn out to be a problem in practice, we can always update it later.

[mildm8nnered]

I swapped saving the dictionaries of baseline violations, keyed on file, to saving a sorted array of baseline violations - 2834540

So exactly identical baselines should also now be identical on disk.

[mildm8nnered]

I've added a compare action to the baseline command, that will print violations that occur in another baseline, but not in the original.

[mildm8nnered]

I think I've addressed everything so far, but let me know if I've missed anything ...

[SimplyDanny]

I think I've addressed everything so far, but let me know if I've missed anything ...

I kept the open points as "unresolved" and added a few more comments.

[aiKrice]

Very eager to see it :)

[SimplyDanny]

This is a very good implementation to start with and collect some feedback for. Having it as an incubation allows us to further enhance it if that's required.

Thanks a lot, @mildm8nnered, for picking up and refining previous PRs!

[mildm8nnered]

This is a very good implementation to start with and collect some feedback for. Having it as an incubation allows us to further enhance it if that's required.

Thanks a lot, @mildm8nnered, for picking up and refining previous PRs!

Thank you for all your help and feedback @SimplyDanny

We may be able to close these old baseline PRs now:

#3421 and #4908 ...

[aiKrice]

Hello @mildm8nnered , First of all great job.
I would like to be sure how should I use the baseline. Naively, I try in the same "spirit" than Detekt (for kotlin).

So here is my commands:

1. Generate a baseline with swiftlint --write-baseline config/swiftlint/baseline.json
2. Apply it: swiftlint --silence-deprecation-warnings --strict --baseline config/swiftlint/baseline.json --config config/swiftlint/swiftlint.yml myswiftfile_with_violations_baselined.swift

But I still have the violations output. Could you tell me what I am wrong ?

[mildm8nnered]

Hello @mildm8nnered , First of all great job. I would like to be sure how should I use the baseline. Naively, I try in the same "spirit" than Detekt (for kotlin).

So here is my commands:

1. Generate a baseline with swiftlint --write-baseline config/swiftlint/baseline.json
2. Apply it: swiftlint --silence-deprecation-warnings --strict --baseline config/swiftlint/baseline.json --config config/swiftlint/swiftlint.yml myswiftfile_with_violations_baselined.swift

But I still have the violations output. Could you tell me what I am wrong ?

So you need to use the same config file to generate the baseline that you will use when using it to filter violations.

I would have expected your commands to be:

swiftlint --write-baseline config/swiftlint/baseline.json --config config/swiftlint/swiftlint.yml myswiftfile_with_violations_baselined.swift

and

swiftlint --silence-deprecation-warnings --strict --baseline config/swiftlint/baseline.json --config config/swiftlint/swiftlint.yml myswiftfile_with_violations_baselined.swift

--strict should not make a difference - it will mean that the baseline violations and detected violations will not be exact matches (because severity will differ), but when we filter non-exact matches, we ignore severity (and line number).

Please let us know if this does not fix it for you.