Closed aiKrice closed 1 month ago
Hi @aiKrice - I replied to your comment here - https://github.com/realm/SwiftLint/pull/5475#issuecomment-2120065797, but I'll reply here as well.
Firstly, thank you so much for the feedback, which is incredibly useful (and also for the Medium articles - it is great to get the word out there).
Coming back to your specific issues:
You need to use the same config file when you generate the baseline with --write-baseline
as when you use it for filtering via --baseline
, as the config file will determine which rules are in effect, and also things like rule configurations, thresholds for individual rules etc.
This should work, as in both cases, swiftlint
is just running the lint
command, and this has hardly changed in the baseline PR, apart from some hooks to write and/or read the baseline.
It's maybe because this file was listed as excluded in my config file" - it is exactly that.
You can generate a baseline with --enable-all-rules
, but you should not do that, as it will contain all possible violations, and will not have any rule configurations or rule specific thresholds etc.
The reason that is mentioned in #5553 is more as a way of saying - "these are all our possible violations, let's pick which rules from here we would like to enable".
I agree that this can be a bit annoying and unhelpful when trouble-shooting. During development, I did turn pretty printing on, and in the Periphery implementation of the baseline feature I currently have pretty-printing on.
The reason I turned this off was because of concerns about baseline file size. Not so much in the context of a single baseline, but more in the context of updating baselines under source control where they will occupy space in the git history.
With pretty printing, the size of the file is increased by about ~25% (ballpark, I don't have exact figures offhand).
Originally we were going to recommend compressing the baseline in the repo, and uncompressing it before use, or even having SwiftLint compress and uncompress the file itself.
Having done some experiments with git history sizes - see https://github.com/realm/SwiftLint/issues/5553#issuecomment-2094753091, it actually looks like its better to store it uncompressed, as that way git
's internal packing and compression will actually produce good compression ratios without any extra work on our part.
In the absence of pretty-printing, you can use the baseline
command to see what violations are present in the baseline - for example:
swiftlint baseline --reporter summary MyBaseline.json
for a summary, or something like
swiftlint baseline --reporter xcode MyBaseline.json
for individual warnings. You could even run the latter a run script build phase, to see the Baseline violations reported directly within Xcode.
In Periphery there is no equivalent of the baseline
command, so for right now, I'm leaving pretty-printing on, but that may change as we work our way though code review etc.
Hope this helps - please let us know if things are not working the way you expected after this.
Any feedback about bugs, usability, whether the documentation is clear enough etc. are very very welcome.
Incidentally, I was very interested to see https://detekt.dev/docs/introduction/baseline/ and https://googlesamples.github.io/android-custom-lint-rules/usage/baselines.md.html which I think you mentioned somewhere, and which I was not aware of - I will definitely have a closer look at those.
Hello @mildm8nnered , Thank for being so reactive 🚀 .
1️⃣. Baseline generation:
I've tried this command swiftlint --write-baseline baseline.json -config config.yml file.swift
but I "didn't" expect this if I compare to Dekekt and AndroidLint. Because this means that I have to provide all files one by one ? In this case is there a parameter for that, I probably miss it. But I really think this could be improve (on a developer experience perspective), meanwhile I will implement a recursive find and update my Medium Article based on that. I prefer this rather than having a huge baseline.
2️⃣. Baseline writing output. I was trapped ! I didn't understand that the output because it said: linting done! found violations and I didn't expect such an output which could lead me that the baseline failed. In this case, I suggest a verbose mode disabled by default or something else. Also I am wondering why if I add a file in the exclusion in my config.yml, it is taken in account in the baseline generation. It should be excluded (it is like this in Detekt and AndroidLint)
3️⃣ . Ok aligned of course. I did it because I though no choice
4️⃣ . If we could provide a pretty printing option it would be nice (i'm aligned of course on what you said).
To sum it up, here are my opinions and suggestions : Baseline generation command: We could provide a system to browse all swiftlint file (except if I have one) Baseline output: disabled output by default / provide a verbose Option for json prettifier.
Overall. Nice !!!
Gonna modify my Medium article with temp solution of recursive swift browsing.
And available for discussing for Android equivalent. The friendly link medium article for how to generate the baseline for AndroidLint or Detekt in a nutshell is in an another article -> https://medium.com/@SaezChristopher/4-best-tools-to-implement-into-your-githooks-in-an-android-project-dfc93e1852b7?sk=a15ad91c046ab14f17614b7c43a486b0
Hello @mildm8nnered , Thank for being so reactive 🚀 .
1️⃣. Baseline generation: I've tried this command
swiftlint --write-baseline baseline.json -config config.yml file.swift
but I "didn't" expect this if I compare to Dekekt and AndroidLint. Because this means that I have to provide all files one by one ? In this case is there a parameter for that, I probably miss it. But I really think this could be improve (on a developer experience perspective), meanwhile I will implement a recursive find and update my Medium Article based on that. I prefer this rather than having a huge baseline.
So you should not have to provide your files one by one at all. For "medium-ish" side projects, for some value of "medium-ish" - mine is quarter of a million lines of Swift code, or thereabouts, enabling about 20 rules that I would have liked to turn on, but couldn't because of existing violations, the baseline is about 2.5MB.
That is not really that large by modern standards. Even with --enable-all-rules
, the baseline there for me is about 7.5MB, which is more substantial, but not really that bad. We commit CocoaPods in my main work project, and the SwiftLint binary is 33MB.
I wasn't aware of a lot the prior art when I started, so I can't comment yet on Detekt and AndroidLint (but I am intending to check them out), but in summary, I would say, just invoke SwiftLint exactly how you would normally, with identical command lines in the writing (--write-baseline
) and filtering (--baseline
) cases, apart from the baseline arguments.
2️⃣. Baseline writing output. I was trapped ! I didn't understand that the output because it said: linting done! found violations and I didn't expect such an output which could lead me that the baseline failed. In this case, I suggest a verbose mode disabled by default or something else. Also I am wondering why if I add a file in the exclusion in my config.yml, it is taken in account in the baseline generation. It should be excluded (it is like this in Detekt and AndroidLint)
So in SwiftLint's case, the baseline just says "ignore and do not report violations that are in the baseline" - there is no real extra output to say "we filtered these many violations".
We could definitely look at adding something at least as a summary "0 violations detected, 6,107 violations filtered", but we don't do that right now.
Reporting filtered violations in detail - that would be difficult. You might want to know "how many violations were filtered", or "how many violations in the baseline are not actually present in the tree any more". You can do that with the baseline
command, and it's compare
subcommand, but it takes extra work.
4️⃣ . If we could provide a pretty printing option it would be nice (i'm aligned of course on what you said).
To sum it up, here are my opinions and suggestions : Baseline generation command: We could provide a system to browse all swiftlint file (except if I have one)
Could you clarify that a bit, if my response to 1) above doesn't already cover that for you?
Baseline output: disabled output by default / provide a verbose Option for json prettifier.
So swiftlint baseline --reporter json Baseline.json
will pretty-print the JSON for you.
Re-using the reporters for this is actually really nice, because a baseline is just a collection of violations, and we have multiple reporters (and can always add more) for reporting violations in various formats.
Overall. Nice !!!
Thanks!
Gonna modify my Medium article with temp solution of recursive swift browsing.
So maybe we could thrash out the issues here, and that might save you having to make multiple edits (and once again, thank you so much for popularizing this).
And available for discussing for Android equivalent. The friendly link medium article for how to generate the baseline for AndroidLint or Detekt in a nutshell is in an another article -> https://medium.com/@SaezChristopher/4-best-tools-to-implement-into-your-githooks-in-an-android-project-dfc93e1852b7?sk=a15ad91c046ab14f17614b7c43a486b0
I am definitely going to check those out as soon as I have a chance!
For info @mildm8nnered . I have performed:
swiftlint --write-baseline config/swiftlint/baseline.json --config config/swiftlint/swiftlint.yml $(find . -name "*.swift")
It works, for sure.
For point 4, tried also, it works with the command. At least no need to install an additionnal tools.
I will perform a last update of the article. Thank you 👍
So you should not need to do the $(find . -name "*.swift")
trick - SwiftLint should be looking for the *.swift
files itself, based on your configuration, just like a normal lint
run.
Does that not work for you?
swiftlint --write-baseline config/swiftlint/baseline.json --config config/swiftlint/swiftlint.yml $(find . -name "*.swift")
It works, for sure.For point 4, tried also, it works with the command. At least no need to install an additionnal tools.
I will perform a last update of the article. Thank you 👍
@mildm8nnered Do you see something wrong / missing in my config file above ?
swiftlint --write-baseline config/swiftlint/baseline.json --config config/swiftlint/swiftlint.yml
Output
Linting Swift files in current working directory
Error: No lintable files found at paths: ''
I tried --path .
but it says it's deprecated + it doesnt work as well XD
warning: The --path option is deprecated. Pass the path(s) to lint last to the swiftlint command.
Linting Swift files at paths .
Error: No lintable files found at paths: '.'
And my team and I are running under zsh
So I think it's not your config file per se, but where you've put it (in config/swiftlint
). The (very recently) updated readme says "Configure SwiftLint by adding a .swiftlint.yml
file from the directory you'll run SwiftLint from."
SwiftLint implicitly expects the main config file to be at the top of the tree, and then can use various mechanisms to refine the configuration.
In your case, SwiftLint is probably trying to scan config/swiftlint/Shopmium
etc.
If you need to have the config file in config/swiftlint
, I think you can probably adjust the paths (../../Shopmium
perhaps), and that should probably work.
New Issue Checklist
Describe the bug
I played this week end on with the baseline feature. First of all thank @mildm8nnered and all contributors, Amazing job and I am here also to help with my way, by trying hard.
I noticed several behavior that imho, are not exactly what I expected, as an Android developer as well, Detekt and AndroidLint baseline user.
1️⃣. I generate a baseline with this command:
I noticed a different behavior in the content of the baseline if I specifiy my config file
2️⃣. In both case above, I noticed that some rules some not ignored in the baseline so swiftlint failed. It was these rules :
It's maybe because this file was listed as excluded in my config file (see it below #githooks)
3️⃣. I generated a baseline with all rules enabled (as suggested in another issue opened) (but baseline should not be generated this way but against a configuration file), In this case "it works".
4️⃣. The baseline json file is a single 1 line file 😱 . In this case, Xcode (well, xcode...) cant read it. So I use a third party to format it: prettier -> https://github.com/prettier/prettier to format it. I think this should be opt-in because to compare this on a PR even to make it readable, it better. But by using prettier and an intermediate tool, there are some noises and too many illegitimates diffs.
Voila, hoping it can help, I am ready to kepp helping for improving it on my project 💪 .
Environment
version: 0.55.1 Homebrew, Mac M1PRO 14" Xcode 15.4 Build version 15F31d