SwiftLint
SwiftLint copied to clipboard
realm
Fix excluded path calculation when passing in individual files
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.
| 12 Messages | |
|---|---|
| :book: | Linting Aerial with this PR took 2.05s vs 2.0s on master (2% slower) |
| :book: | Linting Alamofire with this PR took 2.86s vs 2.94s on master (2% faster) |
| :book: | Linting Firefox with this PR took 10.01s vs 9.85s on master (1% slower) |
| :book: | Linting Kickstarter with this PR took 15.4s vs 15.68s on master (1% faster) |
| :book: | Linting Moya with this PR took 1.49s vs 1.41s on master (5% slower) |
| :book: | Linting Nimble with this PR took 1.29s vs 1.27s on master (1% slower) |
| :book: | Linting Quick with this PR took 0.62s vs 0.6s on master (3% slower) |
| :book: | Linting Realm with this PR took 4.25s vs 4.33s on master (1% faster) |
| :book: | Linting SourceKitten with this PR took 1.07s vs 1.05s on master (1% slower) |
| :book: | Linting Sourcery with this PR took 8.5s vs 8.62s on master (1% faster) |
| :book: | Linting Swift with this PR took 11.37s vs 11.37s on master (0% slower) |
| :book: | Linting WordPress with this PR took 18.65s vs 18.69s on master (0% faster) |
Generated by :no_entry_sign: 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 -- argument so that's not surprising.
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.
| Before | After |
|---|---|
 |
 |
The "group files" step on master takes about 10.5 seconds (already very long if you ask me, but there are tens of thousands of Swift files to group).
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
Dir1 and Dir2 have hundreds of subdirectories, a few layers deep, with Swift files mostly at the leaves.
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: https://github.com/realm/SwiftLint/pull/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?