From e7871bf527bfb58eb47c074de0606a47c2e4c191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Mu=C3=B1oz=20Tous?= Date: Sat, 7 Sep 2024 12:43:46 +0200 Subject: [PATCH 1/5] fix(useHookAtTopLevel): avoid diagnostics on non React projects --- CHANGELOG.md | 2 ++ .../lint/correctness/use_hook_at_top_level.rs | 24 +++++++++++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b96eb7793708..b2a6b243a70b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -145,6 +145,8 @@ our [guidelines for writing a good changelog entry](https://github.com/biomejs/b #### Bug fixes +- Fix `useHookAtTopLevel` to avoid reporting diagnostics in non React projects [#3476](https://github.com/biomejs/biome/issues/3476). Contributed by @Javimtib92 + - `biome lint --write` now takes `--only` and `--skip` into account ([#3470](https://github.com/biomejs/biome/issues/3470)). Contributed by @Conaclos - Fix [#3368](https://github.com/biomejs/biome/issues/3368), now the reporter `github` tracks the diagnostics that belong to formatting and organize imports. Contributed by @ematipico diff --git a/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs b/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs index 45143c60e88c..4e916afce42c 100644 --- a/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs +++ b/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs @@ -1,4 +1,5 @@ use crate::react::hooks::{is_react_component, is_react_hook, is_react_hook_call}; +use crate::services::manifest::ManifestServices; use crate::services::semantic::{SemanticModelBuilderVisitor, SemanticServices}; use biome_analyze::RuleSource; use biome_analyze::{ @@ -19,6 +20,7 @@ use biome_js_syntax::{ JsObjectBindingPatternShorthandProperty, JsReturnStatement, JsSyntaxKind, JsSyntaxNode, JsTryFinallyStatement, TextRange, }; +use biome_project::PackageJson; use biome_rowan::{declare_node_union, AstNode, Language, SyntaxNode, WalkEvent}; use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; @@ -231,6 +233,12 @@ fn is_nested_function_inside_component_or_hook(function: &AnyJsFunctionOrMethod) }) } +fn is_within_react_project(manifest: &Option) -> bool { + manifest.as_ref().map_or(false, |package_json| { + package_json.dependencies.contains("react") + }) +} + /// Model for tracking which function calls are preceeded by an early return. /// /// The keys in the model are call sites and each value is the text range of an @@ -330,6 +338,7 @@ impl Visitor for FunctionCallVisitor { pub struct FunctionCallServices { early_returns: EarlyReturnsModel, semantic_services: SemanticServices, + manifest_services: ManifestServices, } impl FunctionCallServices { @@ -340,6 +349,10 @@ impl FunctionCallServices { fn semantic_model(&self) -> &SemanticModel { self.semantic_services.model() } + + fn manifest(&self) -> &Option { + self.manifest_services.manifest.as_ref() + } } impl FromServices for FunctionCallServices { @@ -353,6 +366,7 @@ impl FromServices for FunctionCallServices { Ok(Self { early_returns: early_returns.clone(), semantic_services: SemanticServices::from_services(rule_key, services)?, + manifest_services: ManifestServices::from_services(rule_key, services)?, }) } } @@ -411,8 +425,14 @@ impl Rule for UseHookAtTopLevel { Err(_) => None, }; - // Early return for any function call that's not a hook call: - if !is_react_hook_call(call) { + // Since we can't definitively know if a function is a React hook, we use the following heuristics: + // + // - Check if the function is within the scope of a react project by checking package.json + // - Verify if the call matches the pattern of a React hook + // If any of these checks fail, we assume it's not a React hook call and we return early: + let manifest = ctx.manifest(); + + if !is_within_react_project(manifest) || !is_react_hook_call(call) { return None; } From 571febd0ed4da34535369cba98cca7ff70b96eca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Mu=C3=B1oz=20Tous?= Date: Sun, 8 Sep 2024 19:52:33 +0200 Subject: [PATCH 2/5] add react dependency to specs tests --- .../correctness/useHookAtTopLevel/customHook.package.json | 5 +++++ .../useHookAtTopLevel/deprecatedConfig.package.json | 5 +++++ .../specs/correctness/useHookAtTopLevel/invalid.package.json | 5 +++++ .../specs/correctness/useHookAtTopLevel/valid.package.json | 5 +++++ 4 files changed, 20 insertions(+) create mode 100644 crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/customHook.package.json create mode 100644 crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/deprecatedConfig.package.json create mode 100644 crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.package.json create mode 100644 crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.package.json diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/customHook.package.json b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/customHook.package.json new file mode 100644 index 000000000000..e3bf5e190e28 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/customHook.package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "react": "latest" + } +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/deprecatedConfig.package.json b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/deprecatedConfig.package.json new file mode 100644 index 000000000000..e3bf5e190e28 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/deprecatedConfig.package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "react": "latest" + } +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.package.json b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.package.json new file mode 100644 index 000000000000..e3bf5e190e28 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "react": "latest" + } +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.package.json b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.package.json new file mode 100644 index 000000000000..e3bf5e190e28 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.package.json @@ -0,0 +1,5 @@ +{ + "dependencies": { + "react": "latest" + } +} From b88cd831e0028cbe3582ea40886d29354a33797e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Mu=C3=B1oz=20Tous?= Date: Mon, 9 Sep 2024 16:14:31 +0200 Subject: [PATCH 3/5] default to true when manifest is not found --- .../src/lint/correctness/use_hook_at_top_level.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs b/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs index 4e916afce42c..e21f1dcadf23 100644 --- a/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs +++ b/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs @@ -234,7 +234,7 @@ fn is_nested_function_inside_component_or_hook(function: &AnyJsFunctionOrMethod) } fn is_within_react_project(manifest: &Option) -> bool { - manifest.as_ref().map_or(false, |package_json| { + manifest.as_ref().map_or(true, |package_json| { package_json.dependencies.contains("react") }) } From 0883a467f78b0af9dbe446799e2c2819601cabae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Mu=C3=B1oz=20Tous?= Date: Mon, 9 Sep 2024 17:46:09 +0200 Subject: [PATCH 4/5] revert package.json files from test now that manifest check defaults to true --- .../correctness/useHookAtTopLevel/customHook.package.json | 5 ----- .../useHookAtTopLevel/deprecatedConfig.package.json | 5 ----- .../specs/correctness/useHookAtTopLevel/invalid.package.json | 5 ----- .../specs/correctness/useHookAtTopLevel/valid.package.json | 5 ----- 4 files changed, 20 deletions(-) delete mode 100644 crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/customHook.package.json delete mode 100644 crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/deprecatedConfig.package.json delete mode 100644 crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.package.json delete mode 100644 crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.package.json diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/customHook.package.json b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/customHook.package.json deleted file mode 100644 index e3bf5e190e28..000000000000 --- a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/customHook.package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "dependencies": { - "react": "latest" - } -} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/deprecatedConfig.package.json b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/deprecatedConfig.package.json deleted file mode 100644 index e3bf5e190e28..000000000000 --- a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/deprecatedConfig.package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "dependencies": { - "react": "latest" - } -} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.package.json b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.package.json deleted file mode 100644 index e3bf5e190e28..000000000000 --- a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/invalid.package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "dependencies": { - "react": "latest" - } -} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.package.json b/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.package.json deleted file mode 100644 index e3bf5e190e28..000000000000 --- a/crates/biome_js_analyze/tests/specs/correctness/useHookAtTopLevel/valid.package.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "dependencies": { - "react": "latest" - } -} From b5aeafea57c2707615fcb5a2d5a7a0333d0dabb4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Mu=C3=B1oz=20Tous?= Date: Sat, 14 Sep 2024 11:49:53 +0200 Subject: [PATCH 5/5] check for react presence in dev dependencies --- .../src/lint/correctness/use_hook_at_top_level.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs b/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs index e21f1dcadf23..94231ebec550 100644 --- a/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs +++ b/crates/biome_js_analyze/src/lint/correctness/use_hook_at_top_level.rs @@ -236,6 +236,7 @@ fn is_nested_function_inside_component_or_hook(function: &AnyJsFunctionOrMethod) fn is_within_react_project(manifest: &Option) -> bool { manifest.as_ref().map_or(true, |package_json| { package_json.dependencies.contains("react") + || package_json.dev_dependencies.contains("react") }) }