From 1ca3cbf53c1f1eef2549e088ab47d9f98c0c8ede Mon Sep 17 00:00:00 2001 From: Dmitry Zakharov Date: Thu, 14 Nov 2024 14:23:56 +0300 Subject: [PATCH] feat(linter): add `import/no-namespace` rule (#7229) --- Cargo.lock | 7 + Cargo.toml | 1 + crates/oxc_linter/Cargo.toml | 1 + crates/oxc_linter/src/rules.rs | 2 + .../src/rules/import/import_no_namespace.rs | 166 ++++++++++++++++++ .../src/snapshots/import_no_namespace.snap | 23 +++ 6 files changed, 200 insertions(+) create mode 100644 crates/oxc_linter/src/rules/import/import_no_namespace.rs create mode 100644 crates/oxc_linter/src/snapshots/import_no_namespace.snap diff --git a/Cargo.lock b/Cargo.lock index d53d25b9944820..b2c517e01547b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -546,6 +546,12 @@ dependencies = [ "windows-sys 0.52.0", ] +[[package]] +name = "fast-glob" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bb10ed0f8a3dca52477be37ac0fb8f9d1fd4cd8d311b4484bdd45c1c56e0c9ec" + [[package]] name = "fastrand" version = "2.1.1" @@ -1701,6 +1707,7 @@ dependencies = [ "convert_case", "cow-utils", "dashmap 6.1.0", + "fast-glob", "globset", "insta", "itertools", diff --git a/Cargo.toml b/Cargo.toml index c273851d260306..8d32f30ead5718 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -140,6 +140,7 @@ dashmap = "6.1.0" encoding_rs = "0.8.34" encoding_rs_io = "0.1.7" env_logger = { version = "0.11.5", default-features = false } +fast-glob = "0.4.0" flate2 = "1.0.34" futures = "0.3.31" glob = "0.3.1" diff --git a/crates/oxc_linter/Cargo.toml b/crates/oxc_linter/Cargo.toml index 3899430b4564e6..50a9006ba030a3 100644 --- a/crates/oxc_linter/Cargo.toml +++ b/crates/oxc_linter/Cargo.toml @@ -40,6 +40,7 @@ bitflags = { workspace = true } convert_case = { workspace = true } cow-utils = { workspace = true } dashmap = { workspace = true } +fast-glob = { workspace = true } globset = { workspace = true, features = ["serde1"] } itertools = { workspace = true } json-strip-comments = { workspace = true } diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 486491322f426f..8714025070a8a2 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -12,6 +12,7 @@ mod import { pub mod default; pub mod export; pub mod first; + pub mod import_no_namespace; pub mod max_dependencies; pub mod named; pub mod namespace; @@ -627,6 +628,7 @@ oxc_macros::declare_all_lint_rules! { import::default, import::export, import::first, + import::import_no_namespace, import::max_dependencies, import::named, import::namespace, diff --git a/crates/oxc_linter/src/rules/import/import_no_namespace.rs b/crates/oxc_linter/src/rules/import/import_no_namespace.rs new file mode 100644 index 00000000000000..44371dc8253689 --- /dev/null +++ b/crates/oxc_linter/src/rules/import/import_no_namespace.rs @@ -0,0 +1,166 @@ +use fast_glob::glob_match; +use oxc_diagnostics::OxcDiagnostic; + +use oxc_macros::declare_oxc_lint; +use oxc_span::{CompactStr, Span}; +use oxc_syntax::module_record::ImportImportName; + +use crate::{context::LintContext, rule::Rule}; + +fn import_no_namespace_diagnostic(span: Span) -> OxcDiagnostic { + OxcDiagnostic::warn("Usage of namespaced aka wildcard \"*\" imports prohibited") + .with_help("Use named or default imports") + .with_label(span) +} + +#[derive(Debug, Default, Clone)] +pub struct ImportNoNamespace(Box); + +#[derive(Debug, Default, Clone)] +pub struct ImportNoNamespaceConfig { + ignore: Vec, +} + +impl std::ops::Deref for ImportNoNamespace { + type Target = ImportNoNamespaceConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +declare_oxc_lint!( + /// ### What it does + /// + /// Enforce a convention of not using namespaced (a.k.a. "wildcard" *) imports. + /// + /// ### Why is this bad? + /// + /// Namespaced imports, while sometimes used, are generally considered less ideal in modern JavaScript development for several reasons: + /// + /// 1. **Specificity and Namespace Pollution**: + /// * **Specificity**: Namespaced imports import the entire module, bringing in everything, even if you only need a few specific functions or classes. This can lead to potential naming conflicts if different modules have the same names for different functions. + /// * **Pollution**: Importing an entire namespace pollutes your current scope with potentially unnecessary functions and variables. It increases the chance of accidental use of an unintended function or variable, leading to harder-to-debug errors. + /// + /// 2. **Maintainability**: + /// * **Clarity**: Namespaced imports can make it harder to understand which specific functions or classes are being used in your code. This is especially true in larger projects with numerous imports. + /// * **Refactoring**: If a function or class name changes within the imported module, you might need to update several parts of your code if you are using namespaced imports. This becomes even more challenging when dealing with multiple namespaces. + /// + /// 3. **Modern Practice**: + /// * **Explicit Imports**: Modern JavaScript practices encourage explicit imports for specific components. This enhances code readability and maintainability. + /// * **Tree-Shaking**: Tools like Webpack and Rollup use tree-shaking to remove unused code from your bundles. Namespaced imports can prevent efficient tree-shaking, leading to larger bundle sizes. + /// + /// ### Options + /// + /// `ignore` : array of glob strings for modules that should be ignored by the rule. + /// + /// ```json + /// { + /// "ignores": ["*.json"] + /// } + /// ``` + /// + /// ### Examples + /// + /// Examples of **incorrect** code for this rule: + /// ```js + /// import * as user from 'user-lib'; + /// + /// import some, * as user from './user'; + /// ``` + /// + /// Examples of **correct** code for this rule: + /// ```js + /// import { getUserName, isUser } from 'user-lib'; + /// + /// import user from 'user-lib'; + /// import defaultExport, { isUser } from './user'; + /// ``` + /// + ImportNoNamespace, + style, + pending // TODO: fixer +); + +/// +impl Rule for ImportNoNamespace { + fn from_configuration(value: serde_json::Value) -> Self { + let obj = value.get(0); + Self(Box::new(ImportNoNamespaceConfig { + ignore: obj + .and_then(|v| v.get("ignore")) + .and_then(serde_json::Value::as_array) + .map(|v| { + v.iter().filter_map(serde_json::Value::as_str).map(CompactStr::from).collect() + }) + .unwrap_or_default(), + })) + } + + fn run_once(&self, ctx: &LintContext<'_>) { + let module_record = ctx.module_record(); + + if module_record.not_esm { + return; + } + + module_record.import_entries.iter().for_each(|entry| { + match &entry.import_name { + ImportImportName::NamespaceObject => { + let source = entry.module_request.name(); + + if self.ignore.is_empty() { + ctx.diagnostic(import_no_namespace_diagnostic(entry.local_name.span())); + } else { + if !source.contains('.') { + return; + } + + if self + .ignore + .iter() + .any(|pattern| glob_match(pattern, source.trim_start_matches("./"))) + { + return; + } + + ctx.diagnostic(import_no_namespace_diagnostic(entry.local_name.span())); + } + } + ImportImportName::Name(_) | ImportImportName::Default(_) => {} + }; + }); + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + (r"import { a, b } from 'foo';", None), + (r"import { a, b } from './foo';", None), + (r"import bar from 'bar';", None), + (r"import bar from './bar';", None), + ( + r"import * as bar from './ignored-module.ext';", + Some(serde_json::json!([{ "ignore": ["*.ext"] }])), + ), + ( + r"import * as bar from './ignored-module.js'; + import * as baz from './other-module.ts'", + Some(serde_json::json!([{ "ignore": ["*.js", "*.ts"] }])), + ), + ]; + + let fail = vec![ + (r"import * as foo from 'foo';", None), + (r"import defaultExport, * as foo from 'foo';", None), + (r"import * as foo from './foo';", None), + ]; + + Tester::new(ImportNoNamespace::NAME, pass, fail) + .change_rule_path("index.js") + .with_import_plugin(true) + .test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/import_no_namespace.snap b/crates/oxc_linter/src/snapshots/import_no_namespace.snap new file mode 100644 index 00000000000000..af3286c6777177 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/import_no_namespace.snap @@ -0,0 +1,23 @@ +--- +source: crates/oxc_linter/src/tester.rs +--- + ⚠ eslint-plugin-import(import-no-namespace): Usage of namespaced aka wildcard "*" imports prohibited + ╭─[index.js:1:13] + 1 │ import * as foo from 'foo'; + · ─── + ╰──── + help: Use named or default imports + + ⚠ eslint-plugin-import(import-no-namespace): Usage of namespaced aka wildcard "*" imports prohibited + ╭─[index.js:1:28] + 1 │ import defaultExport, * as foo from 'foo'; + · ─── + ╰──── + help: Use named or default imports + + ⚠ eslint-plugin-import(import-no-namespace): Usage of namespaced aka wildcard "*" imports prohibited + ╭─[index.js:1:13] + 1 │ import * as foo from './foo'; + · ─── + ╰──── + help: Use named or default imports