Skip to content

Commit

Permalink
fix(linter): useNodejsImportProtocol takes in dependencies in consi…
Browse files Browse the repository at this point in the history
…deration
  • Loading branch information
ematipico committed Jun 25, 2024
1 parent 91522a5 commit 25d474c
Show file tree
Hide file tree
Showing 11 changed files with 397 additions and 332 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b
#### Bug fixes

- `useConsistentArrayType` and `useShorthandArrayType` now ignore `Array` in the `extends` and `implements` clauses. Fix [#3247](https://github.com/biomejs/biome/issues/3247). Contributed by @Conaclos
- Fixes [#3066](https://github.com/biomejs/biome/issues/3066) by taking into account the dependencies declared in the `package.json`. Contributed by @ematipico

## v1.8.2 (2024-06-20)

Expand Down
16 changes: 10 additions & 6 deletions crates/biome_js_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,7 @@ where

services.insert_service(Arc::new(AriaRoles));
services.insert_service(Arc::new(AriaProperties));
if let Some(manifest) = manifest {
services.insert_service(Arc::new(manifest));
}
services.insert_service(Arc::new(manifest));
services.insert_service(source_type);
(
analyzer.run(AnalyzerContext {
Expand Down Expand Up @@ -238,6 +236,7 @@ mod tests {
use biome_diagnostics::{Diagnostic, DiagnosticExt, PrintDiagnostic, Severity};
use biome_js_parser::{parse, JsParserOptions};
use biome_js_syntax::{JsFileSource, TextRange, TextSize};
use biome_project::{Dependencies, PackageJson};
use std::slice;

use crate::lint::correctness::use_exhaustive_dependencies::{Hook, HooksOptions};
Expand All @@ -256,7 +255,7 @@ mod tests {
String::from_utf8(buffer).unwrap()
}

const SOURCE: &str = r#"<div class={`px-2 foo p-4 bar ${variable}`}/>"#;
const SOURCE: &str = r#"import buffer from "buffer"; "#;

let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default());

Expand All @@ -268,13 +267,15 @@ mod tests {
dependencies_index: Some(1),
stable_result: StableHookResult::None,
};
let rule_filter = RuleFilter::Rule("nursery", "useSortedClasses");
let rule_filter = RuleFilter::Rule("style", "useNodejsImportProtocol");

options.configuration.rules.push_rule(
RuleKey::new("nursery", "useHookAtTopLevel"),
RuleOptions::new(HooksOptions { hooks: vec![hook] }, None),
);

let mut dependencies = Dependencies::default();
dependencies.add("buffer", "latest");
analyze(
&parsed.tree(),
AnalysisFilter {
Expand All @@ -283,7 +284,10 @@ mod tests {
},
&options,
JsFileSource::tsx(),
None,
Some(PackageJson {
dependencies,
..Default::default()
}),
|signal| {
if let Some(diag) = signal.diagnostic() {
error_ranges.push(diag.location().span.unwrap());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use biome_analyze::{
context::RuleContext, declare_rule, ActionCategory, Ast, FixKind, Rule, RuleDiagnostic,
RuleSource,
context::RuleContext, declare_rule, ActionCategory, FixKind, Rule, RuleDiagnostic, RuleSource,
};
use biome_console::markup;
use biome_js_syntax::{inner_string_text, AnyJsImportLike, JsSyntaxKind, JsSyntaxToken};
use biome_rowan::BatchMutationExt;

use crate::services::manifest::Manifest;
use crate::{globals::is_node_builtin_module, JsRuleAction};

declare_rule! {
Expand All @@ -14,6 +14,13 @@ declare_rule! {
/// The rule marks traditional imports like `import fs from "fs";` as invalid,
/// suggesting the format `import fs from "node:fs";` instead.
///
/// The rule also isn't triggered if there are dependencies declared in the `package.json` that match
/// the name of a built-in Node.js module.
///
/// :::caution
/// The rule doesn't support dependencies installed inside a monorepo.
/// :::
///
/// ## Examples
///
/// ### Invalid
Expand Down Expand Up @@ -51,7 +58,7 @@ declare_rule! {
}

impl Rule for UseNodejsImportProtocol {
type Query = Ast<AnyJsImportLike>;
type Query = Manifest<AnyJsImportLike>;
type State = JsSyntaxToken;
type Signals = Option<Self::State>;
type Options = ();
Expand All @@ -62,7 +69,15 @@ impl Rule for UseNodejsImportProtocol {
return None;
}
let module_name = node.module_name_token()?;
is_node_module_without_protocol(&inner_string_text(&module_name)).then_some(module_name)
let module_name_trimmed = inner_string_text(&module_name);
if ctx.is_dependency(&module_name_trimmed)
|| ctx.is_dev_dependency(&module_name_trimmed)
|| ctx.is_peer_dependency(&module_name_trimmed)
|| ctx.is_optional_dependency(&module_name_trimmed)
{
return None;
}
is_node_module_without_protocol(&module_name_trimmed).then_some(module_name)
}

fn diagnostic(_: &RuleContext<Self>, module_name: &Self::State) -> Option<RuleDiagnostic> {
Expand Down
28 changes: 22 additions & 6 deletions crates/biome_js_analyze/src/services/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,40 @@ use std::sync::Arc;

#[derive(Debug, Clone)]
pub struct ManifestServices {
pub(crate) manifest: Arc<PackageJson>,
pub(crate) manifest: Arc<Option<PackageJson>>,
}

impl ManifestServices {
pub(crate) fn is_dependency(&self, specifier: &str) -> bool {
self.manifest.dependencies.contains(specifier)
self.manifest
.as_ref()
.as_ref()
.map(|pkg| pkg.dependencies.contains(specifier))
.unwrap_or_default()
}

pub(crate) fn is_dev_dependency(&self, specifier: &str) -> bool {
self.manifest.dev_dependencies.contains(specifier)
self.manifest
.as_ref()
.as_ref()
.map(|pkg| pkg.dev_dependencies.contains(specifier))
.unwrap_or_default()
}

pub(crate) fn is_peer_dependency(&self, specifier: &str) -> bool {
self.manifest.peer_dependencies.contains(specifier)
self.manifest
.as_ref()
.as_ref()
.map(|pkg| pkg.peer_dependencies.contains(specifier))
.unwrap_or_default()
}

pub(crate) fn is_optional_dependency(&self, specifier: &str) -> bool {
self.manifest.optional_dependencies.contains(specifier)
self.manifest
.as_ref()
.as_ref()
.map(|pkg| pkg.optional_dependencies.contains(specifier))
.unwrap_or_default()
}
}

Expand All @@ -35,7 +51,7 @@ impl FromServices for ManifestServices {
rule_key: &RuleKey,
services: &ServiceBag,
) -> biome_diagnostics::Result<Self, MissingServicesDiagnostic> {
let manifest: &Arc<PackageJson> = services.get_service().ok_or_else(|| {
let manifest: &Arc<Option<PackageJson>> = services.get_service().ok_or_else(|| {
MissingServicesDiagnostic::new(rule_key.rule_name(), &["PackageJson"])
})?;

Expand Down
Loading

0 comments on commit 25d474c

Please sign in to comment.