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 finding the nested config when a single file path is passed #3379

Merged
merged 3 commits into from
Nov 8, 2020

Conversation

sethfri
Copy link
Contributor

@sethfri sethfri commented Oct 11, 2020

This was previously attempted in #3342, but produced a bug in the case where --config is used to specify a config from outside of the source tree. The --config argument wasn't always being used as an override, and was being merged with the config in the source tree. This has now been addressed and reverts the revert done in #3362.

Fixes #3341

@SwiftLintBot
Copy link

SwiftLintBot commented Oct 11, 2020

31 Warnings
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/ManifestLoader.swift:38:9: warning: Inclusive Language Violation: Declaration blacklist contains the term "blacklist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Aerial: /Aerial/Source/Models/Sources/SourceInfo.swift:14:16: warning: Inclusive Language Violation: Declaration blacklist contains the term "blacklist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Alamofire: /Example/Source/MasterViewController.swift:28:7: warning: Inclusive Language Violation: Declaration MasterViewController contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Firefox: /Storage/ThirdParty/SwiftData.swift:745:25: warning: Inclusive Language Violation: Declaration sqliteMasterCursor contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Firefox: /Storage/SQL/BrowserSchema.swift:1437:13: warning: Inclusive Language Violation: Declaration sqliteMasterCursor contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Firefox: /Storage/SQL/BrowserSchema.swift:1498:13: warning: Inclusive Language Violation: Declaration sqliteMasterCursor contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Firefox: /Sync/Keys.swift:107:24: warning: Inclusive Language Violation: Declaration init(downloaded:master:) contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Kickstarter: /KsApi/models/CreditCardType.swift:9:8: warning: Inclusive Language Violation: Declaration mastercard contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Kickstarter: /KsApi/models/templates/graphql/GraphCreditCardTemplate.swift:4:21: warning: Inclusive Language Violation: Declaration masterCard contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/App/Networking/NetworkLogger.swift:10:9: warning: Inclusive Language Violation: Declaration whitelist contains the term "whitelist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/App/Networking/NetworkLogger.swift:11:9: warning: Inclusive Language Violation: Declaration blacklist contains the term "blacklist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Sourcery: /SourceryTests/Stub/Performance-Code/Kiosk/App/Networking/NetworkLogger.swift:13:5: warning: Inclusive Language Violation: Declaration init(whitelist:blacklist:) contains the term "blacklist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Swift: /stdlib/public/Platform/TiocConstants.swift:172:12: warning: Inclusive Language Violation: Declaration TIOCPTMASTER contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Swift: /stdlib/private/StdlibUnittest/RaceTest.swift:419:6: warning: Inclusive Language Violation: Declaration masterThreadStopWorkers(:) contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Swift: /stdlib/private/StdlibUnittest/RaceTest.swift:427:6: warning: Inclusive Language Violation: Declaration masterThreadOneTrial(:) contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in Swift: /stdlib/private/StdlibUnittest/RaceTest.swift:610:7: warning: Inclusive Language Violation: Declaration masterThreadBody contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/Models/BlogSettings.swift:86:20: warning: Inclusive Language Violation: Declaration commentsBlacklistKeys contains the term "blacklist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/Models/BlogSettings.swift:99:20: warning: Inclusive Language Violation: Declaration commentsFromKnownUsersWhitelisted contains the term "whitelist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/Models/BlogSettings.swift:234:20: warning: Inclusive Language Violation: Declaration jetpackLoginWhiteListedIPAddresses contains the term "whitelist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/Extensions/URL+Helpers.swift:119:10: warning: Inclusive Language Violation: Declaration appendingHideMasterbarParameters() contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/Extensions/URL+Helpers.swift:180:16: warning: Inclusive Language Violation: Declaration appendingHideMasterbarParameters() contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift:345:13: warning: Inclusive Language Violation: Declaration blacklistKeys contains the term "blacklist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Blog/Site Settings/DiscussionSettingsViewController.swift:344:22: warning: Inclusive Language Violation: Declaration pressedBlacklist(_:) contains the term "blacklist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Blog/Site Settings/JetpackSecuritySettingsViewController.swift:224:17: warning: Inclusive Language Violation: Declaration whiteListedIPs contains the term "whitelist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/ViewRelated/Blog/Site Settings/JetpackSecuritySettingsViewController.swift:222:10: warning: Inclusive Language Violation: Declaration pressedWhitelistedIPAddresses() contains the term "whitelist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/Utility/WebViewControllerConfiguration.swift:9:15: warning: Inclusive Language Violation: Declaration addsHideMasterbarParameters contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/Classes/Utility/WebKitViewController.swift:82:15: warning: Inclusive Language Violation: Declaration addsHideMasterbarParameters contains the term "master" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/WordPressUITests/Screens/BaseScreen.swift:137:9: warning: Inclusive Language Violation: Declaration hasWhiteListedIdentifier contains the term "whitelist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/WordPressUITests/Screens/BaseScreen.swift:138:13: warning: Inclusive Language Violation: Declaration whiteListedIdentifiers contains the term "whitelist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/WordPressTest/BlogSettingsDiscussionTests.swift:25:10: warning: Inclusive Language Violation: Declaration testCommentsAutoapprovalFromKnownUsersEnablesWhitelistedFlag() contains the term "whitelist" which is not considered inclusive. (inclusive_language)
⚠️ This PR introduced a violation in WordPress: /WordPress/WordPressTest/BlogSettingsDiscussionTests.swift:32:10: warning: Inclusive Language Violation: Declaration testCommentsAutoapprovalEverythingDisablesManualModerationAndWhitelistedFlags() contains the term "whitelist" which is not considered inclusive. (inclusive_language)
12 Messages
📖 Linting Aerial with this PR took 1.95s vs 1.94s on master (0% slower)
📖 Linting Alamofire with this PR took 2.73s vs 2.78s on master (1% faster)
📖 Linting Firefox with this PR took 9.33s vs 9.35s on master (0% faster)
📖 Linting Kickstarter with this PR took 15.38s vs 15.62s on master (1% faster)
📖 Linting Moya with this PR took 1.34s vs 1.37s on master (2% faster)
📖 Linting Nimble with this PR took 1.37s vs 1.38s on master (0% faster)
📖 Linting Quick with this PR took 0.65s vs 0.65s on master (0% slower)
📖 Linting Realm with this PR took 4.44s vs 4.46s on master (0% faster)
📖 Linting SourceKitten with this PR took 1.05s vs 1.05s on master (0% slower)
📖 Linting Sourcery with this PR took 7.17s vs 7.2s on master (0% faster)
📖 Linting Swift with this PR took 11.0s vs 11.1s on master (0% faster)
📖 Linting WordPress with this PR took 17.41s vs 17.72s on master (1% faster)

