Fix bug where nested configuration excluded files are ignored #2648
Conversation
There was a problem hiding this comment.
@marcelofabri @Dschee I'd appreciate some feedback when you get the chance :)
There was a problem hiding this comment.
I've made some comments, please review them. Apart from that, I hope @marcelofabri or @jpsim can jump in here regarding if this is ok to merge since I have no experience at all with multiple configuration files yet.
Generated by 🚫 Danger |
|
Thanks for updating the code. I have one more question: Do you think you could add tests to prevent this change from breaking again in the future? |
|
Thanks for the PR @Bruschidy54!
Tests would be great, although we don't currently have the ability to run unit tests against the |
|
@Dschee @jpsim Is it possible to accept the PR without |
|
Yes, should be fine. I have some small followups that I'll do in a separate PR to avoid force-pushing to your fork's Thanks for the fix @Bruschidy54 and for your code review @Dschee! |
|
Small followups here: #2707 |
Fixes #2447
When SwiftLint is linting files in
visitLintableFilesinConfiguration+CommandLine, it:getFiles. This is where the exclusion of files occurs based on the parent configuration's exclusion list.groupFiles. This is where configurations for each available file are determined (and if nested configurations exist, merged). After these configurations are determined and the files are grouped accordingly, no more files are excluded from the lintable files list. Even though a file's configuration thinks it should be excluded, these files are not removed from the list of lintable files, generating the bug.My solution is to skip files whose merged configurations specify they should be excluded in step 2 or
groupFiles. Therefore, they will not be visited in step 3.