Revive excludedPaths by stopping double resolution - #6825
Revive excludedPaths by stopping double resolution#6825tomotakatakahashi wants to merge 1 commit into
Conversation
Generated by 🚫 Danger |
This commit stops resolving a path to an absolute path in Configuration.groupFiles. This will resolve the issue mentioned in comments in realm#6795 that the "excluded" configuration in .swiftlint.yml is ignored when file paths are passed as arguments.
3e1a6c8 to
40ce904
Compare
SimplyDanny
left a comment
There was a problem hiding this comment.
This change is causing changes in scanned OSS repositories. That's definitely unexpected and should be revisited.
|
Thank you for your feedback. I wasn't entirely clear on the comment from SwiftLintBot, but I now understand that I need to address the warnings. I will try to look into this, though my time might be a bit limited. |
|
@SimplyDanny After a little investigation, I now think the changes in the scanned OSS repositories are expected. The files listed under "This PR fixed a violation":
are all excluded via .swiftlint.yml files (1, 2, 3, 4) in that repository. So if the main branch were working correctly, these files shouldn't produce any violations at all. Therefore "This PR fixed a violation" messages are desirable and expected. My guess is that the double-resolution bug was introduced by PR #6636, but somehow SwiftLintBot couldn't catch it. Could you take a look? |
Summary
This commit stops resolving a path to an absolute path in
Configuration.groupFiles. This will resolve the issue mentioned in comments in #6795 that theexcludedconfiguration in .swiftlint.yml is ignored when file paths are passed as arguments.Context
The
excludedconfiguration is sometimes ignored since v0.64.0 (#6795 (comment)). The issue seems to happen when file paths are passed as arguments (for example, runningswiftlint Foo.swift Bar.swiftinstead ofswiftlint .orswiftlint).Using a debugger, I found that
excludedRelativePathin the diff was actually not a relative path, because it had already been resolved in Configuration+Parsing.swift. As a result,let excludedPathin the diff was being doubly-resolved to an invalid path like"file:///Users/username/projects/foo/Users/username/projects/foo/MyPackage/Package.swift".Test
Arguments Passed On LaunchNoProblem.swift MyPackage/Package.swift, running the swiftlint scheme:Comment