Fix excluded path calculation when passing in individual files #3534
Conversation
While we currently resolve globs in excluded paths when SwiftLint is run from the root directory, we do not resolve them when individual files are passed in using the `--` argument. This fixes that.
6908fce to
0fcad96
Compare
Generated by 🚫 Danger |
|
Thanks for the PR, the change looks good! It'd be great to add tests to the CLI target, but that's a different discussion. At least OSSCheck didn't surface any changes, though we're not passing individual files directly using the I'm going to run this on some closed source projects to check for possible regressions and will merge if nothing comes up. |
|
Unfortunately this causes some significant performance regressions in one of my closed-source projects.
The "group files" step on The same step with this change takes at least 5 minutes, I don't know how long it would take to complete because I stopped it there. I can't share the project, but the relevant parts are this configuration: included: [Dir1, Dir2]
excluded:
- Dir2/*/.ext
- Dir2/Subdir1/tests/fixtures
- Dir2/Subdir2
PS: To get these numbers, I started peppering some OS Signpost events and spans in the CLI target. I'd like to clean that up and make a PR since I believe that would be very helpful to others debugging SwiftLint's performance. |
|
Pushed up a PR with the signposts integration: #3535 |
|
@jpsim Sorry, haven't had a chance to get back to this. I'm stuck at the part where I need to create my own sample project with hundreds of subdirectories 😅 Maybe it would be good to create some sample projects for contributors to test against? |
While we currently resolve globs in excluded paths when SwiftLint is run
from the root directory, we do not resolve them when individual files
are passed in using the
--argument. This fixes that.It doesn't look like this code path currently has any integration tests, and providing the ability to easily add new integration tests looks like a much bigger project. I have tested this locally and verified that it fixes the bug I was seeing in
0.42.0, but feel free to suggest other tests I can add within the existing test infrastructure.