Skip to content

Commit eef93b4

Browse files
feat(linter): add import/no-unassigned-import (#10970)
Relates to #1117 Rule detail:https://github.com/import-js/eslint-plugin-import/blob/v2.31.0/docs/rules/no-unassigned-import.md --------- Co-authored-by: Cameron Clark <cameron.clark@hey.com>
1 parent 58d2e6e commit eef93b4

File tree

3 files changed

+228
-0
lines changed

3 files changed

+228
-0
lines changed

crates/oxc_linter/src/rules.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ mod import {
3131
pub mod no_named_default;
3232
pub mod no_namespace;
3333
pub mod no_self_import;
34+
pub mod no_unassigned_import;
3435
pub mod no_webpack_loader_syntax;
3536
pub mod unambiguous;
3637
}
@@ -711,6 +712,7 @@ oxc_macros::declare_all_lint_rules! {
711712
import::exports_last,
712713
import::first,
713714
import::group_exports,
715+
import::no_unassigned_import,
714716
import::no_empty_named_blocks,
715717
import::no_anonymous_default_export,
716718
import::no_absolute_path,
Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
use globset::{Glob, GlobSet, GlobSetBuilder};
2+
use oxc_ast::{
3+
AstKind,
4+
ast::{Argument, Expression},
5+
};
6+
use oxc_diagnostics::OxcDiagnostic;
7+
use oxc_macros::declare_oxc_lint;
8+
use oxc_span::{CompactStr, Span};
9+
use serde_json::Value;
10+
11+
use crate::{AstNode, context::LintContext, rule::Rule};
12+
13+
fn no_unassigned_import_diagnostic(span: Span, msg: &str) -> OxcDiagnostic {
14+
OxcDiagnostic::warn(msg.to_string())
15+
.with_help("Consider assigning the import to a variable or removing it if it's unused.")
16+
.with_label(span)
17+
}
18+
19+
#[derive(Debug, Default, Clone)]
20+
pub struct NoUnassignedImport(Box<NoUnassignedImportConfig>);
21+
22+
#[derive(Debug, Default, Clone)]
23+
pub struct NoUnassignedImportConfig {
24+
globs: GlobSet,
25+
}
26+
27+
impl std::ops::Deref for NoUnassignedImport {
28+
type Target = NoUnassignedImportConfig;
29+
30+
fn deref(&self) -> &Self::Target {
31+
&self.0
32+
}
33+
}
34+
35+
declare_oxc_lint!(
36+
/// ### What it does
37+
///
38+
/// This rule aims to remove modules with side-effects by reporting when a module is imported but not assigned.
39+
///
40+
/// ### Why is this bad?
41+
///
42+
/// With both CommonJS' require and the ES6 modules' import syntax,
43+
/// it is possible to import a module but not to use its result.
44+
/// This can be done explicitly by not assigning the module to a variable.
45+
/// Doing so can mean either of the following things:
46+
/// * The module is imported but not used
47+
/// * The module has side-effects. Having side-effects,
48+
/// makes it hard to know whether the module is actually used or can be removed.
49+
/// It can also make it harder to test or mock parts of your application.
50+
///
51+
/// ### Examples
52+
///
53+
/// Examples of **incorrect** code for this rule:
54+
/// ```js
55+
/// import 'should'
56+
/// require('should')
57+
/// ```
58+
///
59+
/// Examples of **correct** code for this rule:
60+
/// ```js
61+
/// import _ from 'foo'
62+
/// import _, {foo} from 'foo'
63+
/// import _, {foo as bar} from 'foo'
64+
/// const _ = require('foo')
65+
/// const {foo} = require('foo')
66+
/// const {foo: bar} = require('foo')
67+
/// bar(require('foo'))
68+
/// ```
69+
NoUnassignedImport,
70+
import,
71+
suspicious,
72+
);
73+
74+
fn build_globset(patterns: Vec<CompactStr>) -> Result<GlobSet, globset::Error> {
75+
if patterns.is_empty() {
76+
return Ok(GlobSet::empty());
77+
}
78+
let mut builder = GlobSetBuilder::new();
79+
for pattern in patterns {
80+
let pattern_str = pattern.as_str();
81+
builder.add(Glob::new(pattern_str)?);
82+
}
83+
builder.build()
84+
}
85+
86+
impl Rule for NoUnassignedImport {
87+
fn from_configuration(value: Value) -> Self {
88+
let obj = value.get(0);
89+
let allow = obj
90+
.and_then(|v| v.get("allow"))
91+
.and_then(Value::as_array)
92+
.map(|v| v.iter().filter_map(Value::as_str).map(CompactStr::from).collect())
93+
.unwrap_or_default();
94+
Self(Box::new(NoUnassignedImportConfig { globs: build_globset(allow).unwrap_or_default() }))
95+
}
96+
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
97+
match node.kind() {
98+
AstKind::ImportDeclaration(import_decl) => {
99+
if import_decl.specifiers.is_some() {
100+
return;
101+
}
102+
let source_str = import_decl.source.value.as_str();
103+
if !self.globs.is_match(source_str) {
104+
ctx.diagnostic(no_unassigned_import_diagnostic(
105+
import_decl.span,
106+
"Imported module should be assigned",
107+
));
108+
}
109+
}
110+
AstKind::ExpressionStatement(statement) => {
111+
let Expression::CallExpression(call_expr) = &statement.expression else {
112+
return;
113+
};
114+
if !call_expr.is_require_call() {
115+
return;
116+
}
117+
let first_arg = &call_expr.arguments[0];
118+
let Argument::StringLiteral(source_str) = first_arg else {
119+
return;
120+
};
121+
if !self.globs.is_match(source_str.value.as_str()) {
122+
ctx.diagnostic(no_unassigned_import_diagnostic(
123+
call_expr.span,
124+
"A `require()` style import is forbidden.",
125+
));
126+
}
127+
}
128+
_ => {}
129+
}
130+
}
131+
}
132+
133+
#[test]
134+
fn test() {
135+
use crate::tester::Tester;
136+
use serde_json::json;
137+
138+
let pass = vec![
139+
("import _ from 'foo'", None),
140+
("import foo from 'foo'", None),
141+
("import foo, { bar } from 'foo'", None),
142+
("import * as _ from 'foo'", None),
143+
("require('lodash')()", None),
144+
("require('lodash').foo", None),
145+
("require('lodash').foo()", None),
146+
("const _ = require('./')", None),
147+
("bar(require('foo'))", None),
148+
("const [a, b] = require('lodash')", None),
149+
("import 'app.css'", Some(json!([{ "allow": ["**/*.css"]}]))),
150+
("import 'app.css'", Some(json!([{ "allow": ["*.css"]}]))),
151+
("import './app.css'", Some(json!([{ "allow": ["**/*.css"]}]))),
152+
("import '../dist/app.css'", Some(json!([{ "allow": ["**/*.css"]}]))),
153+
("import '../dist/app.js'", Some(json!([{ "allow": ["**/dist/**"]}]))),
154+
("import 'foo/bar'", Some(json!([{ "allow": ["foo/**"]}]))),
155+
("import 'foo/bar'", Some(json!([{ "allow": ["foo/bar"]}]))),
156+
("import 'babel-register'", Some(json!([{ "allow": ["babel-register"]}]))),
157+
("require('./app.css')", Some(json!([{ "allow": ["**/*.css"]}]))),
158+
("import './styles/app.css'", Some(json!([{ "allow": ["**/styles/*.css"]}]))),
159+
];
160+
161+
let fail = vec![
162+
("require('should')", None),
163+
("import 'foo'", None),
164+
("import './styles/app.css'", Some(json!([{ "allow": ["styles/*.css"]}]))),
165+
("import './app.css'", Some(json!([{ "allow": ["**/*.js"]}]))),
166+
("import './app.css'", Some(json!([{ "allow": ["**/dir/**"]}]))),
167+
("import './app.js'", None),
168+
("require('./app.css')", Some(json!([{ "allow": ["**/*.js"]}]))),
169+
];
170+
171+
Tester::new(NoUnassignedImport::NAME, NoUnassignedImport::PLUGIN, pass, fail)
172+
.change_rule_path("no-unassigned-import.js")
173+
.with_import_plugin(true)
174+
.test_and_snapshot();
175+
}
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
---
2+
source: crates/oxc_linter/src/tester.rs
3+
---
4+
eslint-plugin-import(no-unassigned-import): A `require()` style import is forbidden.
5+
╭─[no-unassigned-import.js:1:1]
6+
1require('should')
7+
· ─────────────────
8+
╰────
9+
help: Consider assigning the import to a variable or removing it if it's unused.
10+
11+
eslint-plugin-import(no-unassigned-import): Imported module should be assigned
12+
╭─[no-unassigned-import.js:1:1]
13+
1import 'foo'
14+
· ────────────
15+
╰────
16+
help: Consider assigning the import to a variable or removing it if it's unused.
17+
18+
eslint-plugin-import(no-unassigned-import): Imported module should be assigned
19+
╭─[no-unassigned-import.js:1:1]
20+
1import './styles/app.css'
21+
· ─────────────────────────
22+
╰────
23+
help: Consider assigning the import to a variable or removing it if it's unused.
24+
25+
eslint-plugin-import(no-unassigned-import): Imported module should be assigned
26+
╭─[no-unassigned-import.js:1:1]
27+
1import './app.css'
28+
· ──────────────────
29+
╰────
30+
help: Consider assigning the import to a variable or removing it if it's unused.
31+
32+
eslint-plugin-import(no-unassigned-import): Imported module should be assigned
33+
╭─[no-unassigned-import.js:1:1]
34+
1import './app.css'
35+
· ──────────────────
36+
╰────
37+
help: Consider assigning the import to a variable or removing it if it's unused.
38+
39+
eslint-plugin-import(no-unassigned-import): Imported module should be assigned
40+
╭─[no-unassigned-import.js:1:1]
41+
1import './app.js'
42+
· ─────────────────
43+
╰────
44+
help: Consider assigning the import to a variable or removing it if it's unused.
45+
46+
eslint-plugin-import(no-unassigned-import): A `require()` style import is forbidden.
47+
╭─[no-unassigned-import.js:1:1]
48+
1require('./app.css')
49+
· ────────────────────
50+
╰────
51+
help: Consider assigning the import to a variable or removing it if it's unused.

0 commit comments

Comments
 (0)