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(linter): can't disable no-nested-ternary rule anymore #8600

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
1 change: 0 additions & 1 deletion apps/oxlint/fixtures/typescript_eslint/eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
"plugins": ["@typescript-eslint"],
"rules": {
"no-loss-of-precision": "off",
"@typescript-eslint/no-loss-of-precision": "error",
"@typescript-eslint/no-namespace": "warn"
}
}
2 changes: 1 addition & 1 deletion apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ mod test {
];
let result = test(args);
assert_eq!(result.number_of_files, 1);
assert_eq!(result.number_of_warnings, 3);
assert_eq!(result.number_of_warnings, 2);
assert_eq!(result.number_of_errors, 0);
}

Expand Down
59 changes: 31 additions & 28 deletions crates/oxc_linter/src/config/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,28 +110,32 @@ impl OxlintRules {
}
}
_ => {
// For overlapping rule names, use the "error" one
// "no-loss-of-precision": "off",
// "@typescript-eslint/no-loss-of-precision": "error"
if let Some(rule_config) =
rule_configs.iter().find(|r| r.severity.is_warn_deny())
{
let config = rule_config.config.clone().unwrap_or_default();

if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
rules_to_replace
.push(RuleWithSeverity::new(rule.read_json(config), rule.severity));
}
// If the given rule is not found in the rule list (for example, if all rules are disabled),
// then look it up in the entire rules list and add it.
else if let Some(rule) = all_rules.iter().find(|r| r.name() == *name) {
rules_to_replace.push(RuleWithSeverity::new(
rule.read_json(config),
rule_config.severity,
));
}
} else if rule_configs.iter().all(|r| r.severity.is_allow()) {
if let Some(rule) = rules_for_override.iter().find(|r| r.name() == *name) {
let rules = rules_for_override
.iter()
.filter(|r| r.name() == *name)
.map(|r| (r.plugin_name(), r))
.collect::<FxHashMap<_, _>>();

for rule_config in rule_configs {
if rule_config.severity.is_warn_deny() {
let config = rule_config.config.clone().unwrap_or_default();
if let Some(rule) = rules.get(&rule_config.plugin_name.as_str()) {
rules_to_replace.push(RuleWithSeverity::new(
rule.read_json(config),
rule.severity,
));
}
// If the given rule is not found in the rule list (for example, if all rules are disabled),
// then look it up in the entire rules list and add it.
else if let Some(rule) = all_rules.iter().find(|r| {
r.name() == *name && r.plugin_name() == rule_config.plugin_name
}) {
rules_to_replace.push(RuleWithSeverity::new(
rule.read_json(config),
rule_config.severity,
));
}
} else if let Some(&rule) = rules.get(&rule_config.plugin_name.as_str()) {
rules_to_remove.push(rule.clone());
}
}
Expand Down Expand Up @@ -465,9 +469,8 @@ mod test {
fn test_override_plugin_prefix_duplicates() {
let configs = [
// FIXME: this should be valid
// json!({ "@typescript-eslint/no-unused-vars": "error" }),
json!({ "no-unused-vars": "off", "typescript/no-unused-vars": "error" }),
json!({ "no-unused-vars": "off", "@typescript-eslint/no-unused-vars": "error" }),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@typescript-eslint/no-unused-vars does not exist.

Copy link
Collaborator

Choose a reason for hiding this comment

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

here is the rule: https://typescript-eslint.io/rules/no-unused-vars/

Internally we are remapping some eslint rules to typescript here:

// List of Eslint rules that have Typescript equivalents.
const TYPESCRIPT_COMPATIBLE_ESLINT_RULES: phf::Set<&'static str> = phf::phf_set! {
"class-methods-use-this",
"default-param-last",
"init-declarations",
"max-params",
"no-array-constructor",
"no-dupe-class-members",
"no-empty-function",
"no-invalid-this",
"no-loop-func",
"no-loss-of-precision",
"no-magic-numbers",
"no-redeclare",
"no-restricted-imports",
"no-shadow",
"no-unused-expressions",
"no-unused-vars",
"no-use-before-define",
"no-useless-constructor",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Internally we are remapping some eslint rules to typescript here:

Got it.

// json!({ "@unicorn-eslint/no-nested-ternary": "error" }),
json!({ "no-nested-ternary": "off", "unicorn/no-nested-ternary": "error" }),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"no-nested-ternary" => "eslint/no-nested-ternary"

Currently, we do not support rules without an explicit plugin_name, which would automatically match all rules:
"no-nested-ternary" => "eslint/no-nested-ternary" and "unicorn/no-nested-ternary"

];

for config in &configs {
Expand All @@ -476,21 +479,21 @@ mod test {

assert_eq!(rules.len(), 1, "{config:?}");
let rule = rules.iter().next().unwrap();
assert_eq!(rule.name(), "no-unused-vars", "{config:?}");
assert_eq!(rule.name(), "no-nested-ternary", "{config:?}");
assert_eq!(rule.severity, AllowWarnDeny::Deny, "{config:?}");
}

for config in &configs {
let mut rules = RuleSet::default();
rules.insert(RuleWithSeverity {
rule: RuleEnum::EslintNoUnusedVars(Default::default()),
rule: RuleEnum::UnicornNoNestedTernary(Default::default()),
severity: AllowWarnDeny::Warn,
});
r#override(&mut rules, config);

assert_eq!(rules.len(), 1, "{config:?}");
let rule = rules.iter().next().unwrap();
assert_eq!(rule.name(), "no-unused-vars", "{config:?}");
assert_eq!(rule.name(), "no-nested-ternary", "{config:?}");
assert_eq!(rule.severity, AllowWarnDeny::Warn, "{config:?}");
}
}
Expand Down
Loading