realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.65k stars 2.22k forks source link

Keeping up to date with new rules #2295

Closed alexito4 closed 9 months ago

alexito4 commented 6 years ago

New Issue Checklist

Enhancement Request

I would like to configure the linter in a way that has all the rules enabled by default and we can disable the specific ones we don't want in the config file, kind of what you can do with Xcode warnings. This makes it easy to use new rules when updating the linter and therefore keep an eye on improvements that can be done in your codebase.

My initial idea was to use [--enable-all-rules] but as the documentation mentions this will ignore the disabled rules so it's not working for the described scenario.

[--enable-all-rules]
    run all rules, even opt-in and disabled ones, ignoring `whitelist_rules`

The current alternative I"m using is to run ./swiftlint rules --config ./swiftlint.yml -d after an update. That shows the disabled rules but it shows both the opt-in and the disabled ones. So you need to visually parse the table or write some command that filters only for the opt-in ones that are not disabled in the config.

Or is this a scenario that you wouldn't recommend?

Thanks

marcelofabri commented 6 years ago

Would it be feasible to add a variation of --enable-all-rules that enables opt-in rules but respects the disabled ones in the config?

I'm not sure this approach is worth the extra complexity. I'd rather try to think about something where we solve the problem in a better way.

One approach would be to add the information of when (which version) a rule was added to SwiftLint as part of RuleConfiguration. Then we could have a new command that shows rules added between the two versions. In this case, you'd need to know what was the previous version you were using though.

alexito4 commented 6 years ago

I'm not sure this approach is worth the extra complexity.

Yes I think you're right that's why I wanted to ask for another solution 😃. That said, I wonder what's the thinking behind not considering this use case, in my mind I see it as an extension of Xcode -Weverything. I'm not trying to push for it, quite the opposite, I'm more worried about if I shouldn't be thinking about it in this way 🤔

One approach would be to add the information of when (which version) a rule was added to SwiftLint as part of RuleConfiguration. Then we could have a new command that shows rules added between the two versions.

This seems like a good solution to me. Do you think this would be a new command or an extension of the rules one? An idea around this would be to mimic some package managers behaviour and check online for a list of new rules since the current local installation. But that's going the extra mile 😅.

In this case, you'd need to know what was the previous version you were using though.

It may not be ideal but I think it's not a problem honestly. It's a small annoyance compared to having to read the list and filter it in your head ^^

Thanks for the reply and the enhancement consideration ^^

marcelofabri commented 6 years ago

Do you think this would be a new command or an extension of the rules one?

I think it could be another parameter to rules since it'd allow combining with the other options.

An idea around this would be to mimic some package managers behaviour and check online for a list of new rules since the current local installation

I don't think that makes to much sense because it's not like they can run a command to update the rules, they need to update SwiftLint itself.

DarkoDamjanovic commented 5 years ago

I would like to request the same feature like alexito4 describes:

I would like to configure the linter in a way that has all the rules enabled by default and we can disable the specific ones we don't want in the config file, kind of what you can do with Xcode warnings. This makes it easy to use new rules when updating the linter and therefore keep an eye on improvements that can be done in your codebase.

Adding another parameter to rules would need to constantly check and maintain if rules have changed. imho it would be way easier to handle if there would be an option to just enable all of the rules but the one in disabled_rules keep to be respected.

Proposal:

opt_in_rules:
  - all

I wonder why this is not the default anyway as most linters I know work like this. At first all are enabled and then it is possible to apply certain templates (like "most_used_rules".. etc..) or just disable specifically one by one.

Thanks for your attention.

4brunu commented 5 years ago

I'm also interested in this. Another option would be ./swiftlint lint --enable-all-opt-in-rules What do you think? @marcelofabri If you agree I can try to create a PR with it. Thanks

acecilia commented 5 years ago

Another alternative; the way I manage to be updated about all rules I am interested in is by:

