Implement baseline functionality #3421

PaulTaykalo · 2020-11-09T20:25:34Z

This is the rebased version of #2580

SwiftLintBot · 2020-11-09T20:50:05Z

	2 Warnings
⚠️	Please include a CHANGELOG entry to credit yourself! You can find it at CHANGELOG.md.
⚠️	This PR may need tests.

	12 Messages
📖	Linting Aerial with this PR took 1.96s vs 2.02s on master (2% faster)
📖	Linting Alamofire with this PR took 2.78s vs 2.81s on master (1% faster)
📖	Linting Firefox with this PR took 9.36s vs 9.46s on master (1% faster)
📖	Linting Kickstarter with this PR took 15.58s vs 15.63s on master (0% faster)
📖	Linting Moya with this PR took 1.44s vs 1.48s on master (2% faster)
📖	Linting Nimble with this PR took 1.45s vs 1.38s on master (5% slower)
📖	Linting Quick with this PR took 0.63s vs 0.63s on master (0% slower)
📖	Linting Realm with this PR took 4.51s vs 4.6s on master (1% faster)
📖	Linting SourceKitten with this PR took 1.11s vs 1.06s on master (4% slower)
📖	Linting Sourcery with this PR took 7.24s vs 7.2s on master (0% slower)
📖	Linting Swift with this PR took 11.12s vs 11.2s on master (0% faster)
📖	Linting WordPress with this PR took 17.63s vs 17.88s on master (1% faster)

Here's an example of your CHANGELOG entry:

