Skip to content
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"rules": {
// `no-debugger` does not accept options; this should error when an options object is provided
"eslint/no-debugger": ["error", { "some": "option" }]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"plugins": ["jest", "react"],
"categories": {
"correctness": "off"
},
"rules": {
// These rules have `config = Value` in their declare_oxc_lint! macro
// to indicate they accept configuration options, even though they use
// manual from_configuration implementations.
"eslint/no-empty-function": ["error", { "allow": ["functions"] }],
"eslint/no-restricted-imports": ["error", { "paths": ["lodash"] }],
"eslint/no-warning-comments": ["error", { "terms": ["todo", "fixme"] }],
"jest/valid-title": ["error", { "ignoreSpaces": true }],
"react/forbid-dom-props": ["error", { "forbid": ["id"] }]
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// This file is used to test that rules with dummy config declarations
// can accept configuration options without errors.
// The actual content doesn't matter - we're testing config parsing, not linting.
const x = 1;
18 changes: 18 additions & 0 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1362,6 +1362,13 @@ export { redundant };
.test_and_snapshot(&[]);
}

#[test]
fn test_invalid_config_rule_without_config_but_options() {
Tester::new()
.with_cwd("fixtures/invalid_config_rules_without_config".into())
.test_and_snapshot(&[]);
}

#[test]
// Ensure the config validation works with vitest/no-hooks, which
// is an alias of jest/no-hooks.
Expand Down Expand Up @@ -1412,6 +1419,17 @@ export { redundant };
Tester::new().with_cwd("fixtures/valid_complex_config".into()).test_and_snapshot(&[]);
}

/// Test that rules with dummy `config = Value` declarations can accept
/// configuration options without errors. This test should be removed in
/// the future, once these rules have been updated to use proper config
/// structs.
#[test]
fn test_valid_config_rules_with_dummy_config() {
Tester::new()
.with_cwd("fixtures/valid_config_rules_with_dummy_config".into())
.test_and_snapshot(&[]);
}

#[test]
fn test_invalid_config_invalid_config_complex_enum() {
Tester::new()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: apps/oxlint/src/tester.rs
---
##########
arguments:
working directory: fixtures/invalid_config_rules_without_config
----------
Failed to parse oxlint configuration file.

x Invalid configuration for rule `no-debugger`:
| This rule does not accept configuration options.

----------
CLI result: InvalidOptionConfig
----------
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
---
source: apps/oxlint/src/tester.rs
---
##########
arguments:
working directory: fixtures/valid_config_rules_with_dummy_config
----------
Found 0 warnings and 0 errors.
Finished in <variable>ms on 1 file with 5 rules using 1 threads.
----------
CLI result: LintSucceeded
----------
88 changes: 88 additions & 0 deletions crates/oxc_linter/src/config/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,18 @@ impl OxlintRules {
.find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
});
if let Some(rule) = rule {
// If the user provided a non-empty options array for a rule that does not
// declare a `config =` type in its declaration, treat this as an invalid
// configuration and report an error.
if !rule_config.config.is_empty() && !rule.has_config() {
errors.push(OverrideRulesError::RuleConfiguration {
rule_name: rule_config.full_name().into_owned(),
message: "This rule does not accept configuration options."
.to_string(),
});
continue;
}

// Configs are stored as `SmallVec<[Value; 1]>`, but `from_configuration` expects
// a single `Value` with `Value::Null` being the equivalent of empty config
let config = if rule_config.config.is_empty() {
Expand Down Expand Up @@ -747,6 +759,35 @@ mod test {
}
}

#[test]
fn test_override_rules_errors_for_rules_without_config() {
let rules_config = OxlintRules::deserialize(&json!({
"eslint/no-debugger": ["error", { "some": "option" }]
}))
.unwrap();

let mut builtin_rules = RuleSet::default();
let mut external_rules = FxHashMap::default();
let mut store = ExternalPluginStore::default();

match rules_config.override_rules(
&mut builtin_rules,
&mut external_rules,
&RULES,
&mut store,
) {
Err(errors) => {
assert!(errors.len() == 1, "expected one error, got {errors:#?}");
assert!(matches!(
&errors[0],
super::OverrideRulesError::RuleConfiguration { rule_name, .. }
if rule_name == "eslint/no-debugger" || rule_name == "no-debugger"
));
}
Ok(()) => panic!("expected errors from invalid config"),
}
}

#[test]
fn test_override_rules_errors_sorted() {
let rules_config = OxlintRules::deserialize(&json!({
Expand All @@ -773,4 +814,51 @@ mod test {
Ok(()) => panic!("expected errors from invalid configs"),
}
}

/// Test that rules with dummy `config = Value` declarations don't error
/// when configuration options are passed to them. These rules have manual
/// `from_configuration` implementations but need `config =` in their
/// `declare_oxc_lint!` macro to pass the `has_config()` check.
#[test]
fn test_rules_with_dummy_config_accept_options() {
let rules_config = OxlintRules::deserialize(&json!({
"eslint/no-empty-function": ["error", { "allow": ["functions"] }],
"eslint/no-restricted-imports": ["error", { "paths": ["lodash"] }],
"eslint/no-warning-comments": ["error", { "terms": ["todo", "fixme"] }],
"jest/valid-title": ["error", { "ignoreSpaces": true }],
"react/forbid-dom-props": ["error", { "forbid": ["id"] }]
}))
.unwrap();

let mut builtin_rules = RuleSet::default();
let mut external_rules = FxHashMap::default();
let mut store = ExternalPluginStore::default();

// These rules should accept configuration without errors
rules_config
.override_rules(&mut builtin_rules, &mut external_rules, &RULES, &mut store)
.expect("rules with dummy config should accept configuration options");

// Verify the rules were actually added
assert!(
builtin_rules.iter().any(|(r, _)| r.name() == "no-empty-function"),
"no-empty-function should be in the rule set"
);
assert!(
builtin_rules.iter().any(|(r, _)| r.name() == "no-restricted-imports"),
"no-restricted-imports should be in the rule set"
);
assert!(
builtin_rules.iter().any(|(r, _)| r.name() == "no-warning-comments"),
"no-warning-comments should be in the rule set"
);
assert!(
builtin_rules.iter().any(|(r, _)| r.name() == "valid-title"),
"valid-title should be in the rule set"
);
assert!(
builtin_rules.iter().any(|(r, _)| r.name() == "forbid-dom-props"),
"forbid-dom-props should be in the rule set"
);
}
}
Loading
Loading