Swiftlint: Change in behavior of nested configs

[marcelofabri]

New Issue Checklist

• [x] Updated SwiftLint to the latest version
• [x] I searched for existing GitHub issues

Describe the bug

After updating to 0.41.0, I've noticed a change of behavior with nested configs and using --config. We have a config file in the root of the project and nested configs in Test folders using disabled_rules.

Before

disabled_rules from the nested config would be applied when calling swiftlint lint --config .swiftlint.yml

After

disabled_rules from the nested config are NOT applied when calling swiftlint lint --config .swiftlint.yml. They're applied if I don't pass --config though.

This is especially a problem because we use danger-ruby-swiftlint, which always uses --config

Environment

• SwiftLint version (run swiftlint version to be sure)? 0.41.0
• Installation method used (Homebrew, CocoaPods, building from source, etc)? Release page
• Paste your configuration file:

# insert yaml contents here

• Are you using nested configurations?
  If so, paste their relative paths and respective contents.
• Which Xcode version are you using (check xcodebuild -version)? 12.2.1
• Do you have a sample that shows the issue? Run echo "[string here]" | swiftlint lint --no-cache --use-stdin --enable-all-rules
  to quickly test if your example is really demonstrating the issue. If your example is more
  complex, you can use swiftlint lint --path [file here] --no-cache --enable-all-rules.

// This triggers a violation:
let foo = try! bar()

[sethfri]

Sorry for this pain @marcelofabri. This was an intentional change that @jpsim and I discussed in #3362, and it was implemented in #3379. The behavior you were relying on was indeed how SwiftLint was working, but was deemed unintentional behavior. If you're explicitly passing in a config, it doesn't really make sense to merge that config with any other configs. We now treat an explicitly passed-in config as an "override" instead.

[sethfri]

This likely should have been noted as a breaking change in the release notes instead of a bug fix, but it _was_
released along with other breaking changes, so I think that's still ok in terms of versioning.

[marcelofabri]

Thanks for the clarification, @sethfri - I'm glad this is intentional. Unfortunately (as you've probably realized now), handling nested configs is kind of a landmine 😅

I've submitted https://github.com/ashfurrow/danger-ruby-swiftlint/pull/154 to handle this on danger-ruby-swiftlint

[sethfri]

Totally agree 😅

Hopefully the new behavior for --config is a little clearer going forward. It will always be treated as an explicit override and never implicitly merged with other configs. I think it's the implicit nature of SwiftLint's config system that makes it especially tricky in general

[cltnschlosser]

Is there any way to disable rules in a specific directory when not using nested configs?

[sethfri]

@cltnschlosser I don't believe so. I think that was the intended use case for nested configs. Is there a reason you can't use nested configs?

[cltnschlosser]

Well we have multiple projects using the same config via --config. Previously nested configs was being used as well, but that _feature_ was removed. It would be nice to share the configs without needing to duplicate them.

Maybe simlinks will work, will have to test.

[jpsim]

Can you not disable a rule in a nested config any more?

[sethfri]

@jpsim You can. I believe @cltnschlosser is talking about how passing in a config vía --config is treated as an override instead of being merged

[jpsim]

Ah yes, that’s totally what we discussed too when merging the PR.

[cltnschlosser]

Looks like we can simlink to the config and drop the --config parameter in all the projects.

[soranoba]

(root)
├── .swiftlint.yml
├── Dir (pwd)
│   ├──

Before 0.41.0, when swiftlint --path ../ was executed from pwd in the graph above, it behaved the same as when executed swiftlint from root.

That's not the case with 0.41.0.
Is this also the intended behavior change?

[jpsim]

SwiftLint's configuration merging/nesting behavior has always been a bit of a landmine. It's not always clear how given a mix of various parameters the tool _should_ behave and when it's not clear, how we can reason about the observed behavior.

Before continuing much further, I'd really appreciate if someone (perhaps @sethfri or @fredpi since you both had to consider a lot scenarios when touching configuration resolution lately) could take a stab at _documenting_ the various scenarios and expected behaviors for configuration resolution (finding + merging) so that we can more formally discuss tradeoffs in strategies.

We've covered configuration merging through unit tests before, but that hasn't done much to avoid accidentally breaking the complex range of possible usage patterns that exist "in the wild".

[fredpi]

@jpsim I guess it is best to document nested configs over at https://github.com/realm/SwiftLint/pull/3058. That PR already updates the documentation regarding nested configs and I will try to make it even more specific.

[buschtoens]

IMO what @soranoba described in https://github.com/realm/SwiftLint/issues/3417#issuecomment-724443010 is a legit bug:

(root)
├── .swiftlint.yml
├── Dir (pwd)
│   ├──

Before 0.41.0, when swiftlint --path ../ was executed from pwd in the graph above, it behaved the same as when executed swiftlint from root.

We have some excluded paths in our .swiftlint.yml. If I run just swiftlint lint from the (root), these paths are respected. However, if I run swiftlint lint from Dir with either of the following, the excluded paths are not honored:

• swiftlint lint --config ../.swiftlint.yml --path ../
• swiftlint lint --config ../.swiftlint.yml
• swiftlint lint --path ../
• swiftlint lint

[buschtoens]

In the hopes of this getting fixed and not being intended behavior (I don't see how it would be), here's how you can downgrade back to 0.40.3, in case you are using brew:

# Download the formula right before it was upgraded to `0.41.0`
wget https://raw.githubusercontent.com/Homebrew/homebrew-core/bb4c1a37e7f2e96da070ecc70a6d312d78bda672/Formula/swiftlint.rb

# Uninstall current version
brew uninstall swiftlint

# Install `0.40.3`
brew install ./swiftlint.rb

And now swiftlint lint --path ../ (as described in https://github.com/realm/SwiftLint/issues/3417#issuecomment-726218225) works as expected again.

[buschtoens]

Another option, if you don't want to downgrade, is to replace --path with cd, like so:

# broken
swiftlint lint --path ../

# works
(cd ../ && swiftlint lint)

By wrapping the commands in braces the CWD is only changed for swiftlint.

This works with 0.40.3 _and_ 0.41.0.

[jpsim]

@buschtoens I encourage you to weigh in on the efforts to formalize the file path resolution & merging of configurations in https://github.com/realm/SwiftLint/pull/3058 to ensure that your use cases are carefully considered.

[fredpi]

I have updated the documentation on multiple configurations via #3058 that is now merged into master. I guess we can close this then, @jpsim?