Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Fix behavior of jsRules:true to include explicitly disabled rules. #4517

Merged
merged 1 commit into from
Feb 23, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/usage/configuration/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ configure which rules get run and each of their options.
- Legacy: A boolean value may be specified instead of the above object, and is equivalent to setting no options with default severity.
- Any rules specified in this block will override those configured in any base configuration being extended.
- [Check out the full rules list here][3].
- `jsRules?: any | boolean`: Same format as `rules` or explicit `true` to copy all valid active rules from rules. These rules are applied to `.js` and `.jsx` files.
- `jsRules?: any | boolean`: Same format as `rules` or explicit `true` to copy all rule configurations for JS-compatible rules from `rules`. These rules are applied to `.js` and `.jsx` files.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documentation is clarified

- `defaultSeverity?: "error" | "warning" | "off"`: The severity level that is applied to rules in this config file as well as rules in any inherited config files which have their severity set to "default". If undefined, "error" is used as the defaultSeverity.
- `linterOptions?: { exclude?: string[] }`:
- `exclude: string[]`: An array of globs. Any file matching these globs will not be linted. All exclude patterns are relative to the configuration file they were specified in.
Expand Down
14 changes: 6 additions & 8 deletions src/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -592,14 +592,12 @@ export function parseConfigFile(
const validJsRules = new Map<string, Partial<IOptions>>();
if (copyRulestoJsRules) {
rules.forEach((ruleOptions, ruleName) => {
if (ruleOptions.ruleSeverity !== "off") {
const Rule = findRule(ruleName, rulesDirectory);
if (
Rule !== undefined &&
(Rule.metadata === undefined || !Rule.metadata.typescriptOnly)
) {
validJsRules.set(ruleName, ruleOptions);
}
const Rule = findRule(ruleName, rulesDirectory);
if (
Rule !== undefined &&
(Rule.metadata === undefined || !Rule.metadata.typescriptOnly)
) {
validJsRules.set(ruleName, ruleOptions);
}
});
}
Expand Down
5 changes: 4 additions & 1 deletion test/configurationTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ describe("Configuration", () => {
rawConfig = {
jsRules: true,
rules: {
// valid rule for JS
eofline: true,
},
};
Expand All @@ -151,7 +152,9 @@ describe("Configuration", () => {
rawConfig = {
jsRules: true,
rules: {
eofline: true,
// valid rule for JS, disabled (should be copied over)
eofline: false,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tweaked this existing test case to cover the new behavior of keeping disabled rules

// non-valid rule for JS (should NOT be copied over)
typedef: true,
},
};
Expand Down