From 290227874857906770ff87352b3c45c65237441f Mon Sep 17 00:00:00 2001 From: kaykdm Date: Fri, 5 Jan 2024 20:43:32 +0900 Subject: [PATCH 1/6] feat(linter): implement @typescript-eslint/triple-slash-reference --- crates/oxc_linter/src/rules.rs | 2 + .../typescript/triple_slash_reference.rs | 275 ++++++++++++++++++ .../src/snapshots/triple_slash_reference.snap | 44 +++ 3 files changed, 321 insertions(+) create mode 100644 crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs create mode 100644 crates/oxc_linter/src/snapshots/triple_slash_reference.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index dcbc377a4759a..78a3a3c9f29aa 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -109,6 +109,7 @@ mod typescript { pub mod no_unsafe_declaration_merging; pub mod no_var_requires; pub mod prefer_as_const; + pub mod triple_slash_reference; } mod jest { @@ -353,6 +354,7 @@ oxc_macros::declare_all_lint_rules! { typescript::no_unsafe_declaration_merging, typescript::no_var_requires, typescript::prefer_as_const, + typescript::triple_slash_reference, jest::expect_expect, jest::max_expects, jest::no_alias_methods, diff --git a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs new file mode 100644 index 0000000000000..1b94f7ed4b3b2 --- /dev/null +++ b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs @@ -0,0 +1,275 @@ +use std::collections::HashSet; + +use oxc_ast::{ + ast::{ModuleDeclaration, TSModuleReference}, + AstKind, +}; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::{self, Error}, +}; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use regex::Regex; + +use crate::{context::LintContext, rule::Rule}; + +#[derive(Debug, Error, Diagnostic)] +#[error("typescript-eslint(triple-slash-reference): Do not use a triple slash reference for {0}, use `import` style instead.")] +#[diagnostic(severity(warning), help("Use of triple-slash reference type directives is generally discouraged in favor of ECMAScript Module imports."))] +struct TripleSlashReferenceDiagnostic(String, #[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct TripleSlashReference(Box); + +#[derive(Debug, Clone, Default)] +pub struct TripleSlashReferenceConfig { + lib: LibOption, + path: PathOption, + types: TypesOption, +} +#[derive(Debug, Default, Clone, PartialEq)] +enum LibOption { + #[default] + Always, + Never, +} +#[derive(Debug, Default, Clone, PartialEq)] +enum PathOption { + Always, + #[default] + Never, +} +#[derive(Debug, Default, Clone, PartialEq)] +enum TypesOption { + Always, + Never, + #[default] + PreferImport, +} + +impl std::ops::Deref for TripleSlashReference { + type Target = TripleSlashReferenceConfig; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +declare_oxc_lint!( + /// ### What it does + /// Disallow certain triple slash directives in favor of ES6-style import declarations. + /// + /// ### Why is this bad? + /// Use of triple-slash reference type directives is generally discouraged in favor of ECMAScript Module imports. + /// + /// ### Example + /// ```javascript + /// /// + /// globalThis.value; + /// ``` + TripleSlashReference, + correctness +); + +impl Rule for TripleSlashReference { + fn from_configuration(value: serde_json::Value) -> Self { + let options: Option<&serde_json::Value> = value.get(0); + Self(Box::new(TripleSlashReferenceConfig { + lib: options + .and_then(|x| x.get("lib")) + .and_then(serde_json::Value::as_str) + .map_or_else(LibOption::default, |value| match value { + "always" => LibOption::Always, + "never" => LibOption::Never, + _ => LibOption::default(), + }), + path: options + .and_then(|x| x.get("path")) + .and_then(serde_json::Value::as_str) + .map_or_else(PathOption::default, |value| match value { + "always" => PathOption::Always, + "never" => PathOption::Never, + _ => PathOption::default(), + }), + types: options + .and_then(|x| x.get("types")) + .and_then(serde_json::Value::as_str) + .map_or_else(TypesOption::default, |value| match value { + "always" => TypesOption::Always, + "never" => TypesOption::Never, + "prefer-import" => TypesOption::PreferImport, + _ => TypesOption::default(), + }), + })) + } + fn run_once(&self, ctx: &LintContext) { + let mut import_source_set = HashSet::new(); + for node in ctx.nodes().iter() { + match node.kind() { + AstKind::ModuleDeclaration(ModuleDeclaration::ImportDeclaration(import)) => { + import_source_set.insert(import.source.value.as_str()); + } + AstKind::TSImportEqualsDeclaration(decl) => match *decl.module_reference { + TSModuleReference::ExternalModuleReference(ref mod_ref) => { + import_source_set.insert(mod_ref.expression.value.as_str()); + } + TSModuleReference::TypeName(_) => {} + }, + _ => {} + } + } + + let reference_regexp = + Regex::new(r#"^\/\s* + // + // + import * as foo from 'foo'; + import * as bar from 'bar'; + import * as baz from 'baz'; + "#, + Some(serde_json::json!([{ "path": "never", "types": "never", "lib": "never" }])), + ), + ( + r#" + // + // + // + import foo = require('foo'); + import bar = require('bar'); + import baz = require('baz'); + "#, + Some(serde_json::json!([{ "path": "never", "types": "never", "lib": "never" }])), + ), + ( + r#" + /// + /// + /// + import * as foo from 'foo'; + import * as bar from 'bar'; + import * as baz from 'baz'; + "#, + Some(serde_json::json!([{ "path": "always", "types": "always", "lib": "always" }])), + ), + ( + r#" + /// + /// + /// + import foo = require('foo'); + import bar = require('bar'); + import baz = require('baz'); + "#, + Some(serde_json::json!([{ "path": "always", "types": "always", "lib": "always" }])), + ), + ( + r#" + /// + /// + /// + import foo = foo; + import bar = bar; + import baz = baz; + "#, + Some(serde_json::json!([{ "path": "always", "types": "always", "lib": "always" }])), + ), + ( + r#" + /// + /// + /// + import foo = foo.foo; + import bar = bar.bar.bar.bar; + import baz = baz.baz; + "#, + Some(serde_json::json!([{ "path": "always", "types": "always", "lib": "always" }])), + ), + (r"import * as foo from 'foo';", Some(serde_json::json!([{ "path": "never" }]))), + (r"import foo = require('foo');", Some(serde_json::json!([{ "path": "never" }]))), + (r"import * as foo from 'foo';", Some(serde_json::json!([{ "types": "never" }]))), + (r"import foo = require('foo');", Some(serde_json::json!([{ "types": "never" }]))), + (r"import * as foo from 'foo';", Some(serde_json::json!([{ "lib": "never" }]))), + (r"import foo = require('foo');", Some(serde_json::json!([{ "lib": "never" }]))), + (r"import * as foo from 'foo';", Some(serde_json::json!([{ "types": "prefer-import" }]))), + (r"import foo = require('foo');", Some(serde_json::json!([{ "types": "prefer-import" }]))), + ( + r#" + /// + import * as bar from 'bar'; + "#, + Some(serde_json::json!([{ "types": "prefer-import" }])), + ), + ( + r#" + /* + /// + */ + import * as foo from 'foo'; + "#, + Some(serde_json::json!([{ "path": "never", "types": "never", "lib": "never" }])), + ), + ]; + + let fail = vec![ + ( + r#" + /// + import * as foo from 'foo'; + "#, + Some(serde_json::json!([{ "types": "prefer-import" }])), + ), + ( + r#" + /// + import foo = require('foo'); + "#, + Some(serde_json::json!([{ "types": "prefer-import" }])), + ), + (r#"/// "#, Some(serde_json::json!([{ "path": "never" }]))), + (r#"/// "#, Some(serde_json::json!([{ "types": "never" }]))), + (r#"/// "#, Some(serde_json::json!([{ "lib": "never" }]))), + ]; + + Tester::new(TripleSlashReference::NAME, pass, fail).test_and_snapshot(); +} diff --git a/crates/oxc_linter/src/snapshots/triple_slash_reference.snap b/crates/oxc_linter/src/snapshots/triple_slash_reference.snap new file mode 100644 index 0000000000000..a94660bd885f6 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/triple_slash_reference.snap @@ -0,0 +1,44 @@ +--- +source: crates/oxc_linter/src/tester.rs +expression: triple_slash_reference +--- + ⚠ typescript-eslint(triple-slash-reference): Do not use a triple slash reference for foo, use `import` style instead. + ╭─[triple_slash_reference.tsx:1:1] + 1 │ + 2 │ /// + · ───────────────────────────── + 3 │ import * as foo from 'foo'; + ╰──── + help: Use of triple-slash reference type directives is generally discouraged in favor of ECMAScript Module imports. + + ⚠ typescript-eslint(triple-slash-reference): Do not use a triple slash reference for foo, use `import` style instead. + ╭─[triple_slash_reference.tsx:1:1] + 1 │ + 2 │ /// + · ───────────────────────────── + 3 │ import foo = require('foo'); + ╰──── + help: Use of triple-slash reference type directives is generally discouraged in favor of ECMAScript Module imports. + + ⚠ typescript-eslint(triple-slash-reference): Do not use a triple slash reference for foo, use `import` style instead. + ╭─[triple_slash_reference.tsx:1:1] + 1 │ /// + · ──────────────────────────── + ╰──── + help: Use of triple-slash reference type directives is generally discouraged in favor of ECMAScript Module imports. + + ⚠ typescript-eslint(triple-slash-reference): Do not use a triple slash reference for foo, use `import` style instead. + ╭─[triple_slash_reference.tsx:1:1] + 1 │ /// + · ───────────────────────────── + ╰──── + help: Use of triple-slash reference type directives is generally discouraged in favor of ECMAScript Module imports. + + ⚠ typescript-eslint(triple-slash-reference): Do not use a triple slash reference for foo, use `import` style instead. + ╭─[triple_slash_reference.tsx:1:1] + 1 │ /// + · ─────────────────────────── + ╰──── + help: Use of triple-slash reference type directives is generally discouraged in favor of ECMAScript Module imports. + + From 9e7557c531107eb2c9971d7bffbe0f231265cbbe Mon Sep 17 00:00:00 2001 From: kaykdm Date: Fri, 5 Jan 2024 22:34:00 +0900 Subject: [PATCH 2/6] Use once_cell for regex --- .../src/rules/typescript/triple_slash_reference.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs index 1b94f7ed4b3b2..967308c9e1d2f 100644 --- a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs +++ b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; +use once_cell::sync::Lazy; use oxc_ast::{ ast::{ModuleDeclaration, TSModuleReference}, AstKind, @@ -14,6 +15,10 @@ use regex::Regex; use crate::{context::LintContext, rule::Rule}; +static REFERENCE_REGEX: Lazy = Lazy::new(|| { + Regex::new(r#"^\/\s* Date: Sat, 6 Jan 2024 19:32:57 +0900 Subject: [PATCH 3/6] Use program.body instead of ctx.nodes() --- .../typescript/triple_slash_reference.rs | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs index 967308c9e1d2f..c7c13dd06a945 100644 --- a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs +++ b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs @@ -2,7 +2,7 @@ use std::collections::HashSet; use once_cell::sync::Lazy; use oxc_ast::{ - ast::{ModuleDeclaration, TSModuleReference}, + ast::{Declaration, ModuleDeclaration, Statement, TSModuleReference}, AstKind, }; use oxc_diagnostics::{ @@ -109,17 +109,25 @@ impl Rule for TripleSlashReference { })) } fn run_once(&self, ctx: &LintContext) { + let Some(root) = ctx.nodes().iter().next() else { return }; + let AstKind::Program(program) = root.kind() else { return }; let mut import_source_set = HashSet::new(); - for node in ctx.nodes().iter() { - match node.kind() { - AstKind::ModuleDeclaration(ModuleDeclaration::ImportDeclaration(import)) => { - import_source_set.insert(import.source.value.as_str()); + + for stmt in &program.body { + match stmt { + Statement::Declaration(Declaration::TSImportEqualsDeclaration(decl)) => { + match *decl.module_reference { + TSModuleReference::ExternalModuleReference(ref mod_ref) => { + import_source_set.insert(mod_ref.expression.value.as_str()); + } + TSModuleReference::TypeName(_) => {} + } } - AstKind::TSImportEqualsDeclaration(decl) => match *decl.module_reference { - TSModuleReference::ExternalModuleReference(ref mod_ref) => { - import_source_set.insert(mod_ref.expression.value.as_str()); + Statement::ModuleDeclaration(st) => match **st { + ModuleDeclaration::ImportDeclaration(ref decl) => { + import_source_set.insert(decl.source.value.as_str()); } - TSModuleReference::TypeName(_) => {} + _ => {} }, _ => {} } From a1131b1d0b39b46627c53099474686a12c11660d Mon Sep 17 00:00:00 2001 From: kaykdm Date: Sat, 6 Jan 2024 21:29:22 +0900 Subject: [PATCH 4/6] Use range to iterate comments --- .../rules/typescript/triple_slash_reference.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs index c7c13dd06a945..b6312a54f45e4 100644 --- a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs +++ b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs @@ -10,7 +10,7 @@ use oxc_diagnostics::{ thiserror::{self, Error}, }; use oxc_macros::declare_oxc_lint; -use oxc_span::Span; +use oxc_span::{GetSpan, Span}; use regex::Regex; use crate::{context::LintContext, rule::Rule}; @@ -123,18 +123,21 @@ impl Rule for TripleSlashReference { TSModuleReference::TypeName(_) => {} } } - Statement::ModuleDeclaration(st) => match **st { - ModuleDeclaration::ImportDeclaration(ref decl) => { + Statement::ModuleDeclaration(st) => { + if let ModuleDeclaration::ImportDeclaration(ref decl) = **st { import_source_set.insert(decl.source.value.as_str()); } - _ => {} - }, + } _ => {} } } + // We don't need to iterate over all comments since Triple-slash directives are only valid at the top of their containing file. + // We are trying to get the first statement start potioin, falling back to the program end if statement does not exist + let comments_range_end = program.body.first().map_or(program.span.end, |v| v.span().start); + let comments = ctx.semantic().trivias().comments(); - for (start, comment) in comments { + for (start, comment) in comments.range(0..comments_range_end) { let raw = &ctx.semantic().source_text()[*start as usize..comment.end() as usize]; if let Some(captures) = REFERENCE_REGEX.captures(raw) { let group1 = captures.get(1).map_or("", |m| m.as_str()); From ab93d0445ba6aae8a558bf1f4aa5c967b8a0d75c Mon Sep 17 00:00:00 2001 From: kaykdm Date: Sat, 6 Jan 2024 22:49:28 +0900 Subject: [PATCH 5/6] Iterate comments first and then check import --- .../typescript/triple_slash_reference.rs | 68 +++++++++++-------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs index b6312a54f45e4..5cb5060db2d4b 100644 --- a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs +++ b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs @@ -1,4 +1,4 @@ -use std::collections::HashSet; +use std::collections::HashMap; use once_cell::sync::Lazy; use oxc_ast::{ @@ -111,32 +111,13 @@ impl Rule for TripleSlashReference { fn run_once(&self, ctx: &LintContext) { let Some(root) = ctx.nodes().iter().next() else { return }; let AstKind::Program(program) = root.kind() else { return }; - let mut import_source_set = HashSet::new(); - - for stmt in &program.body { - match stmt { - Statement::Declaration(Declaration::TSImportEqualsDeclaration(decl)) => { - match *decl.module_reference { - TSModuleReference::ExternalModuleReference(ref mod_ref) => { - import_source_set.insert(mod_ref.expression.value.as_str()); - } - TSModuleReference::TypeName(_) => {} - } - } - Statement::ModuleDeclaration(st) => { - if let ModuleDeclaration::ImportDeclaration(ref decl) = **st { - import_source_set.insert(decl.source.value.as_str()); - } - } - _ => {} - } - } // We don't need to iterate over all comments since Triple-slash directives are only valid at the top of their containing file. // We are trying to get the first statement start potioin, falling back to the program end if statement does not exist let comments_range_end = program.body.first().map_or(program.span.end, |v| v.span().start); - let comments = ctx.semantic().trivias().comments(); + let mut refs_for_import = HashMap::new(); + for (start, comment) in comments.range(0..comments_range_end) { let raw = &ctx.semantic().source_text()[*start as usize..comment.end() as usize]; if let Some(captures) = REFERENCE_REGEX.captures(raw) { @@ -153,14 +134,41 @@ impl Rule for TripleSlashReference { )); } - if group1 == "types" - && self.types == TypesOption::PreferImport - && import_source_set.contains(group2) - { - ctx.diagnostic(TripleSlashReferenceDiagnostic( - group2.to_string(), - Span { start: *start - 2, end: comment.end() }, - )); + if group1 == "types" && self.types == TypesOption::PreferImport { + refs_for_import.insert(group2, Span { start: *start - 2, end: comment.end() }); + } + } + } + + if !refs_for_import.is_empty() { + for stmt in &program.body { + match stmt { + Statement::Declaration(Declaration::TSImportEqualsDeclaration(decl)) => { + match *decl.module_reference { + TSModuleReference::ExternalModuleReference(ref mod_ref) => { + if let Some(v) = + refs_for_import.get(mod_ref.expression.value.as_str()) + { + ctx.diagnostic(TripleSlashReferenceDiagnostic( + mod_ref.expression.value.to_string(), + *v, + )); + } + } + TSModuleReference::TypeName(_) => {} + } + } + Statement::ModuleDeclaration(st) => { + if let ModuleDeclaration::ImportDeclaration(ref decl) = **st { + if let Some(v) = refs_for_import.get(decl.source.value.as_str()) { + ctx.diagnostic(TripleSlashReferenceDiagnostic( + decl.source.value.to_string(), + *v, + )); + } + } + } + _ => {} } } } From dc71ea4dd4343960f1504ba51a03457510132e45 Mon Sep 17 00:00:00 2001 From: kaykdm Date: Mon, 8 Jan 2024 22:40:04 +0900 Subject: [PATCH 6/6] Stop using regex for string extract --- .../typescript/triple_slash_reference.rs | 51 +++++++++++++++---- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs index 5cb5060db2d4b..a58bd7efdf30a 100644 --- a/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs +++ b/crates/oxc_linter/src/rules/typescript/triple_slash_reference.rs @@ -1,6 +1,5 @@ use std::collections::HashMap; -use once_cell::sync::Lazy; use oxc_ast::{ ast::{Declaration, ModuleDeclaration, Statement, TSModuleReference}, AstKind, @@ -11,14 +10,9 @@ use oxc_diagnostics::{ }; use oxc_macros::declare_oxc_lint; use oxc_span::{GetSpan, Span}; -use regex::Regex; use crate::{context::LintContext, rule::Rule}; -static REFERENCE_REGEX: Lazy = Lazy::new(|| { - Regex::new(r#"^\/\s* Option<(String, String)> { + if !raw.starts_with('/') { + return None; + } + + let reference_start = "' after the start index + if let Some(end_idx) = raw[start_idx..].find(reference_end) { + let reference_str = &raw[start_idx + reference_start.len()..start_idx + end_idx]; + + // Split the string by whitespaces + let parts = reference_str.split_whitespace(); + + // Filter parts that start with attribute key pattern + let filtered_parts: Vec<&str> = parts + .into_iter() + .filter(|part| { + part.starts_with("types=") + || part.starts_with("path=") + || part.starts_with("lib=") + }) + .collect(); + + if let Some(attr) = filtered_parts.first() { + // Split the attribute by '=' to get key and value + let attr_parts: Vec<&str> = attr.split('=').collect(); + if attr_parts.len() == 2 { + let key = attr_parts[0].trim().trim_matches('"').to_string(); + let value = attr_parts[1].trim_matches('"').trim_end_matches('/').to_string(); + return Some((key, value)); + } + } + } + } + None +} + #[test] fn test() { use crate::tester::Tester;