tuist / tuist-plugin-lint

A plugin that extends Tuist with SwiftLint functionalities.
MIT License
37 stars 5 forks source link

Honour excluded files within SwiftLint config file #12

Open lordcodes opened 2 years ago

lordcodes commented 2 years ago

When using the regular SwiftLint binary it can extract customisations from a .swiftlint config file, such as opt-in rules to enable, rules to disable, configurations for different rules and files to exclude. This currently doesn't seem to be possible when using the plugin as it instead intelligently works out the files to lint.

It should involve looking at how the SwiftLint CLI target extracts customisations from this config file and how it passes it to the SwiftLintFramework, that this plugin is using internally.

danieleformichelli commented 2 years ago

Is the configuration not read at all or just the included/excluded files?

lordcodes commented 2 years ago

I added a file to be excluded and that didn't seem to be honoured. But that is a very good point, due to the excluded file not being excluded I made the assumption it wasn't read due to the plugin using SwiftLintFramework. I will confirm this is actually the case and check SwiftLint to check this behaviour is provided by the framework and not by the SwiftLint binary they provide.

lordcodes commented 2 years ago

It looks like '.swiftlint.yml' is actually handled within SwiftLintFramework. It also looks like there is a separate argument concerning whether to exclude a file that the config file says to exclude but that is listed specifically in the list of input files. Therefore, I suspect that is all that is happening here.

https://github.com/tuist/tuist-plugin-lint/blob/1cf03aa2384ea428abe932e8eeaae750dd2d9826/Sources/TuistPluginSwiftLintFramework/SwiftLintFrameworkAdapter/SwiftLintFrameworkAdapter.swift#L90

forceExclude is false, which means if the file is 'excluded' within the SwiftLint config file it will still be linted. The swiftlint binary offers forceExclude as a command line argument, defaulting it to false.

The main difference, is generally when using swiftlint binary, you can just run SwiftLint from top of your project without any filepaths as input and so 'excluded' in config is always honoured. However, in this case, as the plugin specifies the list of files to run it on, the 'excluded' in config will always be ignored due to forceExclude being false. As a user of this plugin isn't specifying the list of files to lint themselves, I feel forceExclude should be defaulted to true in the plugin, as otherwise the result feels unexpected as a consumer. It basically just means the excluded option in config is always ignored.