Generated by 🚫 Danger

@codecov-io
Copy link

codecov-io commented Oct 11, 2020

Codecov Report

Merging #3379 into master will increase coverage by 0.03%.
The diff coverage is 60.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3379      +/-   ##
==========================================
+ Coverage   90.48%   90.52%   +0.03%     
==========================================
  Files         417      417              
  Lines       20508    20510       +2     
==========================================
+ Hits        18557    18566       +9     
+ Misses       1951     1944       -7     
Impacted Files Coverage Δ
...iftlint/Extensions/Configuration+CommandLine.swift 0.00% <0.00%> (ø)
...ntFramework/Extensions/Configuration+Merging.swift 92.59% <100.00%> (+1.21%) ⬆️
...LintFrameworkTests/ConfigurationTests+Nested.swift 97.72% <100.00%> (+0.16%) ⬆️
...ftLintFramework/Rules/Lint/PrivateOutletRule.swift 96.15% <0.00%> (-3.85%) ⬇️
...tFramework/Rules/Lint/ValidIBInspectableRule.swift 98.64% <0.00%> (-1.36%) ⬇️
...tyle/MultipleClosuresWithTrailingClosureRule.swift 85.52% <0.00%> (+1.31%) ⬆️
...mework/Rules/Idiomatic/NoFallthroughOnlyRule.swift 100.00% <0.00%> (+1.92%) ⬆️
...ramework/Rules/Style/NoSpaceInMethodCallRule.swift 100.00% <0.00%> (+15.78%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da408b5...32476e4. Read the comment docs.

@sethfri
Copy link
Contributor Author

sethfri commented Oct 11, 2020

@jpsim This should be ready for your input! A couple things:

  1. Are you able to test this against your closed source project to make sure it's the right fix?
  2. Do you have any recs for an automated test for the edge case your project found? It's not immediately obvious to me where to test a passed-in command line argument

@sethfri
Copy link
Contributor Author

sethfri commented Oct 25, 2020

Hey @jpsim, I know it's a crazy time, but since it's been a couple weeks do you think you could take a look at the few PRs I have out? They're starting to encounter merge conflicts, and these bugs are blocking my team from using SwiftLint in our setup.

@sethfri sethfri force-pushed the fixNestedConfigs branch 2 times, most recently from ba053b7 to 02f9267 Compare October 26, 2020 02:29
Copy link
Collaborator

@jpsim jpsim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thanks for the PR! Will merge when CI is green.

@jpsim jpsim merged commit e316bd6 into realm:master Nov 8, 2020
@sethfri sethfri mentioned this pull request Nov 9, 2020
2 tasks
@sethfri sethfri deleted the fixNestedConfigs branch January 7, 2022 10:29
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.

SwiftLint doesn't find a nested configuration when one specific file is specified
4 participants