Skip to content

Commit

Permalink
fix(lint_rules): false positives of several rules (#2278)
Browse files Browse the repository at this point in the history
  • Loading branch information
Sec-ant authored Apr 3, 2024
1 parent ca3595f commit 9ba3e9f
Show file tree
Hide file tree
Showing 17 changed files with 107 additions and 20 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

### Linter

#### Bug fixes

- Lint rules `useNodejsImportProtocol`, `useNodeAssertStrict`, `noRestrictedImports`, `noNodejsModules` will no longer check `declare module` statements anymore. Contributed by @Sec-ant

#### New features

#### Enhancements
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ impl Rule for NoNodejsModules {
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let module_name = ctx.query().module_name_token()?;
let node = ctx.query();
if node.is_in_ts_module_declaration() {
return None;
}
let module_name = node.module_name_token()?;
is_node_builtin_module(&inner_string_text(&module_name))
.then_some(module_name.text_trimmed_range())
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,11 @@ impl Rule for NoRestrictedImports {
type Options = Box<RestrictedImportsOptions>;

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let module_name = ctx.query().module_name_token()?;
let node = ctx.query();
if node.is_in_ts_module_declaration() {
return None;
}
let module_name = node.module_name_token()?;
let inner_text = inner_string_text(&module_name);

ctx.options()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use crate::services::manifest::Manifest;
use biome_analyze::{context::RuleContext, declare_rule, Rule, RuleDiagnostic};
use biome_console::markup;
use biome_js_syntax::{AnyJsImportSpecifierLike, TsExternalModuleDeclaration};
use biome_rowan::{AstNode, SyntaxNodeOptionExt};
use biome_js_syntax::AnyJsImportSpecifierLike;
use biome_rowan::AstNode;

declare_rule! {
/// Disallow the use of dependencies that aren't specified in the `package.json`.
Expand Down Expand Up @@ -44,13 +44,8 @@ impl Rule for NoUndeclaredDependencies {

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();

if let Some(parent_syntax_kind) = node.syntax().parent().kind() {
// Ignore module declaration statements:
// declare module "jest";
if TsExternalModuleDeclaration::can_cast(parent_syntax_kind) {
return None;
}
if node.is_in_ts_module_declaration() {
return None;
}

let token_text = node.inner_string_text()?;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,13 @@ impl Rule for UseNodeAssertStrict {

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let node = ctx.query();

let text = node.module_name_token()?;
if inner_string_text(&text) == "node:assert" {
return Some(text);
if node.is_in_ts_module_declaration() {
return None;
}
let module_name = node.module_name_token()?;
if inner_string_text(&module_name) == "node:assert" {
return Some(module_name);
}

None
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ impl Rule for UseNodejsImportProtocol {
type Options = ();

fn run(ctx: &RuleContext<Self>) -> Self::Signals {
let module_name = ctx.query().module_name_token()?;
let node = ctx.query();
if node.is_in_ts_module_declaration() {
return None;
}
let module_name = node.module_name_token()?;
is_node_module_without_protocol(&inner_string_text(&module_name)).then_some(module_name)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module "node:fs" {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid.ts
---
# Input
```ts
declare module "node:fs" {}
```
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
"noRestrictedImports": {
"level": "error",
"options": {
"paths": {}
"paths": {
"node:fs": "Importing from node:fs is forbidden"
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module "node:fs" {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid.ts
---
# Input
```ts
declare module "node:fs" {}
```
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module "node:assert" {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid.ts
---
# Input
```ts
declare module "node:assert" {}
```
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
declare module "module" {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
expression: valid.ts
---
# Input
```ts
declare module "module" {}
```
37 changes: 35 additions & 2 deletions crates/biome_js_syntax/src/import_ext.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use crate::{
inner_string_text, AnyJsBinding, AnyJsImportClause, AnyJsNamedImportSpecifier,
JsCallExpression, JsImport, JsImportAssertion, JsImportCallExpression, JsModuleSource,
JsSyntaxToken,
JsSyntaxToken, TsExternalModuleDeclaration,
};
use biome_rowan::{declare_node_union, AstNode, SyntaxResult, TokenText};
use biome_rowan::{declare_node_union, AstNode, SyntaxNodeOptionExt, SyntaxResult, TokenText};

impl JsImport {
/// It checks if the source of an import against the string `source_to_check`
Expand Down Expand Up @@ -303,4 +303,37 @@ impl AnyJsImportSpecifierLike {
}
}
}

/// Check whether the js import specifier like is in a ts module declaration:
/// ```ts
/// declare "abc" {}
/// ```
/// ## Examples
///
/// ```
/// use biome_js_factory::make;
/// use biome_js_syntax::{AnyJsImportSpecifierLike, JsSyntaxKind, JsSyntaxToken};
/// use biome_rowan::TriviaPieceKind;
///
/// let module_token = JsSyntaxToken::new_detached(JsSyntaxKind::MODULE_KW, "module", [], []);
/// let module_source = make::js_module_source(make::js_string_literal("foo"));
/// let module_declaration = make::ts_external_module_declaration(module_token, module_source).build();
/// let any_import_specifier = AnyJsImportSpecifierLike::JsModuleSource(module_declaration.source().expect("module source"));
/// assert!(any_import_specifier.is_in_ts_module_declaration());
///
/// let module_source = make::js_module_source(make::js_string_literal("bar"));
/// let any_import_specifier = AnyJsImportSpecifierLike::JsModuleSource(module_source);
/// assert!(!any_import_specifier.is_in_ts_module_declaration());
/// ```
pub fn is_in_ts_module_declaration(&self) -> bool {
// It first has to be a JsModuleSource
if !matches!(self, AnyJsImportSpecifierLike::JsModuleSource(_)) {
return false;
}
// Then test whether its parent is a TsExternalModuleDeclaration
if let Some(parent_syntax_kind) = self.syntax().parent().kind() {
return TsExternalModuleDeclaration::can_cast(parent_syntax_kind);
}
false
}
}
4 changes: 4 additions & 0 deletions website/src/content/docs/internals/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b

### Linter

#### Bug fixes

- Lint rules `useNodejsImportProtocol`, `useNodeAssertStrict`, `noRestrictedImports`, `noNodejsModules` will no longer check `declare module` statements anymore. Contributed by @Sec-ant

#### New features

#### Enhancements
Expand Down

0 comments on commit 9ba3e9f

Please sign in to comment.