When a new version of swiftlint is released, I can use the diff between swiftlint rules executions to catch new/removed/changed rules easily, and update my swiftlint.yml file accordingly. Also, in my experience, this file is quite large and swiftlint takes some time to parse it (like 1-2 seconds on every execution). Enabling the cache fixes this problem.

Hope you find this useful :)

grantneufeld commented 4 years ago

I would prefer to not have to check SwiftLint’s rules with every update, and then have to modify my .swiftlint.yml files across all of my projects, to add each new rule.

Something like a simple opt_in_rules: all (as has been suggested above), with disabled_rules: able to override that on a per-rule basis, would be ideal.

Yes, in some rare cases, there are opt-in rules that are mutually exclusive (such as extension_access_modifier and no_extension_access_modifier)—almost guaranteeing warnings when both are active. I’d rather have warnings pop up in Xcode when those rules are added to SwiftLint, than not have either of the rules running at all. Which is actually a good way to get me to read up on the rules and what they mean and why I might (or might not) want to follow them. I can then go and disable the one(s) I don’t want.

Thanks!

acecilia commented 4 years ago

@grantneufeld Just some more context: I maintain a unique swiftlint.yml file shared across projects. I use submodules for it.

alexito4 commented 4 years ago

One alternative I was thinking is to improve the table generated with the ./swiftlint rules --config ./swiftlint.yml -d command. Right now there is a column called enabled in your config but even if you have the rule on your config but is disabled, it will still show up in there. If we could make all the rules that are in the config be removed from the table (disabled or not doesn't matter) or even add a new column that says "present in config", it would help a lot. We can just have the table as a txt in git and easily see the diff.

mdaymard commented 4 years ago

I'm also very interested in that feature. --enable-all-rules-except-disabled would be perfect

stale[bot] commented 3 years ago

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!

grantneufeld commented 3 years ago

Has there been any progress or further thinking on this issue? I really hope the option to have new rules auto-included can be implemented. The default non-inclusion remains suboptimal. Thanks.

stale[bot] commented 3 years ago

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!

alexito4 commented 3 years ago

I wrote my process here https://alejandromp.com/blog/how-to-update-swiftlint/ in case it helps anybody looking at this issue.

mildm8nnered commented 9 months ago

I think this was resolved by https://github.com/realm/SwiftLint/pull/4544

alexito4 commented 4 months ago

nevermind, i was still using the only_rules, I didn't realise it had to be used in the opt_in_rules

Details @mildm8nnered do you know if using the `all` still outputs `yes` in the `enabled in your config` column of the rules command output? ![Screenshot 2024-06-05 at 14 56 17](https://github.com/realm/SwiftLint/assets/750912/f3c4e28e-3551-4765-b5bb-52fdd88aac61) Switching to use `all` and diffing the output of the command table seems to indicate the opt-in rules are not enabled anymore. Is it expected as the rule is not explicitly in the file?
alexito4 commented 4 months ago

Just an update for future reference.

This works great! My previous setup was to have only_rules and then list the disabled ones commented. Using opt_in_rules all technique has made realise that there were a few rules that I didn't take into account and where disabled.

And that's the thing, with time I've made my workflow be very explicit in the config file about the rules. I list the rules enabled and also the disabled ones, with reasoning on both when necessary since not all are clear yes or no.

With this in mind I realise now that having the opt_in_rules all doesn't help me a lot because I still want to check every rule and put it in the config, I want explicit control as to help the team know why we have the setup we have. By using optin all I endup by not knowing which ones are enabled. Still I have the rules table commited in the repo so it's easy to see changes. Ultimetly doing my old system or the new avialble with optin all doesn't make a difference to me so I'm still deciding if is worth switching.

Turns out that what I really want from a config system for a linter/formatter is:

I realise I'm probably the only one with the need of this explicit workflow so I will move on with what I have now and keep this issue closed 😉

In any case thanks Martin for the new option ❤️