Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug where SwiftLint ignores excluded files list in a nested configuration file #2621

Closed
wants to merge 15 commits into from

Conversation

Bruschidy54
Copy link
Contributor

@Bruschidy54 Bruschidy54 commented Jan 31, 2019

Fixes #2447

When SwiftLint is linting files in visitLintableFiles in Configuration+CommandLine, it:

  1. Gathers all lintable files in getFiles. This is where the exclusion of files occurs based on the parent configuration's exclusion list.

  2. These files are grouped by their specific configuration in 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.

  3. Finally, each file is visited by the linter.

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.

@Bruschidy54 Bruschidy54 changed the title Fix bug where SwiftLint ignores excluded files in a nested configuratin File Fix bug where SwiftLint ignores excluded files list in a nested configuration file Jan 31, 2019
@SwiftLintBot
Copy link

SwiftLintBot commented Jan 31, 2019

1 Error
🚫 Please rebase to get rid of the merge commits in this PR
12 Messages
📖 Linting Aerial with this PR took 1.88s vs 1.99s on master (5% faster)
📖 Linting Alamofire with this PR took 4.88s vs 4.12s on master (18% slower)
📖 Linting Firefox with this PR took 15.36s vs 12.63s on master (21% slower)
📖 Linting Kickstarter with this PR took 0.23s vs 0.2s on master (15% slower)
📖 Linting Moya with this PR took 2.32s vs 1.98s on master (17% slower)
📖 Linting Nimble with this PR took 2.14s vs 2.1s on master (1% slower)
📖 Linting Quick with this PR took 0.68s vs 0.65s on master (4% slower)
📖 Linting Realm with this PR took 4.08s vs 4.16s on master (1% faster)
📖 Linting SourceKitten with this PR took 1.34s vs 1.39s on master (3% faster)
📖 Linting Sourcery with this PR took 4.75s vs 4.24s on master (12% slower)
📖 Linting Swift with this PR took 27.16s vs 29.94s on master (9% faster)
📖 Linting WordPress with this PR took 20.82s vs 23.95s on master (13% faster)

Generated by 🚫 Danger

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants