From 3a552812cd8a273959dd66cf6bf18859bc8769df Mon Sep 17 00:00:00 2001 From: Victorien Elvinger Date: Sat, 14 Sep 2024 19:10:55 +0200 Subject: [PATCH] fix: ignore non-valid package name, bun imports, and recognize Definitely Typed --- CHANGELOG.md | 4 +- .../correctness/no_undeclared_dependencies.rs | 116 ++++++++++++++---- .../valid.package.json | 3 +- .../{valid.js.snap => valid.ts} | 15 +-- .../{valid.js => valid.ts.snap} | 17 ++- 5 files changed, 121 insertions(+), 34 deletions(-) rename crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/{valid.js.snap => valid.ts} (78%) rename crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/{valid.js => valid.ts.snap} (63%) diff --git a/CHANGELOG.md b/CHANGELOG.md index b96eb7793708..cd3fbd9d8c77 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,7 +35,9 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b #### Bug fixes -- [useSemanticElements](https://biomejs.dev/linter/rules/use-semantic-elements/): ignore `alert` and `alertdialog` roles ([3858](https://github.com/biomejs/biome/issues/3858)). Controbuted by @Conaclos +- [useSemanticElements](https://biomejs.dev/linter/rules/use-semantic-elements/) now ignores `alert` and `alertdialog` roles ([3858](https://github.com/biomejs/biome/issues/3858)). Contributed by @Conaclos + +- [noUndeclaredDependencies](https://biomejs.dev/linter/rules/no-undeclared-dependencies/) now ignores `@/` imports and recognizes type imports from Definitely Typed and `bun` imports. Contributed by @Conaclos ### Parser diff --git a/crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs b/crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs index 2f9c1a540270..674ea45ca842 100644 --- a/crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs +++ b/crates/biome_js_analyze/src/lint/correctness/no_undeclared_dependencies.rs @@ -1,16 +1,18 @@ use crate::services::manifest::Manifest; use biome_analyze::{context::RuleContext, declare_lint_rule, Rule, RuleDiagnostic}; use biome_console::markup; -use biome_js_syntax::AnyJsImportLike; +use biome_js_syntax::{AnyJsImportClause, AnyJsImportLike}; use biome_rowan::AstNode; declare_lint_rule! { /// Disallow the use of dependencies that aren't specified in the `package.json`. /// - /// Indirect dependencies will trigger the rule because they aren't declared in the `package.json`. This means that if package `@org/foo` has a dependency on `lodash`, and then you use + /// Indirect dependencies will trigger the rule because they aren't declared in the `package.json`. + /// This means that if the package `@org/foo` has a dependency on `lodash`, and then you use /// `import "lodash"` somewhere in your project, the rule will trigger a diagnostic for this import. /// - /// The rule ignores imports using a protocol such as `node:`, `bun:`, `jsr:`, `https:`. + /// The rule ignores imports that are not valid package names. + /// This includes internal imports that start with `#` and `@/` and imports with a protocol such as `node:`, `bun:`, `jsr:`, `https:`. /// /// To ensure that Visual Studio Code uses relative imports when it automatically imports a variable, /// you may set [`javascript.preferences.importModuleSpecifier` and `typescript.preferences.importModuleSpecifier`](https://code.visualstudio.com/docs/getstarted/settings) to `relative`. @@ -53,36 +55,34 @@ impl Rule for NoUndeclaredDependencies { } let token_text = node.inner_string_text()?; - let text = token_text.text(); - - // Ignore relative path imports - // Ignore imports using a protocol such as `node:`, `bun:`, `jsr:`, `https:`, and so on. - if text.starts_with('.') || text.contains(':') { - return None; - } - - let mut parts = text.split('/'); - let mut pointer = 0; - if let Some(maybe_scope) = parts.next() { - pointer += maybe_scope.len(); - if maybe_scope.starts_with('@') { - // scoped package: @mui/material/Button - // the package name is @mui/material, not @mui - pointer += parts.next().map_or(0, |s| s.len() + 1) - } - } - let package_name = &text[..pointer]; - + let package_name = parse_package_name(token_text.text())?; if ctx.is_dependency(package_name) || ctx.is_dev_dependency(package_name) || ctx.is_peer_dependency(package_name) || ctx.is_optional_dependency(package_name) // Self package imports - // TODO: we should also check that an `.` exportse exists. + // TODO: we should also check that an `.` exports exists. // See https://nodejs.org/api/packages.html#self-referencing-a-package-using-its-name || ctx.name() == Some(package_name) + // Ignore `bun` import + || package_name == "bun" { return None; + } else if !package_name.starts_with('@') { + // Handle DefinitelyTyped imports https://github.com/DefinitelyTyped/DefinitelyTyped + // e.g. `lodash` can import typ[es from `@types/lodash`. + if let Some(import_clause) = node.parent::() { + if import_clause.type_token().is_some() { + let package_name = format!("@types/{package_name}"); + if ctx.is_dependency(&package_name) + || ctx.is_dev_dependency(&package_name) + || ctx.is_peer_dependency(&package_name) + || ctx.is_optional_dependency(&package_name) + { + return None; + } + } + } } Some(()) @@ -106,3 +106,71 @@ impl Rule for NoUndeclaredDependencies { ) } } + +fn parse_package_name(path: &str) -> Option<&str> { + let mut in_scope = false; + for (i, c) in path.bytes().enumerate() { + match c { + b'@' if i == 0 => { + in_scope = true; + } + // uppercase characters are not allowed in package name + // and a package name cannot start with an underscore. + // Here we are more tolerant and accept them. + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' => {} + b'/' => { + if in_scope { + if i == 1 { + // Invalid empty scope + // `@/` + return None; + } else { + // We consumed the scope. + // `@scope/` + in_scope = false; + } + } else if i == 0 { + // absolute path + return None; + } else { + // We consumed the package name + return Some(&path[..i]); + } + } + _ => { + return None; + } + } + } + // Handle cases where only the scope is given. e.g. `@scope/` + (!path.ends_with('/')).then_some(path) +} + +#[test] +fn test() { + assert_eq!( + parse_package_name("@scope/package-name"), + Some("@scope/package-name") + ); + assert_eq!( + parse_package_name("@scope/package-name/path"), + Some("@scope/package-name") + ); + assert_eq!(parse_package_name("package_"), Some("package_")); + assert_eq!(parse_package_name("package/path"), Some("package")); + assert_eq!(parse_package_name("0"), Some("0")); + assert_eq!(parse_package_name("0/path"), Some("0")); + assert_eq!(parse_package_name("-"), Some("-")); + assert_eq!(parse_package_name("-/path"), Some("-")); + + // Invalid package names that we accept + assert_eq!(parse_package_name("PACKAGE"), Some("PACKAGE")); + assert_eq!(parse_package_name("_"), Some("_")); + + // Invalid package names that we reject + assert_eq!(parse_package_name("@/path"), None); + assert_eq!(parse_package_name("."), None); + assert_eq!(parse_package_name("./path"), None); + assert_eq!(parse_package_name("#path"), None); + assert_eq!(parse_package_name("/path"), None); +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.package.json b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.package.json index 7fd5ccf62082..9a2fe7128c4e 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.package.json +++ b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.package.json @@ -9,7 +9,8 @@ "tailwindcss": "3.4.1" }, "devDependencies": { - "@testing-library/react": "1.0.0" + "@testing-library/react": "1.0.0", + "@types/lodash": "1.0.0" }, "peerDependencies": { "peer-dep": "1.0.0" diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.js.snap b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts similarity index 78% rename from crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.js.snap rename to crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts index 56179a093931..1d703a9f78da 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.js.snap +++ b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts @@ -1,9 +1,3 @@ ---- -source: crates/biome_js_analyze/tests/spec_tests.rs -expression: valid.js ---- -# Input -```jsx import "./valid"; import "react"; import "@testing-library/react"; @@ -26,4 +20,11 @@ import "peer-dep"; import "optional-dep"; import "my-package" -``` + +import "@/internal"; +import "#internal"; + +// import from `@types/jest` +import type * as jest from "lodash"; + +import "bun"; \ No newline at end of file diff --git a/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.js b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts.snap similarity index 63% rename from crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.js rename to crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts.snap index bdf33467bc17..69f753227f16 100644 --- a/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.js +++ b/crates/biome_js_analyze/tests/specs/correctness/noUndeclaredDependencies/valid.ts.snap @@ -1,3 +1,9 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: valid.ts +--- +# Input +```ts import "./valid"; import "react"; import "@testing-library/react"; @@ -19,4 +25,13 @@ import { fontFamily } from "tailwindcss/defaultTheme"; import "peer-dep"; import "optional-dep"; -import "my-package" \ No newline at end of file +import "my-package" + +import "@/internal"; +import "#internal"; + +// import from `@types/jest` +import type * as jest from "lodash"; + +import "bun"; +```