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-console-typo": "error",
"yikes": "error"
}
}
53 changes: 52 additions & 1 deletion apps/oxlint/src/config_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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() {
Expand Down
7 changes: 7 additions & 0 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(&[]);
Expand Down
Original file line number Diff line number Diff line change
@@ -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
----------
Original file line number Diff line number Diff line change
@@ -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
```
```
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import { defineConfig } from "#oxlint";

export default defineConfig({
rules: {
"no-console-typo": "error",
},
});
63 changes: 62 additions & 1 deletion crates/oxc_linter/src/config/config_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
101 changes: 71 additions & 30 deletions crates/oxc_linter/src/config/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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}")
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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::<Vec<_>>();

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::<Vec<_>>();

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;
Expand Down
Loading