Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ mod import {
pub mod no_named_default;
pub mod no_namespace;
pub mod no_self_import;
pub mod no_unassigned_import;
pub mod no_webpack_loader_syntax;
pub mod unambiguous;
}
Expand Down Expand Up @@ -711,6 +712,7 @@ oxc_macros::declare_all_lint_rules! {
import::exports_last,
import::first,
import::group_exports,
import::no_unassigned_import,
import::no_empty_named_blocks,
import::no_anonymous_default_export,
import::no_absolute_path,
Expand Down
175 changes: 175 additions & 0 deletions crates/oxc_linter/src/rules/import/no_unassigned_import.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
use globset::{Glob, GlobSet, GlobSetBuilder};
use oxc_ast::{
AstKind,
ast::{Argument, Expression},
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::{CompactStr, Span};
use serde_json::Value;

use crate::{AstNode, context::LintContext, rule::Rule};

fn no_unassigned_import_diagnostic(span: Span, msg: &str) -> OxcDiagnostic {
OxcDiagnostic::warn(msg.to_string())
.with_help("Consider assigning the import to a variable or removing it if it's unused.")
.with_label(span)
}

#[derive(Debug, Default, Clone)]
pub struct NoUnassignedImport(Box<NoUnassignedImportConfig>);

#[derive(Debug, Default, Clone)]
pub struct NoUnassignedImportConfig {
globs: GlobSet,
}

impl std::ops::Deref for NoUnassignedImport {
type Target = NoUnassignedImportConfig;

fn deref(&self) -> &Self::Target {
&self.0
}
}

declare_oxc_lint!(
/// ### What it does
///
/// This rule aims to remove modules with side-effects by reporting when a module is imported but not assigned.
///
/// ### Why is this bad?
///
/// With both CommonJS' require and the ES6 modules' import syntax,
/// it is possible to import a module but not to use its result.
/// This can be done explicitly by not assigning the module to a variable.
/// Doing so can mean either of the following things:
/// * The module is imported but not used
/// * The module has side-effects. Having side-effects,
/// makes it hard to know whether the module is actually used or can be removed.
/// It can also make it harder to test or mock parts of your application.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```js
/// import 'should'
/// require('should')
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// import _ from 'foo'
/// import _, {foo} from 'foo'
/// import _, {foo as bar} from 'foo'
/// const _ = require('foo')
/// const {foo} = require('foo')
/// const {foo: bar} = require('foo')
/// bar(require('foo'))
/// ```
NoUnassignedImport,
import,
suspicious,
);

fn build_globset(patterns: Vec<CompactStr>) -> Result<GlobSet, globset::Error> {
if patterns.is_empty() {
return Ok(GlobSet::empty());
}
let mut builder = GlobSetBuilder::new();
for pattern in patterns {
let pattern_str = pattern.as_str();
builder.add(Glob::new(pattern_str)?);
}
builder.build()
}

impl Rule for NoUnassignedImport {
fn from_configuration(value: Value) -> Self {
let obj = value.get(0);
let allow = obj
.and_then(|v| v.get("allow"))
.and_then(Value::as_array)
.map(|v| v.iter().filter_map(Value::as_str).map(CompactStr::from).collect())
.unwrap_or_default();
Self(Box::new(NoUnassignedImportConfig { globs: build_globset(allow).unwrap_or_default() }))
}
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
match node.kind() {
AstKind::ImportDeclaration(import_decl) => {
if import_decl.specifiers.is_some() {
return;
}
let source_str = import_decl.source.value.as_str();
if !self.globs.is_match(source_str) {
ctx.diagnostic(no_unassigned_import_diagnostic(
import_decl.span,
"Imported module should be assigned",
));
}
}
AstKind::ExpressionStatement(statement) => {
let Expression::CallExpression(call_expr) = &statement.expression else {
return;
};
if !call_expr.is_require_call() {
return;
}
let first_arg = &call_expr.arguments[0];
let Argument::StringLiteral(source_str) = first_arg else {
return;
};
if !self.globs.is_match(source_str.value.as_str()) {
ctx.diagnostic(no_unassigned_import_diagnostic(
call_expr.span,
"A `require()` style import is forbidden.",
));
}
}
_ => {}
}
}
}

#[test]
fn test() {
use crate::tester::Tester;
use serde_json::json;

let pass = vec![
("import _ from 'foo'", None),
("import foo from 'foo'", None),
("import foo, { bar } from 'foo'", None),
("import * as _ from 'foo'", None),
("require('lodash')()", None),
("require('lodash').foo", None),
("require('lodash').foo()", None),
("const _ = require('./')", None),
("bar(require('foo'))", None),
("const [a, b] = require('lodash')", None),
("import 'app.css'", Some(json!([{ "allow": ["**/*.css"]}]))),
("import 'app.css'", Some(json!([{ "allow": ["*.css"]}]))),
("import './app.css'", Some(json!([{ "allow": ["**/*.css"]}]))),
("import '../dist/app.css'", Some(json!([{ "allow": ["**/*.css"]}]))),
("import '../dist/app.js'", Some(json!([{ "allow": ["**/dist/**"]}]))),
("import 'foo/bar'", Some(json!([{ "allow": ["foo/**"]}]))),
("import 'foo/bar'", Some(json!([{ "allow": ["foo/bar"]}]))),
("import 'babel-register'", Some(json!([{ "allow": ["babel-register"]}]))),
("require('./app.css')", Some(json!([{ "allow": ["**/*.css"]}]))),
("import './styles/app.css'", Some(json!([{ "allow": ["**/styles/*.css"]}]))),
];

let fail = vec![
("require('should')", None),
("import 'foo'", None),
("import './styles/app.css'", Some(json!([{ "allow": ["styles/*.css"]}]))),
("import './app.css'", Some(json!([{ "allow": ["**/*.js"]}]))),
("import './app.css'", Some(json!([{ "allow": ["**/dir/**"]}]))),
("import './app.js'", None),
("require('./app.css')", Some(json!([{ "allow": ["**/*.js"]}]))),
];

Tester::new(NoUnassignedImport::NAME, NoUnassignedImport::PLUGIN, pass, fail)
.change_rule_path("no-unassigned-import.js")
.with_import_plugin(true)
.test_and_snapshot();
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
---
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint-plugin-import(no-unassigned-import): A `require()` style import is forbidden.
╭─[no-unassigned-import.js:1:1]
1 │ require('should')
· ─────────────────
╰────
help: Consider assigning the import to a variable or removing it if it's unused.

⚠ eslint-plugin-import(no-unassigned-import): Imported module should be assigned
╭─[no-unassigned-import.js:1:1]
1 │ import 'foo'
· ────────────
╰────
help: Consider assigning the import to a variable or removing it if it's unused.

⚠ eslint-plugin-import(no-unassigned-import): Imported module should be assigned
╭─[no-unassigned-import.js:1:1]
1 │ import './styles/app.css'
· ─────────────────────────
╰────
help: Consider assigning the import to a variable or removing it if it's unused.

⚠ eslint-plugin-import(no-unassigned-import): Imported module should be assigned
╭─[no-unassigned-import.js:1:1]
1 │ import './app.css'
· ──────────────────
╰────
help: Consider assigning the import to a variable or removing it if it's unused.

⚠ eslint-plugin-import(no-unassigned-import): Imported module should be assigned
╭─[no-unassigned-import.js:1:1]
1 │ import './app.css'
· ──────────────────
╰────
help: Consider assigning the import to a variable or removing it if it's unused.

⚠ eslint-plugin-import(no-unassigned-import): Imported module should be assigned
╭─[no-unassigned-import.js:1:1]
1 │ import './app.js'
· ─────────────────
╰────
help: Consider assigning the import to a variable or removing it if it's unused.

⚠ eslint-plugin-import(no-unassigned-import): A `require()` style import is forbidden.
╭─[no-unassigned-import.js:1:1]
1 │ require('./app.css')
· ────────────────────
╰────
help: Consider assigning the import to a variable or removing it if it's unused.
Loading