diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index ed3aa04ff579b..cfd4045a95fde 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -8,6 +8,7 @@ /// mod import { pub mod named; + pub mod no_self_import; } mod deepscan { @@ -202,5 +203,6 @@ oxc_macros::declare_all_lint_rules! { jest::no_interpolation_in_snapshots, unicorn::no_instanceof_array, unicorn::no_unnecessary_await, - import::named + import::named, + import::no_self_import } diff --git a/crates/oxc_linter/src/rules/import/named.rs b/crates/oxc_linter/src/rules/import/named.rs index 7fa69eecd298f..d2a9b1d133c5d 100644 --- a/crates/oxc_linter/src/rules/import/named.rs +++ b/crates/oxc_linter/src/rules/import/named.rs @@ -200,7 +200,7 @@ fn test() { ]; Tester::new_without_config(Named::NAME, pass, fail) - .with_rule_path_extension("js") + .change_rule_path_extension("js") .with_import_plugin(true) .test_and_snapshot(); } diff --git a/crates/oxc_linter/src/rules/import/no_self_import.rs b/crates/oxc_linter/src/rules/import/no_self_import.rs new file mode 100644 index 0000000000000..930dd9f1949b3 --- /dev/null +++ b/crates/oxc_linter/src/rules/import/no_self_import.rs @@ -0,0 +1,116 @@ +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; + +use crate::{context::LintContext, rule::Rule}; + +#[derive(Debug, Error, Diagnostic)] +#[error("eslint-plugin-import(no-self-import): module importing itself is not allowed")] +#[diagnostic(severity(warning))] +struct NoSelfImportDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoSelfImport; + +declare_oxc_lint!( + /// ### What it does + /// + /// Forbid a module from importing itself. This can sometimes happen during refactoring. + /// + /// ### Example + /// + /// ```javascript + /// // foo.js + /// import foo from './foo.js' + /// const foo = require('./foo') + /// ``` + NoSelfImport, + correctness +); + +impl Rule for NoSelfImport { + fn run_once(&self, ctx: &LintContext<'_>) { + let module_record = ctx.semantic().module_record(); + let resolved_absolute_path = &module_record.resolved_absolute_path; + for (request, spans) in &module_record.requested_modules { + let Some(remote_module_record_ref) = module_record.loaded_modules.get(request) else { + continue; + }; + if remote_module_record_ref.value().resolved_absolute_path == *resolved_absolute_path { + for span in spans { + ctx.diagnostic(NoSelfImportDiagnostic(*span)); + } + } + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let mut tester = Tester::new_without_config::(NoSelfImport::NAME, vec![], vec![]) + .with_import_plugin(true); + + { + let pass = vec![ + "import _ from 'lodash'", + "import find from 'lodash.find'", + "import foo from './foo'", + "import foo from '../foo'", + "import foo from 'foo'", + "import foo from './'", + "import foo from '@scope/foo'", + "var _ = require('lodash')", + "var find = require('lodash.find')", + "var foo = require('./foo')", + "var foo = require('../foo')", + "var foo = require('foo')", + "var foo = require('./')", + "var foo = require('@scope/foo')", + "var bar = require('./bar/index')", + ]; + + let fail = vec![ + "import bar from './no-self-import'", + "var bar = require('./no-self-import')", + "var bar = require('./no-self-import.js')", + ]; + + tester = tester.change_rule_path("no-self-import.js").update_expect_pass_fail(pass, fail); + tester.test(); + } + + { + let pass = vec!["var bar = require('./bar')"]; + let fail = vec![]; + + tester = tester.change_rule_path("bar/index.js").update_expect_pass_fail(pass, fail); + tester.test(); + } + + { + let pass = vec![]; + let fail = vec![ + "var bar = require('.')", + "var bar = require('./')", + "var bar = require('././././')", + ]; + + tester = tester.change_rule_path("index.js").update_expect_pass_fail(pass, fail); + tester.test(); + } + + { + let pass = vec![]; + let fail = vec!["var bar = require('../no-self-import-folder')"]; + + tester = tester + .change_rule_path("no-self-import-folder/index.js") + .update_expect_pass_fail(pass, fail); + tester.test_and_snapshot(); + } +} diff --git a/crates/oxc_linter/src/service.rs b/crates/oxc_linter/src/service.rs index 7bd37c4c4b72b..62ab755bf160f 100644 --- a/crates/oxc_linter/src/service.rs +++ b/crates/oxc_linter/src/service.rs @@ -192,7 +192,7 @@ impl Runtime { let semantic_builder = SemanticBuilder::new(source_text, source_type) .with_trivias(ret.trivias) .with_check_syntax_error(check_syntax_errors) - .build_module_record(program); + .build_module_record(path.to_path_buf(), program); let module_record = semantic_builder.module_record(); if self.linter.options().import_plugin { diff --git a/crates/oxc_linter/src/snapshots/no_self_import.snap b/crates/oxc_linter/src/snapshots/no_self_import.snap new file mode 100644 index 0000000000000..393e5a7a02b84 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_self_import.snap @@ -0,0 +1,47 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: no_self_import +--- + ⚠ eslint-plugin-import(no-self-import): module importing itself is not allowed + ╭─[/Users/boshen/github/oxc/crates/oxc_linter/fixtures/import/no-self-import.js:1:1] + 1 │ import bar from './no-self-import' + · ────────────────── + ╰──── + + ⚠ eslint-plugin-import(no-self-import): module importing itself is not allowed + ╭─[/Users/boshen/github/oxc/crates/oxc_linter/fixtures/import/no-self-import.js:1:1] + 1 │ var bar = require('./no-self-import') + · ────────────────── + ╰──── + + ⚠ eslint-plugin-import(no-self-import): module importing itself is not allowed + ╭─[/Users/boshen/github/oxc/crates/oxc_linter/fixtures/import/no-self-import.js:1:1] + 1 │ var bar = require('./no-self-import.js') + · ───────────────────── + ╰──── + + ⚠ eslint-plugin-import(no-self-import): module importing itself is not allowed + ╭─[/Users/boshen/github/oxc/crates/oxc_linter/fixtures/import/index.js:1:1] + 1 │ var bar = require('.') + · ─── + ╰──── + + ⚠ eslint-plugin-import(no-self-import): module importing itself is not allowed + ╭─[/Users/boshen/github/oxc/crates/oxc_linter/fixtures/import/index.js:1:1] + 1 │ var bar = require('./') + · ──── + ╰──── + + ⚠ eslint-plugin-import(no-self-import): module importing itself is not allowed + ╭─[/Users/boshen/github/oxc/crates/oxc_linter/fixtures/import/index.js:1:1] + 1 │ var bar = require('././././') + · ────────── + ╰──── + + ⚠ eslint-plugin-import(no-self-import): module importing itself is not allowed + ╭─[/Users/boshen/github/oxc/crates/oxc_linter/fixtures/import/no-self-import-folder/index.js:1:1] + 1 │ var bar = require('../no-self-import-folder') + · ────────────────────────── + ╰──── + + diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs index fe6c8dfd5e2cb..082852aa6c8e5 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/crates/oxc_linter/src/tester.rs @@ -61,8 +61,23 @@ impl Tester { Self::new(rule_name, expect_pass, expect_fail) } - /// Change the file name extension - pub fn with_rule_path_extension(mut self, extension: &str) -> Self { + pub fn update_expect_pass_fail>( + mut self, + expect_pass: Vec, + expect_fail: Vec, + ) -> Self { + self.expect_pass = expect_pass.into_iter().map(|s| (s.into(), None)).collect::>(); + self.expect_fail = expect_fail.into_iter().map(|s| (s.into(), None)).collect::>(); + self + } + + /// Change the path + pub fn change_rule_path(mut self, path: &str) -> Self { + self.rule_path = self.current_working_directory.join(path); + self + } + /// Change the file extension + pub fn change_rule_path_extension(mut self, extension: &str) -> Self { self.rule_path.set_extension(extension); self } @@ -89,6 +104,13 @@ impl Tester { self.snapshot(); } + pub fn snapshot(&self) { + let name = self.rule_name.replace('-', "_"); + insta::with_settings!({ prepend_module_to_snapshot => false, }, { + insta::assert_snapshot!(name.clone(), self.snapshot, &name); + }); + } + fn test_pass(&mut self) { for (test, config) in self.expect_pass.clone() { let result = self.run(&test, config, false); @@ -116,13 +138,6 @@ impl Tester { } } - fn snapshot(&self) { - let name = self.rule_name.replace('-', "_"); - insta::with_settings!({ prepend_module_to_snapshot => false, }, { - insta::assert_snapshot!(name.clone(), self.snapshot, &name); - }); - } - fn run(&mut self, source_text: &str, config: Option, is_fix: bool) -> TestResult { let allocator = Allocator::default(); let rule = self.find_rule().read_json(config); diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 169b46e7eb9aa..300abbdbe708a 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -1,6 +1,6 @@ //! Semantic Builder -use std::{cell::RefCell, rc::Rc, sync::Arc}; +use std::{cell::RefCell, path::PathBuf, rc::Rc, sync::Arc}; use itertools::Itertools; #[allow(clippy::wildcard_imports)] @@ -125,8 +125,12 @@ impl<'a> SemanticBuilder<'a> { /// Build the module record with a shallow AST visit #[must_use] - pub fn build_module_record(mut self, program: &'a Program<'a>) -> Self { - let mut module_record_builder = ModuleRecordBuilder::default(); + pub fn build_module_record( + mut self, + resolved_absolute_path: PathBuf, + program: &'a Program<'a>, + ) -> Self { + let mut module_record_builder = ModuleRecordBuilder::new(resolved_absolute_path); module_record_builder.visit(program); self.module_record = Arc::new(module_record_builder.build()); self diff --git a/crates/oxc_semantic/src/module_record/builder.rs b/crates/oxc_semantic/src/module_record/builder.rs index 914562fc6ca67..9a92ed3e5e7c2 100644 --- a/crates/oxc_semantic/src/module_record/builder.rs +++ b/crates/oxc_semantic/src/module_record/builder.rs @@ -1,16 +1,22 @@ +use std::path::PathBuf; + #[allow(clippy::wildcard_imports)] use oxc_ast::{ast::*, syntax_directed_operations::BoundNames}; use oxc_span::{Atom, GetSpan, Span}; #[allow(clippy::wildcard_imports)] use oxc_syntax::module_record::*; -#[derive(Debug, Default)] +#[derive(Default)] pub struct ModuleRecordBuilder { pub module_record: ModuleRecord, export_entries: Vec, } impl ModuleRecordBuilder { + pub fn new(resolved_absolute_path: PathBuf) -> Self { + Self { module_record: ModuleRecord::new(resolved_absolute_path), ..Self::default() } + } + pub fn visit(&mut self, program: &Program) { // This avoids additional checks on TypeScript `TsModuleBlock` which // also has `ModuleDeclaration`s. diff --git a/crates/oxc_semantic/src/module_record/mod.rs b/crates/oxc_semantic/src/module_record/mod.rs index 5f1e4277eb394..72067d9b36d28 100644 --- a/crates/oxc_semantic/src/module_record/mod.rs +++ b/crates/oxc_semantic/src/module_record/mod.rs @@ -9,7 +9,7 @@ mod module_record_tests { use oxc_span::{SourceType, Span}; #[allow(clippy::wildcard_imports)] use oxc_syntax::module_record::*; - use std::sync::Arc; + use std::{path::PathBuf, sync::Arc}; use crate::SemanticBuilder; @@ -20,7 +20,7 @@ mod module_record_tests { let program = allocator.alloc(ret.program); let semantic_ret = SemanticBuilder::new(source_text, source_type) .with_trivias(ret.trivias) - .build_module_record(program) + .build_module_record(PathBuf::new(), program) .build(program); Arc::clone(&semantic_ret.semantic.module_record) } diff --git a/crates/oxc_semantic/tests/util/mod.rs b/crates/oxc_semantic/tests/util/mod.rs index e6b4619e4b098..4b087a281f351 100644 --- a/crates/oxc_semantic/tests/util/mod.rs +++ b/crates/oxc_semantic/tests/util/mod.rs @@ -1,4 +1,4 @@ -use std::sync::Arc; +use std::{path::PathBuf, sync::Arc}; use itertools::Itertools; use oxc_allocator::Allocator; @@ -72,7 +72,7 @@ impl SemanticTester { let semantic_ret = SemanticBuilder::new(self.source_text, self.source_type) .with_check_syntax_error(true) .with_trivias(parse.trivias) - .build_module_record(program) + .build_module_record(PathBuf::new(), program) .build(program); if !semantic_ret.errors.is_empty() { diff --git a/crates/oxc_syntax/src/module_record.rs b/crates/oxc_syntax/src/module_record.rs index 7ffe73d9003da..c00915923a6c0 100644 --- a/crates/oxc_syntax/src/module_record.rs +++ b/crates/oxc_syntax/src/module_record.rs @@ -1,6 +1,6 @@ //! [ECMAScript Module Record](https://tc39.es/ecma262/#sec-abstract-module-records) -use std::{hash::BuildHasherDefault, sync::Arc}; +use std::{hash::BuildHasherDefault, path::PathBuf, sync::Arc}; use dashmap::DashMap; use indexmap::IndexMap; @@ -12,8 +12,11 @@ use rustc_hash::{FxHashMap, FxHasher}; /// See /// * /// * -#[derive(Debug, Default)] +#[derive(Default)] pub struct ModuleRecord { + /// Resolved absolute path to this module record + pub resolved_absolute_path: PathBuf, + /// `[[RequestedModules]]` /// /// A List of all the ModuleSpecifier strings used by the module represented by this record to request the importation of a module. The List is in source text occurrence order. @@ -63,6 +66,12 @@ pub struct ModuleRecord { pub export_default_duplicated: Vec, } +impl ModuleRecord { + pub fn new(resolved_absolute_path: PathBuf) -> Self { + Self { resolved_absolute_path, ..Self::default() } + } +} + #[derive(Debug, Clone, PartialEq, Eq)] pub struct NameSpan { name: Atom, diff --git a/tasks/benchmark/benches/semantic.rs b/tasks/benchmark/benches/semantic.rs index 3b69130f7bd6e..7c5d28874a016 100644 --- a/tasks/benchmark/benches/semantic.rs +++ b/tasks/benchmark/benches/semantic.rs @@ -12,6 +12,7 @@ use oxc_parser::Parser; use oxc_semantic::SemanticBuilder; use oxc_span::SourceType; use oxc_tasks_common::TestFiles; +use std::path::PathBuf; fn bench_semantic(criterion: &mut Criterion) { let mut group = criterion.benchmark_group("semantic"); @@ -26,7 +27,7 @@ fn bench_semantic(criterion: &mut Criterion) { let program = allocator.alloc(ret.program); b.iter_with_large_drop(|| { SemanticBuilder::new(source_text, source_type) - .build_module_record(program) + .build_module_record(PathBuf::new(), program) .build(program) }); }, diff --git a/tasks/coverage/src/suite.rs b/tasks/coverage/src/suite.rs index edba8b6dc7365..371f624e3d394 100644 --- a/tasks/coverage/src/suite.rs +++ b/tasks/coverage/src/suite.rs @@ -303,7 +303,7 @@ pub trait Case: Sized + Sync + Send + UnwindSafe { let semantic_ret = SemanticBuilder::new(source_text, source_type) .with_trivias(parser_ret.trivias) .with_check_syntax_error(true) - .build_module_record(program) + .build_module_record(PathBuf::new(), program) .build(program); if let Some(res) = self.check_semantic(&semantic_ret.semantic) { return res;