diff --git a/apps/oxlint/fixtures/cli/invalid_config_missing_builtin_rule/.oxlintrc.json b/apps/oxlint/fixtures/cli/invalid_config_missing_builtin_rule/.oxlintrc.json new file mode 100644 index 0000000000000..e6eaa446c57dc --- /dev/null +++ b/apps/oxlint/fixtures/cli/invalid_config_missing_builtin_rule/.oxlintrc.json @@ -0,0 +1,6 @@ +{ + "rules": { + "no-console-typo": "error", + "yikes": "error" + } +} diff --git a/apps/oxlint/src/config_loader.rs b/apps/oxlint/src/config_loader.rs index f42e67b1fc992..f5be85b2f91b2 100644 --- a/apps/oxlint/src/config_loader.rs +++ b/apps/oxlint/src/config_loader.rs @@ -781,7 +781,7 @@ fn nested_report_unused_disable_directives_not_supported(path: &Path) -> OxcDiag mod test { use std::path::{Path, PathBuf}; - use oxc_linter::ExternalPluginStore; + use oxc_linter::{ConfigStoreBuilder, ExternalPluginStore}; use super::{ConfigLoadError, ConfigLoader, DiscoveredConfig, is_js_config_path}; #[cfg(feature = "napi")] @@ -815,6 +815,19 @@ mod test { JsConfigResult { path, config: Some(config) } } + #[cfg(feature = "napi")] + fn make_js_config_with_rules(path: PathBuf, rules: &serde_json::Value) -> JsConfigResult { + let mut config: oxc_linter::Oxlintrc = serde_json::from_value(serde_json::json!({ + "rules": rules + })) + .unwrap(); + config.path = path.clone(); + if let Some(config_dir) = path.parent() { + config.set_config_dir(config_dir); + } + JsConfigResult { path, config: Some(config) } + } + #[test] fn test_config_path_with_parent_references() { let cwd = std::env::current_dir().unwrap(); @@ -1011,6 +1024,44 @@ mod test { assert_eq!(config.options.type_check, Some(true)); } + #[cfg(feature = "napi")] + #[test] + fn test_root_oxlint_config_ts_rejects_missing_builtin_rule() { + let root_dir = tempfile::tempdir().unwrap(); + let root_path = root_dir.path().join("oxlint.config.ts"); + std::fs::write(&root_path, "export default {};").unwrap(); + + let mut external_plugin_store = ExternalPluginStore::new(false); + let js_loader = make_js_loader({ + move |paths| { + assert_eq!(paths, vec![root_path.to_string_lossy().to_string()]); + Ok(vec![make_js_config_with_rules( + root_path.clone(), + &serde_json::json!({ "no-console-typo": "error" }), + )]) + } + }); + + let oxlintrc = { + let loader = ConfigLoader::new(None, &mut external_plugin_store, &[], None); + let loader = loader.with_js_config_loader(Some(&js_loader)); + loader + .load_root_config(root_dir.path(), Some(&PathBuf::from("oxlint.config.ts"))) + .unwrap() + }; + + let err = ConfigStoreBuilder::from_oxlintrc( + false, + oxlintrc, + None, + &mut external_plugin_store, + None, + ) + .unwrap_err(); + + assert_eq!(err.to_string(), "Rule 'no-console-typo' not found in plugin 'eslint'"); + } + #[cfg(feature = "napi")] #[test] fn test_nested_oxlint_config_ts_rejects_type_aware() { diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index a8d453b9c55f2..de27765b18424 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -1535,6 +1535,13 @@ export { redundant }; .test_and_snapshot(&[]); } + #[test] + fn test_invalid_config_missing_builtin_rule() { + Tester::new() + .with_cwd("fixtures/cli/invalid_config_missing_builtin_rule".into()) + .test_and_snapshot(&[]); + } + #[test] fn test_invalid_config_nested() { Tester::new().with_cwd("fixtures/cli/invalid_config_nested".into()).test_and_snapshot(&[]); diff --git a/apps/oxlint/src/snapshots/fixtures__cli__invalid_config_missing_builtin_rule_@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__cli__invalid_config_missing_builtin_rule_@oxlint.snap new file mode 100644 index 0000000000000..a4d6e028a63ad --- /dev/null +++ b/apps/oxlint/src/snapshots/fixtures__cli__invalid_config_missing_builtin_rule_@oxlint.snap @@ -0,0 +1,16 @@ +--- +source: apps/oxlint/src/tester.rs +--- +########## +arguments: +working directory: fixtures/cli/invalid_config_missing_builtin_rule +---------- +Failed to parse oxlint configuration file. + + x Rule 'no-console-typo' not found in plugin 'eslint' + | + | Rule 'yikes' not found in plugin 'eslint' + +---------- +CLI result: InvalidOptionConfig +---------- diff --git a/apps/oxlint/test/fixtures/js_config_dynamic_missing_builtin_rule/output.snap.md b/apps/oxlint/test/fixtures/js_config_dynamic_missing_builtin_rule/output.snap.md new file mode 100644 index 0000000000000..b2e9817529cbd --- /dev/null +++ b/apps/oxlint/test/fixtures/js_config_dynamic_missing_builtin_rule/output.snap.md @@ -0,0 +1,13 @@ +# Exit code +1 + +# stdout +``` +Failed to parse oxlint configuration file. + + x Rule 'no-console-typo' not found in plugin 'eslint' +``` + +# stderr +``` +``` diff --git a/apps/oxlint/test/fixtures/js_config_dynamic_missing_builtin_rule/oxlint.config.ts b/apps/oxlint/test/fixtures/js_config_dynamic_missing_builtin_rule/oxlint.config.ts new file mode 100644 index 0000000000000..335d9136069f0 --- /dev/null +++ b/apps/oxlint/test/fixtures/js_config_dynamic_missing_builtin_rule/oxlint.config.ts @@ -0,0 +1,7 @@ +import { defineConfig } from "#oxlint"; + +export default defineConfig({ + rules: { + "no-console-typo": "error", + }, +}); diff --git a/crates/oxc_linter/src/config/config_builder.rs b/crates/oxc_linter/src/config/config_builder.rs index 4fb42ab7d023f..3173aa4441cdb 100644 --- a/crates/oxc_linter/src/config/config_builder.rs +++ b/crates/oxc_linter/src/config/config_builder.rs @@ -462,8 +462,11 @@ impl ConfigStoreBuilder { } /// Builds a [`Config`] from the current state of the builder. + /// /// # Errors - /// Returns [`ConfigBuilderError::UnknownRules`] if there are rules that could not be matched. + /// + /// Returns [`ConfigBuilderError`] if the configured rules or overrides + /// cannot be resolved. pub fn build( mut self, external_plugin_store: &mut ExternalPluginStore, @@ -1538,6 +1541,64 @@ mod test { ); } + #[test] + fn test_unknown_builtin_rule_errors_in_root_config() { + let oxlintrc: Oxlintrc = serde_json::from_str( + r#" + { + "rules": { + "no-console-typo": "error" + } + } + "#, + ) + .unwrap(); + + let mut external_plugin_store = ExternalPluginStore::default(); + let err = ConfigStoreBuilder::from_oxlintrc( + true, + oxlintrc, + None, + &mut external_plugin_store, + None, + ) + .unwrap_err(); + + assert_eq!(err.to_string(), "Rule 'no-console-typo' not found in plugin 'eslint'"); + } + + #[test] + fn test_unknown_builtin_rule_errors_in_overrides() { + let oxlintrc: Oxlintrc = serde_json::from_str( + r#" + { + "overrides": [ + { + "files": ["*.js"], + "rules": { + "no-console-typo": "error" + } + } + ] + } + "#, + ) + .unwrap(); + + let mut external_plugin_store = ExternalPluginStore::default(); + let builder = ConfigStoreBuilder::from_oxlintrc( + true, + oxlintrc, + None, + &mut external_plugin_store, + None, + ) + .unwrap(); + let err = builder.build(&mut external_plugin_store).unwrap_err(); + + assert_eq!(err.to_string(), "Rule 'no-console-typo' not found in plugin 'eslint'"); + } + fn config_store_from_path(path: &str) -> Config { let mut external_plugin_store = ExternalPluginStore::default(); ConfigStoreBuilder::from_oxlintrc( diff --git a/crates/oxc_linter/src/config/rules.rs b/crates/oxc_linter/src/config/rules.rs index 8bba05d40bb2e..9e776767ecf73 100644 --- a/crates/oxc_linter/src/config/rules.rs +++ b/crates/oxc_linter/src/config/rules.rs @@ -26,6 +26,13 @@ use crate::{ /// Errors that can occur when overriding rules #[derive(Debug, Clone, PartialEq, Eq)] pub enum OverrideRulesError { + /// Error looking up a builtin rule + RuleNotFound { + /// The plugin the rule belongs to + plugin_name: String, + /// The missing rule name + rule_name: String, + }, /// Error looking up an external rule ExternalRuleLookup(ExternalRuleLookupError), /// Error parsing rule configuration @@ -40,6 +47,9 @@ pub enum OverrideRulesError { impl fmt::Display for OverrideRulesError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + OverrideRulesError::RuleNotFound { plugin_name, rule_name } => { + write!(f, "Rule '{rule_name}' not found in plugin '{plugin_name}'") + } OverrideRulesError::ExternalRuleLookup(e) => write!(f, "{e}"), OverrideRulesError::RuleConfiguration { rule_name, message } => { write!(f, "Invalid configuration for rule `{rule_name}`:\n {message}") @@ -165,6 +175,18 @@ impl OxlintRules { }); } } + } else if RULES + .iter() + .any(|rule| rule.name() == rule_name && rule.plugin_name() == plugin_name) + { + // Known builtin rule, but unavailable in this config because its plugin + // is disabled. Preserve the historical behavior of ignoring it rather than + // treating the config as invalid. + } else { + errors.push(OverrideRulesError::RuleNotFound { + plugin_name: plugin_name.to_string(), + rule_name: rule_name.to_string(), + }); } } else { // If JS plugins are disabled (language server), assume plugin name refers to a JS plugin, @@ -569,36 +591,6 @@ mod test { } } - // FIXME - #[test] - #[should_panic( - expected = "eslint rules should be configurable by their typescript-eslint reimplementations:" - )] - fn test_override_empty_fixme() { - let config = json!({ "@typescript-eslint/no-console": "error" }); - let mut rules = RuleSet::default(); - - rules.clear(); - r#override(&mut rules, &config); - - assert_eq!( - rules.len(), - 1, - "eslint rules should be configurable by their typescript-eslint reimplementations: {config:?}" - ); - let (rule, severity) = rules.iter().next().unwrap(); - assert_eq!( - rule.name(), - "no-console", - "eslint rules should be configurable by their typescript-eslint reimplementations: {config:?}" - ); - assert_eq!( - severity, - &AllowWarnDeny::Deny, - "eslint rules should be configurable by their typescript-eslint reimplementations: {config:?}" - ); - } - #[test] fn test_override_allow() { let mut rules = RuleSet::default(); @@ -638,6 +630,55 @@ mod test { } } + #[test] + fn test_override_ignores_known_rule_when_plugin_disabled() { + let rules_config = + OxlintRules::deserialize(&json!({ "@typescript-eslint/no-namespace": "warn" })) + .unwrap(); + let mut rules = RuleSet::default(); + let mut external_rules_for_override = FxHashMap::default(); + let mut external_linter_store = ExternalPluginStore::default(); + let all_rules = RULES + .iter() + .filter(|rule| rule.plugin_name() != "typescript") + .cloned() + .collect::>(); + + let result = rules_config.override_rules( + &mut rules, + &mut external_rules_for_override, + &all_rules, + &mut external_linter_store, + ); + + assert!(result.is_ok()); + assert!(rules.is_empty()); + } + + #[test] + fn test_override_ignores_known_aliased_rule_when_plugin_disabled() { + let rules_config = + OxlintRules::deserialize(&json!({ "vitest/no-disabled-tests": "error" })).unwrap(); + let mut rules = RuleSet::default(); + let mut external_rules_for_override = FxHashMap::default(); + let mut external_linter_store = ExternalPluginStore::default(); + let all_rules = RULES + .iter() + .filter(|rule| rule.plugin_name() != "jest" && rule.plugin_name() != "vitest") + .cloned() + .collect::>(); + + let result = rules_config.override_rules( + &mut rules, + &mut external_rules_for_override, + &all_rules, + &mut external_linter_store, + ); + + assert!(result.is_ok()); + assert!(rules.is_empty()); + } + #[test] fn test_normalize_plugin_name_in_rules() { use super::super::plugins::normalize_plugin_name;