Skip to content

Fix false positives in inclusive_language rule#3422

Closed
daltonclaybrook wants to merge 2 commits intomasterfrom
dalton/bugfix-3415
Closed

Fix false positives in inclusive_language rule#3422
daltonclaybrook wants to merge 2 commits intomasterfrom
dalton/bugfix-3415

Conversation

@daltonclaybrook
Copy link
Copy Markdown
Contributor

This PR resolves #3415

A noteworthy implication of this PR is that strings like BlackList and whiteList no longer trigger violations because these use camel case and are thus treated as separate terms.

@SwiftLintBot
Copy link
Copy Markdown

1 Warning
⚠️ 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)
29 Messages
📖 Linting Aerial with this PR took 1.99s vs 1.92s on master (3% slower)
📖 Linting Alamofire with this PR took 2.81s vs 2.81s on master (0% slower)
📖 Linting Firefox with this PR took 9.51s vs 9.35s on master (1% slower)
📖 Linting Kickstarter with this PR took 15.93s vs 15.54s on master (2% slower)
📖 Linting Moya with this PR took 1.55s vs 1.41s on master (9% slower)
📖 Linting Nimble with this PR took 1.35s vs 1.39s on master (2% faster)
📖 Linting Quick with this PR took 0.64s vs 0.64s on master (0% slower)
📖 Linting Realm with this PR took 4.64s vs 4.54s on master (2% slower)
📖 Linting SourceKitten with this PR took 1.09s vs 1.06s on master (2% slower)
📖 Linting Sourcery with this PR took 7.28s vs 7.33s on master (0% faster)
📖 Linting Swift with this PR took 11.27s vs 11.14s on master (1% slower)
📖 Linting WordPress with this PR took 17.75s vs 17.65s on master (0% slower)
📖 This PR fixed 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 fixed 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 "whitelist" which is not considered inclusive. (inclusive_language)
📖 This PR fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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 fixed 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)

Generated by 🚫 Danger

@jpsim
Copy link
Copy Markdown
Collaborator

jpsim commented Nov 10, 2020

According to OSSCheck’s results posted above, it also seems like “Whitelisted” no longer trigger violations. Should that be added to the set of default terms?

@daltonclaybrook
Copy link
Copy Markdown
Contributor Author

daltonclaybrook commented Nov 10, 2020

@jpsim Should we add "Whitelisting" and "Whitelists" as well? Is there possibly another solution we haven't thought of?

@jpsim
Copy link
Copy Markdown
Collaborator

jpsim commented Nov 10, 2020

What if you made the terms regex patterns instead, like the way custom rules work? That way you could do [Ww]hitelist[\w]+ and cover everything you want, but just [Mm]aster. Still using the tokenization approach of splitting identifiers by camelCase though.

Regular expressions would also be more useful when configuring the rule.

Or we stick with adding a few permutations of the words ad-hoc.

@daltonclaybrook
Copy link
Copy Markdown
Contributor Author

I like the regex approach. I will tinker with this.

@daltonclaybrook
Copy link
Copy Markdown
Contributor Author

Closing this PR in favor of #3439

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.

inclusive_language false positives: Triggering terms as substrings of identifiers

3 participants