diff --git a/apps/oxlint/fixtures/invalid_config_sort_imports/.oxlintrc.json b/apps/oxlint/fixtures/invalid_config_sort_imports/.oxlintrc.json new file mode 100644 index 0000000000000..c21af33fbafdd --- /dev/null +++ b/apps/oxlint/fixtures/invalid_config_sort_imports/.oxlintrc.json @@ -0,0 +1,10 @@ +{ + "plugins": ["eslint"], + "categories": { + "correctness": "off" + }, + "rules": { + // Invalid config, memberSyntaxSortOrder must have all 4 values. + "eslint/sort-imports": ["error", { "memberSyntaxSortOrder": ["none", "all", "multiple"] }] + } +} diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index bc5c7177e3680..51f7c0eef509a 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -1518,6 +1518,13 @@ export { redundant }; Tester::new().with_cwd("fixtures/extends_invalid_config".into()).test_and_snapshot(&[]); } + #[test] + fn test_invalid_config_invalid_config_sort_imports() { + Tester::new() + .with_cwd("fixtures/invalid_config_sort_imports".into()) + .test_and_snapshot(&[]); + } + #[test] fn test_valid_complex_config() { Tester::new().with_cwd("fixtures/valid_complex_config".into()).test_and_snapshot(&[]); diff --git a/apps/oxlint/src/snapshots/fixtures__invalid_config_sort_imports_@oxlint.snap b/apps/oxlint/src/snapshots/fixtures__invalid_config_sort_imports_@oxlint.snap new file mode 100644 index 0000000000000..199bd553d87eb --- /dev/null +++ b/apps/oxlint/src/snapshots/fixtures__invalid_config_sort_imports_@oxlint.snap @@ -0,0 +1,16 @@ +--- +source: apps/oxlint/src/tester.rs +--- +########## +arguments: +working directory: fixtures/invalid_config_sort_imports +---------- +Failed to parse oxlint configuration file. + + x Invalid configuration for rule `sort-imports`: + | memberSyntaxSortOrder must contain exactly 4 kinds, got 3 + | received config: `{ "memberSyntaxSortOrder": [ "none", "all", "multiple" ] }` + +---------- +CLI result: InvalidOptionConfig +---------- diff --git a/crates/oxc_linter/src/rules/eslint/sort_imports.rs b/crates/oxc_linter/src/rules/eslint/sort_imports.rs index f7c59ec947faa..358e18647bea7 100644 --- a/crates/oxc_linter/src/rules/eslint/sort_imports.rs +++ b/crates/oxc_linter/src/rules/eslint/sort_imports.rs @@ -1,7 +1,6 @@ use std::{ borrow::Cow, fmt::{Display, Write}, - str::FromStr, }; use cow_utils::CowUtils; @@ -10,10 +9,14 @@ use oxc_ast::ast::{ImportDeclaration, ImportDeclarationSpecifier, Statement}; use oxc_diagnostics::OxcDiagnostic; use oxc_macros::declare_oxc_lint; use oxc_span::Span; +use rustc_hash::FxHashSet; use schemars::JsonSchema; -use serde::Serialize; +use serde::{Deserialize, Serialize}; -use crate::{context::LintContext, rule::Rule}; +use crate::{ + context::LintContext, + rule::{DefaultRuleConfig, Rule}, +}; fn unexpected_syntax_order_diagnostic( curr_kind: &ImportKind, @@ -35,11 +38,11 @@ fn sort_members_alphabetically_diagnostic(name: &str, span: Span) -> OxcDiagnost .with_label(span) } -#[derive(Debug, Default, Clone)] +#[derive(Debug, Default, Clone, Deserialize)] pub struct SortImports(Box); -#[derive(Debug, Default, Clone, JsonSchema)] -#[serde(rename_all = "camelCase", default)] +#[derive(Debug, Default, Clone, JsonSchema, Deserialize)] +#[serde(rename_all = "camelCase", default, deny_unknown_fields)] pub struct SortImportsOptions { /// When `true`, the rule ignores case-sensitivity when sorting import names. ignore_case: bool, @@ -50,7 +53,7 @@ pub struct SortImportsOptions { /// When `true`, the rule allows import groups separated by blank lines to be treated independently. allow_separated_groups: bool, /// Specifies the sort order of different import syntaxes. - /// Must include all 4 kinds or else this will fall back to default. + /// Must include all 4 kinds! member_syntax_sort_order: MemberSyntaxSortOrder, } @@ -79,7 +82,7 @@ declare_oxc_lint!( /// /// Examples of **incorrect** code for this rule: /// ```javascript - /// import {b, a, c} from 'foo.js' + /// import { b, a, c } from 'foo.js' /// /// import d from 'foo.js'; /// import e from 'bar.js'; @@ -93,56 +96,7 @@ declare_oxc_lint!( impl Rule for SortImports { fn from_configuration(value: serde_json::Value) -> Result { - let Some(config) = value.get(0) else { - return Ok(Self(Box::default())); - }; - - let ignore_case = - config.get("ignoreCase").and_then(serde_json::Value::as_bool).unwrap_or_default(); - let ignore_member_sort = - config.get("ignoreMemberSort").and_then(serde_json::Value::as_bool).unwrap_or_default(); - let ignore_declaration_sort = config - .get("ignoreDeclarationSort") - .and_then(serde_json::Value::as_bool) - .unwrap_or_default(); - let allow_separated_groups = config - .get("allowSeparatedGroups") - .and_then(serde_json::Value::as_bool) - .unwrap_or_default(); - - let member_syntax_sort_order = config - .get("memberSyntaxSortOrder") - .and_then(|v| v.as_array()) - .map(|arr| { - // memberSyntaxSortOrder in config file must have 4 items - if arr.len() != 4 { - return MemberSyntaxSortOrder::default(); - } - - let kinds: Vec = arr - .iter() - .filter_map(|v| v.as_str()) - .map(ImportKind::from_str) - .filter_map(Result::ok) - .unique() - .collect(); - - // 4 items must all unique and valid. - if kinds.len() != 4 { - return MemberSyntaxSortOrder::default(); - } - - MemberSyntaxSortOrder(kinds) - }) - .unwrap_or_default(); - - Ok(Self(Box::new(SortImportsOptions { - ignore_case, - ignore_declaration_sort, - ignore_member_sort, - allow_separated_groups, - member_syntax_sort_order, - }))) + serde_json::from_value::>(value).map(DefaultRuleConfig::into_inner) } fn run_once(&self, ctx: &LintContext) { @@ -367,6 +321,29 @@ impl SortImports { #[schemars(transparent)] struct MemberSyntaxSortOrder(Vec); +impl<'de> serde::Deserialize<'de> for MemberSyntaxSortOrder { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + let v = Vec::::deserialize(deserializer)?; + if v.len() != 4 { + return Err(serde::de::Error::custom(format!( + "memberSyntaxSortOrder must contain exactly 4 kinds, got {}", + v.len() + ))); + } + // Ensure the values are unique. + let set: FxHashSet<_> = v.iter().collect(); + if set.len() != 4 { + return Err(serde::de::Error::custom( + "memberSyntaxSortOrder must contain 4 unique kinds, but duplicates were found", + )); + } + Ok(MemberSyntaxSortOrder(v)) + } +} + impl Default for MemberSyntaxSortOrder { fn default() -> Self { MemberSyntaxSortOrder(vec![ @@ -417,7 +394,7 @@ impl MemberSyntaxSortOrder { } } -#[derive(Debug, Default, Clone, Hash, Eq, PartialEq, JsonSchema, Serialize)] +#[derive(Debug, Default, Clone, Hash, Eq, PartialEq, JsonSchema, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] enum ImportKind { // `import 'foo.js';` @@ -431,20 +408,6 @@ enum ImportKind { Single, } -impl FromStr for ImportKind { - type Err = &'static str; - - fn from_str(s: &str) -> Result { - match s { - "none" => Ok(ImportKind::None), - "all" => Ok(ImportKind::All), - "multiple" => Ok(ImportKind::Multiple), - "single" => Ok(ImportKind::Single), - _ => Err("Invalid import kind"), - } - } -} - impl Display for ImportKind { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let kind_name = match self { @@ -851,3 +814,25 @@ import a from 'a';", .expect_fix(fix) .test_and_snapshot(); } + +#[test] +fn member_syntax_sort_order_deserialize_valid() { + let json = r#"["none", "all", "multiple", "single"]"#; + let res: MemberSyntaxSortOrder = serde_json::from_str(json).unwrap(); + assert_eq!(res.0.len(), 4); +} + +#[test] +fn member_syntax_sort_order_deserialize_invalid_len() { + let json = r#"["none", "all"]"#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); +} + +#[test] +fn member_syntax_sort_order_deserialize_invalid_dupes() { + // 4 elements but contains duplicates -> should error + let json = r#"["none", "none", "all", "multiple"]"#; + let res = serde_json::from_str::(json); + assert!(res.is_err()); +}