Skip to content

Commit

Permalink
feat(linter): eslint-plugin-import/no-self-import
Browse files Browse the repository at this point in the history
 closes #440 #441
  • Loading branch information
Boshen committed Sep 9, 2023
1 parent 4e5f63a commit d942c3b
Show file tree
Hide file tree
Showing 13 changed files with 224 additions and 24 deletions.
4 changes: 3 additions & 1 deletion crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
/// <https://github.com/import-js/eslint-plugin-import>
mod import {
pub mod named;
pub mod no_self_import;
}

mod deepscan {
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/rules/import/named.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
116 changes: 116 additions & 0 deletions crates/oxc_linter/src/rules/import/no_self_import.rs
Original file line number Diff line number Diff line change
@@ -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::<String>(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();
}
}
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
47 changes: 47 additions & 0 deletions crates/oxc_linter/src/snapshots/no_self_import.snap
Original file line number Diff line number Diff line change
@@ -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]
1import 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]
1var 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]
1var 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]
1var 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]
1var 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]
1var 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]
1var bar = require('../no-self-import-folder')
· ──────────────────────────
╰────


33 changes: 24 additions & 9 deletions crates/oxc_linter/src/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<S: Into<String>>(
mut self,
expect_pass: Vec<S>,
expect_fail: Vec<S>,
) -> Self {
self.expect_pass = expect_pass.into_iter().map(|s| (s.into(), None)).collect::<Vec<_>>();
self.expect_fail = expect_fail.into_iter().map(|s| (s.into(), None)).collect::<Vec<_>>();
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
}
Expand All @@ -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);
Expand Down Expand Up @@ -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<Value>, is_fix: bool) -> TestResult {
let allocator = Allocator::default();
let rule = self.find_rule().read_json(config);
Expand Down
10 changes: 7 additions & 3 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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
Expand Down
8 changes: 7 additions & 1 deletion crates/oxc_semantic/src/module_record/builder.rs
Original file line number Diff line number Diff line change
@@ -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<ExportEntry>,
}

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.
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/src/module_record/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)
}
Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/tests/util/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::sync::Arc;
use std::{path::PathBuf, sync::Arc};

use itertools::Itertools;
use oxc_allocator::Allocator;
Expand Down Expand Up @@ -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() {
Expand Down
13 changes: 11 additions & 2 deletions crates/oxc_syntax/src/module_record.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -12,8 +12,11 @@ use rustc_hash::{FxHashMap, FxHasher};
/// See
/// * <https://tc39.es/ecma262/#table-additional-fields-of-source-text-module-records>
/// * <https://tc39.es/ecma262/#cyclic-module-record>
#[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.
Expand Down Expand Up @@ -63,6 +66,12 @@ pub struct ModuleRecord {
pub export_default_duplicated: Vec<Span>,
}

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,
Expand Down
3 changes: 2 additions & 1 deletion tasks/benchmark/benches/semantic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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)
});
},
Expand Down
2 changes: 1 addition & 1 deletion tasks/coverage/src/suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit d942c3b

Please sign in to comment.