* Implement baseline functionality.  
  [PaulTaykalo](https://github.com/PaulTaykalo)
  [#issue_number](https://github.com/realm/SwiftLint/issues/issue_number)

note: There are two invisible spaces after the entry's text.

Generated by 🚫 Danger

jpsim · 2020-11-10T03:11:26Z

Source/SwiftLintFramework/Baseline/Baseline.swift

@@ -0,0 +1,58 @@
+import Foundation


This file doesn’t appear to use Foundation

jpsim · 2020-11-10T03:12:16Z

Source/SwiftLintFramework/Baseline/Baseline.swift

@@ -0,0 +1,58 @@
+import Foundation
+
+public class Baseline {


The jazzy CI job is failing because these public declarations are undocumented.

stale · 2021-01-09T03:26:06Z

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!

dhoskins · 2021-01-13T17:36:13Z

This issue has been automatically marked as stale because it has not had any recent activity. Please comment to prevent this issue from being closed. Thank you for your contributions!

I'd like to prevent this issue from being closed.

bpollman · 2021-02-22T00:46:15Z

I have been hoping for this branch/feature to make it into the main release for a very long time. I'd be happy to help. What are the chances for this to get reviewed and have a chance at getting merged.

jpsim · 2021-02-23T17:15:35Z

Looking at this again for the first time in a while, there's a few things I'd like the author(s) to think through before we can consider something like this, which I think overall the concept is a good idea.

Should users be able to specify the baseline file path?
Should baselines apply to analyzer rules, or just lint rules?
Right now the baseline implementation is fairly brittle in the sense that if the line numbers change, the baseline record becomes out of date. Do we want to consider a mechanism to minimize the thrash when say a violation is moved down by a few lines because new code was added above it?
Should there be an easy way to reset the baseline? Right now you have to find the baseline file and delete it.
Can we somehow leverage the lint results cache? We're already saving lint results for faster incremental linting operations, so perhaps snapshotting that would be a new command distinct from swiftlint lint?
Unit tests, documentation and changelog entries would be important to add before considering merging something like this.

jpsim · 2021-02-23T17:16:58Z

As I was reviewing this, I was making some edits, which I always find helpful in understand code, they may or may not be helpful to anyone who's interested in continuing this work: https://github.com/realm/SwiftLint/commits/jp-baseline

igormatos · 2021-06-08T20:45:02Z

+1 for that feature

iandreyshev · 2022-07-08T12:12:51Z

+1 for that feature

abhimaandunzo · 2022-11-26T12:58:11Z

Source/SwiftLintFramework/Baseline/Baseline.swift

+    }
+
+    public func saveBaseline(violations: [StyleViolation]) {
+        let fileContent = violations.map(generateForSingleViolation).joined(separator: "\n")


would \n work on all platform ? trying point out things . like when files gets modified in git or in other os

abhimaandunzo · 2022-12-01T17:26:34Z

do we have this baseline implementation in pipeline . I dont see the any urgency in merging this pr. Let us know what is stopping may be we can help

AlfredAfutu · 2023-04-11T08:17:39Z

This is obviously needed as mentioned by a good number of people including myself. Is there a plan on how to proceed with this @jpsim ?

aiKrice · 2023-12-06T07:46:46Z

Please a lot of people are waiting for that.

mildm8nnered · 2024-01-14T23:31:50Z

I'm interested in picking this up. Not sure when I'll have time, or if I'll start from here, or from scratch right now.

Looking at this again for the first time in a while, there's a few things I'd like the author(s) to think through before we can consider something like this, which I think overall the concept is a good idea.

Should users be able to specify the baseline file path?

I think so. My use case is that I will almost always want the baseline to be derived from my main branch.

So I think something like --write-baseline <filename> and --read-baseline <filename> perhaps.

Should baselines apply to analyzer rules, or just lint rules?

I think so, but I would implement this (in CI) with separate baseline files.

Right now the baseline implementation is fairly brittle in the sense that if the line numbers change, the baseline record becomes out of date. Do we want to consider a mechanism to minimize the thrash when say a violation is moved down by a few lines because new code was added above it?

So I was thinking something like, if the file has the same violations, in the same order, ignore the violations regardless of line number.

Not sure how to handle existing violations if say one new violation appearing in a file - whether to report all the violations in the file, or to try to detect which one is "new".

For example, if my baseline violations are

identifier_name, line 20

and my new violations are

identifier name, line 25
identifier name, line 30

which is the new one

Possibly ignore them if the line numbers match exactly, but otherwise report them all.

Should there be an easy way to reset the baseline? Right now you have to find the baseline file and delete it.

I think if we supported --write-baseline <filename> and --read-baseline <filename> with the same <filename>, that would work.

Can we somehow leverage the lint results cache? We're already saving lint results for faster incremental linting operations, so perhaps snapshotting that would be a new command distinct from swiftlint lint?

I would definitely take a look at that.

Unit tests, documentation and changelog entries would be important to add before considering merging something like this.

aiKrice · 2024-02-24T19:35:07Z

very eager to have this feature !!!

mildm8nnered · 2024-02-25T22:50:58Z

So I have a very basic implementation at #5475

Seems to work for my basic test case, but will try to work on it and to add some more docs next week.

SimplyDanny · 2024-05-01T18:56:58Z

Closing as #5475 has superseded this PR. With that being merged, we now have a baseline functionality experimentally available in SwiftLint.

Implement baseline functionality

79c30af

PaulTaykalo requested a review from jpsim November 9, 2020 20:33

jpsim reviewed Nov 10, 2020

View reviewed changes

stale bot added the wontfix Issues that became stale and were auto-closed by a bot. label Jan 9, 2021

stale bot removed the wontfix Issues that became stale and were auto-closed by a bot. label Jan 13, 2021

jpsim mentioned this pull request Feb 23, 2021

Baseline #2580

Closed

jpsim mentioned this pull request Nov 25, 2022

Baseline of swiftlint warning #4577

Closed

abhimaandunzo reviewed Nov 26, 2022

View reviewed changes

ayushgargdunzo mentioned this pull request Nov 28, 2022

Swiftlint baseline functionality danger/swift#549

Open

esilverberg mentioned this pull request Apr 18, 2023

Baseline support, a.k.a. disabling rules based on file name regexps #4908

Closed

SimplyDanny mentioned this pull request Feb 17, 2024

Implement rule-level warning_threshold #5469

Open

2 tasks

mildm8nnered mentioned this pull request Feb 25, 2024

Baseline implementation #5475

Merged

SimplyDanny closed this May 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement baseline functionality #3421

Implement baseline functionality #3421

PaulTaykalo commented Nov 9, 2020

SwiftLintBot commented Nov 9, 2020

jpsim Nov 10, 2020

jpsim Nov 10, 2020

stale bot commented Jan 9, 2021

dhoskins commented Jan 13, 2021

bpollman commented Feb 22, 2021

jpsim commented Feb 23, 2021

jpsim commented Feb 23, 2021

igormatos commented Jun 8, 2021

iandreyshev commented Jul 8, 2022

abhimaandunzo Nov 26, 2022

abhimaandunzo commented Dec 1, 2022

AlfredAfutu commented Apr 11, 2023

aiKrice commented Dec 6, 2023

mildm8nnered commented Jan 14, 2024

aiKrice commented Feb 24, 2024

mildm8nnered commented Feb 25, 2024

SimplyDanny commented May 1, 2024

Implement baseline functionality #3421

Implement baseline functionality #3421

Conversation

PaulTaykalo commented Nov 9, 2020

SwiftLintBot commented Nov 9, 2020

jpsim Nov 10, 2020

Choose a reason for hiding this comment

jpsim Nov 10, 2020

Choose a reason for hiding this comment

stale bot commented Jan 9, 2021

dhoskins commented Jan 13, 2021

bpollman commented Feb 22, 2021

jpsim commented Feb 23, 2021

jpsim commented Feb 23, 2021

igormatos commented Jun 8, 2021

iandreyshev commented Jul 8, 2022

abhimaandunzo Nov 26, 2022

Choose a reason for hiding this comment

abhimaandunzo commented Dec 1, 2022

AlfredAfutu commented Apr 11, 2023

aiKrice commented Dec 6, 2023

mildm8nnered commented Jan 14, 2024

aiKrice commented Feb 24, 2024

mildm8nnered commented Feb 25, 2024

SimplyDanny commented May 1, 2024