馃彈 Ensure that forbidden term allowlists are updated #33490
Conversation
|
Hey @erwinmombay, @jridgewell! These files were changed: |
| // Terms are defined in this file. | ||
| /* eslint-disable local/no-forbidden-terms */ | ||
|
|
||
| // Ensures that file is up-to-date. | ||
| /* eslint "local/forbidden-terms-config": 2 */ | ||
|
|
There was a problem hiding this comment.
For maintainability, would it make sense to move these two rules to a new file build-system/test-configs/.eslintrc.js?
There was a problem hiding this comment.
Sure, moved. It also happens to speed up linting. File-level rules seem to only ignore reports, but still run.
TIMING=1 npx eslint build-system/test-configs/forbidden-terms.js Rule | Time (ms)
:-----------------------------|----------:
prettier/prettier | 248.790
local/forbidden-terms-config | 34.043
- local/no-forbidden-terms | 30.997
+ jsdoc/check-types | 16.759 | @@ -1235,8 +1103,6 @@ const forbiddenTermsSrcInclusive = { | |||
| 'extensions/amp-ad-network-adsense-impl/0.1/amp-ad-network-adsense-impl.js', | |||
| 'extensions/amp-iframe/0.1/amp-iframe.js', | |||
| 'extensions/amp-script/0.1/amp-script.js', | |||
| 'extensions/amp-sidebar/0.1/amp-sidebar.js', | |||
| 'extensions/amp-sidebar/0.2/amp-sidebar.js', | |||
There was a problem hiding this comment.
Bravo! I don't think this kind of a cleanup would've been possible (or would've ever happened) earlier.
There was a problem hiding this comment.
Meta suggestion: Today, all the error messages from local/no-forbidden-terms are split across two lines, which breaks the flow of lint error reporting. Would it make sense / be possible to make them mostly single-line (except for ones that are really too long)?
| } | ||
| } | ||
|
|
||
| function* removeFromArray(fixer, node) { |
There was a problem hiding this comment.
I think you can just fixer.remove(node) and let eslint automatically fix formatting?
There was a problem hiding this comment.
Unfortunately that won't remove the trailing comma nor the whitespace.
I also want to remove comments on the same line, since context for those is lost.
Adds a lint rule `local/forbidden-terms-config` that's used on one file: - Term keys should be string literals. - Allowlist items should be string literals. - Allowed files should contain the term. - `allowlist` property should be removed if empty.
Adds a lint rule
local/forbidden-terms-configthat's used on one file:allowlistproperty should be removed if empty.