From 4d577cf69f422c0ea6131af8d3f88c6c4775afcf Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Sat, 9 Nov 2024 12:10:59 +0300 Subject: [PATCH] feat(linter): add `import/first` rule (#7180) --- crates/oxc_linter/src/rules.rs | 2 + crates/oxc_linter/src/rules/import/first.rs | 235 ++++++++++++++++++++ crates/oxc_linter/src/snapshots/first.snap | 83 +++++++ 3 files changed, 320 insertions(+) create mode 100644 crates/oxc_linter/src/rules/import/first.rs create mode 100644 crates/oxc_linter/src/snapshots/first.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index d804700a2139a..a38c96443687d 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -11,6 +11,7 @@ mod import { // pub mod no_unused_modules; pub mod default; pub mod export; + pub mod first; pub mod max_dependencies; pub mod named; pub mod namespace; @@ -623,6 +624,7 @@ oxc_macros::declare_all_lint_rules! { eslint::valid_typeof, import::default, import::export, + import::first, import::max_dependencies, import::named, import::namespace, diff --git a/crates/oxc_linter/src/rules/import/first.rs b/crates/oxc_linter/src/rules/import/first.rs new file mode 100644 index 0000000000000..32c60e16078b2 --- /dev/null +++ b/crates/oxc_linter/src/rules/import/first.rs @@ -0,0 +1,235 @@ +use oxc_ast::{ + ast::{Statement, TSModuleReference}, + AstKind, +}; +use oxc_diagnostics::OxcDiagnostic; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use std::convert::From; + +use crate::{context::LintContext, rule::Rule}; + +fn first_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Import statements must come first") + .with_help("Move import statement to the top of the file") + .with_label(span) +} + +fn absolute_first_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Relative imports before absolute imports are prohibited") + .with_help("Move absolute import above relative import") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct First { + absolute_first: AbsoluteFirst, +} + +#[derive(Debug, Default, Clone)] +enum AbsoluteFirst { + AbsoluteFirst, + #[default] + DisableAbsoluteFirst, +} + +impl From<&str> for AbsoluteFirst { + fn from(raw: &str) -> Self { + match raw { + "absolute-first" => Self::AbsoluteFirst, + _ => Self::DisableAbsoluteFirst, + } + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Forbids any non-import statements before imports except directives. + /// + /// ### Why is this bad? + /// + /// Notably, imports are hoisted, which means the imported modules will be evaluated + /// before any of the statements interspersed between them. + /// Keeping all imports together at the top of the file may prevent surprises + /// resulting from this part of the spec + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// import { x } from './foo'; + /// export { x }; + /// import { y } from './bar'; + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// import { x } from './foo'; + /// import { y } from './bar'; + /// export { x, y } + /// ``` + /// + /// ### Options + /// + /// with `"absolute-import"`: + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// import { x } from './foo'; + /// import { y } from 'bar' + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// import { y } from 'bar'; + /// import { x } from './foo' + /// ``` + /// + First, + style, + pending // TODO: fixer +); + +fn is_relative_path(path: &str) -> bool { + path.starts_with("./") +} + +/// +impl Rule for First { + fn from_configuration(value: serde_json::Value) -> Self { + let obj = value.get(0); + + Self { + absolute_first: obj + .and_then(serde_json::Value::as_str) + .map(AbsoluteFirst::from) + .unwrap_or_default(), + } + } + + fn run_once(&self, ctx: &LintContext<'_>) { + let mut non_import_count = 0; + let mut any_relative = false; + + let Some(root) = ctx.nodes().root_node() else { + return; + }; + let AstKind::Program(program) = root.kind() else { unreachable!() }; + + for statement in &program.body { + match statement { + Statement::TSImportEqualsDeclaration(decl) => match decl.module_reference { + TSModuleReference::ExternalModuleReference(ref mod_ref) => { + if matches!(self.absolute_first, AbsoluteFirst::AbsoluteFirst) { + if is_relative_path(mod_ref.expression.value.as_str()) { + any_relative = true; + } else if any_relative { + ctx.diagnostic(absolute_first_diagnostic(mod_ref.expression.span)); + } + } + if non_import_count > 0 { + ctx.diagnostic(first_diagnostic(decl.span)); + } + } + TSModuleReference::IdentifierReference(_) + | TSModuleReference::QualifiedName(_) => {} + }, + Statement::ImportDeclaration(decl) => { + if matches!(self.absolute_first, AbsoluteFirst::AbsoluteFirst) { + if is_relative_path(decl.source.value.as_str()) { + any_relative = true; + } else if any_relative { + ctx.diagnostic(absolute_first_diagnostic(decl.source.span)); + } + } + if non_import_count > 0 { + ctx.diagnostic(first_diagnostic(decl.span)); + } + } + _ => { + non_import_count += 1; + } + } + } + } +} + +#[test] +fn test() { + use serde_json::json; + + use crate::tester::Tester; + + let pass = vec![ + ( + r"import { x } from './foo'; import { y } from './bar'; + export { x, y }", + None, + ), + (r"import { x } from 'foo'; import { y } from './bar'", None), + (r"import { x } from './foo'; import { y } from 'bar'", None), + ( + r"import { x } from './foo'; import { y } from 'bar'", + Some(json!(["disable-absolute-first"])), + ), + // Note: original rule contains test case below for `angular-eslint` parser + // which is not implemented in oxc + ( + r"'use directive'; + import { x } from 'foo';", + None, + ), + // covers TSImportEqualsDeclaration (original rule support it, but with no test cases) + ( + r"import { x } from './foo'; + import F3 = require('mod'); + export { x, y }", + None, + ), + ]; + + let fail = vec![ + ( + r"import { x } from './foo'; + export { x }; + import { y } from './bar';", + None, + ), + ( + r"import { x } from './foo'; + export { x }; + import { y } from './bar'; + import { z } from './baz';", + None, + ), + (r"import { x } from './foo'; import { y } from 'bar'", Some(json!(["absolute-first"]))), + ( + r"import { x } from 'foo'; + 'use directive'; + import { y } from 'bar';", + None, + ), + ( + r"var a = 1; + import { y } from './bar'; + if (true) { x() }; + import { x } from './foo'; + import { z } from './baz';", + None, + ), + (r"if (true) { console.log(1) }import a from 'b'", None), + // covers TSImportEqualsDeclaration (original rule support it, but with no test cases) + ( + r"import { x } from './foo'; + export { x }; + import F3 = require('mod');", + None, + ), + ]; + + Tester::new(First::NAME, pass, fail) + .change_rule_path("index.ts") + .with_import_plugin(true) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/first.snap b/crates/oxc_linter/src/snapshots/first.snap new file mode 100644 index 0000000000000..3b0cc6fed1a6b --- /dev/null +++ b/crates/oxc_linter/src/snapshots/first.snap @@ -0,0 +1,83 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:3:15] + 2 │ export { x }; + 3 │ import { y } from './bar'; + · ────────────────────────── + ╰──── + help: Move import statement to the top of the file + + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:3:15] + 2 │ export { x }; + 3 │ import { y } from './bar'; + · ────────────────────────── + 4 │ import { z } from './baz'; + ╰──── + help: Move import statement to the top of the file + + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:4:15] + 3 │ import { y } from './bar'; + 4 │ import { z } from './baz'; + · ────────────────────────── + ╰──── + help: Move import statement to the top of the file + + ⚠ eslint-plugin-import(first): Relative imports before absolute imports are prohibited + ╭─[index.ts:1:46] + 1 │ import { x } from './foo'; import { y } from 'bar' + · ───── + ╰──── + help: Move absolute import above relative import + + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:3:15] + 2 │ 'use directive'; + 3 │ import { y } from 'bar'; + · ──────────────────────── + ╰──── + help: Move import statement to the top of the file + + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:2:15] + 1 │ var a = 1; + 2 │ import { y } from './bar'; + · ────────────────────────── + 3 │ if (true) { x() }; + ╰──── + help: Move import statement to the top of the file + + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:4:15] + 3 │ if (true) { x() }; + 4 │ import { x } from './foo'; + · ────────────────────────── + 5 │ import { z } from './baz'; + ╰──── + help: Move import statement to the top of the file + + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:5:15] + 4 │ import { x } from './foo'; + 5 │ import { z } from './baz'; + · ────────────────────────── + ╰──── + help: Move import statement to the top of the file + + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:1:29] + 1 │ if (true) { console.log(1) }import a from 'b' + · ───────────────── + ╰──── + help: Move import statement to the top of the file + + ⚠ eslint-plugin-import(first): Import statements must come first + ╭─[index.ts:3:15] + 2 │ export { x }; + 3 │ import F3 = require('mod'); + · ─────────────────────────── + ╰──── + help: Move import statement to the top of the file