Skip to content

Commit c163231

Browse files
committed
fix(linter): Update eslint/sort-imports to validate options. (#18378)
This retains the ability to enforce the length requirement for the `memberSyntaxSortOrder`. This rule should potentially be deprecated and replaced by oxfmt in the future, but for now let's support this.
1 parent 79bbcff commit c163231

4 files changed

Lines changed: 91 additions & 73 deletions

File tree

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"plugins": ["eslint"],
3+
"categories": {
4+
"correctness": "off"
5+
},
6+
"rules": {
7+
// Invalid config, memberSyntaxSortOrder must have all 4 values.
8+
"eslint/sort-imports": ["error", { "memberSyntaxSortOrder": ["none", "all", "multiple"] }]
9+
}
10+
}

apps/oxlint/src/lint.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1518,6 +1518,13 @@ export { redundant };
15181518
Tester::new().with_cwd("fixtures/extends_invalid_config".into()).test_and_snapshot(&[]);
15191519
}
15201520

1521+
#[test]
1522+
fn test_invalid_config_invalid_config_sort_imports() {
1523+
Tester::new()
1524+
.with_cwd("fixtures/invalid_config_sort_imports".into())
1525+
.test_and_snapshot(&[]);
1526+
}
1527+
15211528
#[test]
15221529
fn test_valid_complex_config() {
15231530
Tester::new().with_cwd("fixtures/valid_complex_config".into()).test_and_snapshot(&[]);
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
---
2+
source: apps/oxlint/src/tester.rs
3+
---
4+
##########
5+
arguments:
6+
working directory: fixtures/invalid_config_sort_imports
7+
----------
8+
Failed to parse oxlint configuration file.
9+
10+
x Invalid configuration for rule `sort-imports`:
11+
| memberSyntaxSortOrder must contain exactly 4 kinds, got 3
12+
| received config: `{ "memberSyntaxSortOrder": [ "none", "all", "multiple" ] }`
13+
14+
----------
15+
CLI result: InvalidOptionConfig
16+
----------

crates/oxc_linter/src/rules/eslint/sort_imports.rs

Lines changed: 58 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
use std::{
22
borrow::Cow,
33
fmt::{Display, Write},
4-
str::FromStr,
54
};
65

76
use cow_utils::CowUtils;
@@ -10,10 +9,14 @@ use oxc_ast::ast::{ImportDeclaration, ImportDeclarationSpecifier, Statement};
109
use oxc_diagnostics::OxcDiagnostic;
1110
use oxc_macros::declare_oxc_lint;
1211
use oxc_span::Span;
12+
use rustc_hash::FxHashSet;
1313
use schemars::JsonSchema;
14-
use serde::Serialize;
14+
use serde::{Deserialize, Serialize};
1515

16-
use crate::{context::LintContext, rule::Rule};
16+
use crate::{
17+
context::LintContext,
18+
rule::{DefaultRuleConfig, Rule},
19+
};
1720

1821
fn unexpected_syntax_order_diagnostic(
1922
curr_kind: &ImportKind,
@@ -35,11 +38,11 @@ fn sort_members_alphabetically_diagnostic(name: &str, span: Span) -> OxcDiagnost
3538
.with_label(span)
3639
}
3740

38-
#[derive(Debug, Default, Clone)]
41+
#[derive(Debug, Default, Clone, Deserialize)]
3942
pub struct SortImports(Box<SortImportsOptions>);
4043

41-
#[derive(Debug, Default, Clone, JsonSchema)]
42-
#[serde(rename_all = "camelCase", default)]
44+
#[derive(Debug, Default, Clone, JsonSchema, Deserialize)]
45+
#[serde(rename_all = "camelCase", default, deny_unknown_fields)]
4346
pub struct SortImportsOptions {
4447
/// When `true`, the rule ignores case-sensitivity when sorting import names.
4548
ignore_case: bool,
@@ -50,7 +53,7 @@ pub struct SortImportsOptions {
5053
/// When `true`, the rule allows import groups separated by blank lines to be treated independently.
5154
allow_separated_groups: bool,
5255
/// Specifies the sort order of different import syntaxes.
53-
/// Must include all 4 kinds or else this will fall back to default.
56+
/// Must include all 4 kinds!
5457
member_syntax_sort_order: MemberSyntaxSortOrder,
5558
}
5659

@@ -79,7 +82,7 @@ declare_oxc_lint!(
7982
///
8083
/// Examples of **incorrect** code for this rule:
8184
/// ```javascript
82-
/// import {b, a, c} from 'foo.js'
85+
/// import { b, a, c } from 'foo.js'
8386
///
8487
/// import d from 'foo.js';
8588
/// import e from 'bar.js';
@@ -93,56 +96,7 @@ declare_oxc_lint!(
9396

9497
impl Rule for SortImports {
9598
fn from_configuration(value: serde_json::Value) -> Result<Self, serde_json::error::Error> {
96-
let Some(config) = value.get(0) else {
97-
return Ok(Self(Box::default()));
98-
};
99-
100-
let ignore_case =
101-
config.get("ignoreCase").and_then(serde_json::Value::as_bool).unwrap_or_default();
102-
let ignore_member_sort =
103-
config.get("ignoreMemberSort").and_then(serde_json::Value::as_bool).unwrap_or_default();
104-
let ignore_declaration_sort = config
105-
.get("ignoreDeclarationSort")
106-
.and_then(serde_json::Value::as_bool)
107-
.unwrap_or_default();
108-
let allow_separated_groups = config
109-
.get("allowSeparatedGroups")
110-
.and_then(serde_json::Value::as_bool)
111-
.unwrap_or_default();
112-
113-
let member_syntax_sort_order = config
114-
.get("memberSyntaxSortOrder")
115-
.and_then(|v| v.as_array())
116-
.map(|arr| {
117-
// memberSyntaxSortOrder in config file must have 4 items
118-
if arr.len() != 4 {
119-
return MemberSyntaxSortOrder::default();
120-
}
121-
122-
let kinds: Vec<ImportKind> = arr
123-
.iter()
124-
.filter_map(|v| v.as_str())
125-
.map(ImportKind::from_str)
126-
.filter_map(Result::ok)
127-
.unique()
128-
.collect();
129-
130-
// 4 items must all unique and valid.
131-
if kinds.len() != 4 {
132-
return MemberSyntaxSortOrder::default();
133-
}
134-
135-
MemberSyntaxSortOrder(kinds)
136-
})
137-
.unwrap_or_default();
138-
139-
Ok(Self(Box::new(SortImportsOptions {
140-
ignore_case,
141-
ignore_declaration_sort,
142-
ignore_member_sort,
143-
allow_separated_groups,
144-
member_syntax_sort_order,
145-
})))
99+
serde_json::from_value::<DefaultRuleConfig<Self>>(value).map(DefaultRuleConfig::into_inner)
146100
}
147101

148102
fn run_once(&self, ctx: &LintContext) {
@@ -367,6 +321,29 @@ impl SortImports {
367321
#[schemars(transparent)]
368322
struct MemberSyntaxSortOrder(Vec<ImportKind>);
369323

324+
impl<'de> serde::Deserialize<'de> for MemberSyntaxSortOrder {
325+
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
326+
where
327+
D: serde::Deserializer<'de>,
328+
{
329+
let v = Vec::<ImportKind>::deserialize(deserializer)?;
330+
if v.len() != 4 {
331+
return Err(serde::de::Error::custom(format!(
332+
"memberSyntaxSortOrder must contain exactly 4 kinds, got {}",
333+
v.len()
334+
)));
335+
}
336+
// Ensure the values are unique.
337+
let set: FxHashSet<_> = v.iter().collect();
338+
if set.len() != 4 {
339+
return Err(serde::de::Error::custom(
340+
"memberSyntaxSortOrder must contain 4 unique kinds, but duplicates were found",
341+
));
342+
}
343+
Ok(MemberSyntaxSortOrder(v))
344+
}
345+
}
346+
370347
impl Default for MemberSyntaxSortOrder {
371348
fn default() -> Self {
372349
MemberSyntaxSortOrder(vec![
@@ -417,7 +394,7 @@ impl MemberSyntaxSortOrder {
417394
}
418395
}
419396

420-
#[derive(Debug, Default, Clone, Hash, Eq, PartialEq, JsonSchema, Serialize)]
397+
#[derive(Debug, Default, Clone, Hash, Eq, PartialEq, JsonSchema, Serialize, Deserialize)]
421398
#[serde(rename_all = "lowercase")]
422399
enum ImportKind {
423400
// `import 'foo.js';`
@@ -431,20 +408,6 @@ enum ImportKind {
431408
Single,
432409
}
433410

434-
impl FromStr for ImportKind {
435-
type Err = &'static str;
436-
437-
fn from_str(s: &str) -> Result<Self, Self::Err> {
438-
match s {
439-
"none" => Ok(ImportKind::None),
440-
"all" => Ok(ImportKind::All),
441-
"multiple" => Ok(ImportKind::Multiple),
442-
"single" => Ok(ImportKind::Single),
443-
_ => Err("Invalid import kind"),
444-
}
445-
}
446-
}
447-
448411
impl Display for ImportKind {
449412
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
450413
let kind_name = match self {
@@ -851,3 +814,25 @@ import a from 'a';",
851814
.expect_fix(fix)
852815
.test_and_snapshot();
853816
}
817+
818+
#[test]
819+
fn member_syntax_sort_order_deserialize_valid() {
820+
let json = r#"["none", "all", "multiple", "single"]"#;
821+
let res: MemberSyntaxSortOrder = serde_json::from_str(json).unwrap();
822+
assert_eq!(res.0.len(), 4);
823+
}
824+
825+
#[test]
826+
fn member_syntax_sort_order_deserialize_invalid_len() {
827+
let json = r#"["none", "all"]"#;
828+
let res = serde_json::from_str::<MemberSyntaxSortOrder>(json);
829+
assert!(res.is_err());
830+
}
831+
832+
#[test]
833+
fn member_syntax_sort_order_deserialize_invalid_dupes() {
834+
// 4 elements but contains duplicates -> should error
835+
let json = r#"["none", "none", "all", "multiple"]"#;
836+
let res = serde_json::from_str::<MemberSyntaxSortOrder>(json);
837+
assert!(res.is_err());
838+
}

0 commit comments

Comments
 (0)