realm / SwiftLint

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

Config file option to set all violations to errors #5679

Closed ehyche closed 1 month ago

ehyche commented 1 month ago

New Issue Checklist

New rule request

I want to set all of my SwiftLint violations to be errors instead of warnings.

I realize that if I am running from the command-line (or Build Script Phase) that I can just add the --strict option. However, I am using the SwiftLint SPM plugins. So I do not control the invocation of the swiftlint lint call - the SPM plugin does. The only option that I have is the .swiftlint.yml file.

So I need some way in my .swiftlint.yml file to turn the violation severity for every rule to be error instead of warning.

Is there already some way of doing that?

SimplyDanny commented 1 month ago

Set

strict: true

in the configuration.

ehyche commented 1 month ago

I added the following in my .swiftlint.yml:

diff --git a/.swiftlint.yml b/.swiftlint.yml
index 35d9a78..c0c8700 100644
--- a/.swiftlint.yml
+++ b/.swiftlint.yml
@@ -1,3 +1,5 @@
+# This makes all warnings be errors
+strict: true
 opt_in_rules:
   - closure_spacing
   - array_init

and it did not make any difference. Did I add this correctly?

SimplyDanny commented 1 month ago

Yes, that's how it should work with versions >=0.53.0.

ehyche commented 1 month ago

I am on 0.55.1, and adding strict: true at the top of my .swiftlint.yml did not make any difference. Any ideas?

ehyche commented 1 month ago

From my Package.resolved:

    {
      "identity" : "swiftlint",
      "kind" : "remoteSourceControl",
      "location" : "https://github.com/realm/SwiftLint.git",
      "state" : {
        "revision" : "b515723b16eba33f15c4677ee65f3fef2ce8c255",
        "version" : "0.55.1"
      }
    },
SimplyDanny commented 1 month ago

You seem to use the plugin. Are you sure, .swiftlint.yml is really considered at all?

PS: I recommend using the plugins from the dedicated repository. This comes with a bunch of advantages you can read up in its README.

ehyche commented 1 month ago

Yep, I have down on my ToDo list to switch over to pulling the SPM plugins from https://github.com/SimplyDanny/SwiftLintPlugins instead of https://github.com/realm/SwiftLint.

But yes, the .swiftlint.yml is used when using the SPM plugins. I have a .swiftlint.yml in the root of my repository, and then several local packages. Each local package in the repository has its own .swiftlint.yml in the local package root, but each of those just looks like:

parent_config: ../../.swiftlint.yml

And when we modify the .swiftlint.yml at the root of the respository, we can see the effect of those changes (enabling or disabling rules). So we know the .swiftlint.yml is being read and acted upon.

It just doesn't seem like the strict: true has any effect, however.

SimplyDanny commented 1 month ago

But yes, the .swiftlint.yml is used when using the SPM plugins. I have a .swiftlint.yml in the root of my repository, and then several local packages. Each local package in the repository has its own .swiftlint.yml in the local package root, but each of those just looks like:

parent_config: ../../.swiftlint.yml

That's a totally different story now. 😅

All options possible in a configuration have a default value which is implicitly set even if an option is not mentioned in a certain configuration. Together with the rule that child configurations specialize parent configurations, the configurations in your packages override the values in your parent. Since strict is false by default, your strict: true is just overruled.

That's a bit unfortunate, but it's the way things have worked since forever.

That said, and even though a little unwieldy,

parent_config: ../../.swiftlint.yml
strict: true

should work.

ehyche commented 1 month ago

Yep, that worked. Thanks for your help, @SimplyDanny !

SimplyDanny commented 1 month ago

I'm not sure if we should ponder whether to change the current semantic, as it's partially confusing indeed. Changing it would mean breaking a lot of existing setups. Also, the "correct" semantic is yet to be defined considering that there can be parent and child configurations.

ehyche commented 1 month ago

I do find it weird that my parent configuration had strict: true, and the child configs didn't mention strict at all, but yet the child default of strict: false could still override. The more obvious behavior would be that if a child config doesn't specifically override a config value, then the parent config value would be in effect.

SimplyDanny commented 1 month ago

I have filed #5724 to discuss this further.