From 10a1371dca2581f4a4d6ef3fdc3117ced2b91a58 Mon Sep 17 00:00:00 2001 From: Simon Paris Date: Tue, 1 Oct 2024 14:43:56 +0800 Subject: [PATCH] feat(useExhaustiveDependencies): add option to disable errors for unecessary dependencies (#4135) --- crates/biome_js_analyze/src/lib.rs | 17 +--- .../use_exhaustive_dependencies.rs | 56 +++++++++----- .../reportUnnecessaryDependencies.js | 13 ++++ .../reportUnnecessaryDependencies.js.snap | 21 +++++ ...reportUnnecessaryDependencies.options.json | 15 ++++ .../invalid/hooks_incorrect_options.json.snap | 4 +- .../@biomejs/backend-jsonrpc/src/workspace.ts | 18 +++-- .../@biomejs/biome/configuration_schema.json | 77 ++++++++++--------- 8 files changed, 143 insertions(+), 78 deletions(-) create mode 100644 crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.js create mode 100644 crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.js.snap create mode 100644 crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.options.json diff --git a/crates/biome_js_analyze/src/lib.rs b/crates/biome_js_analyze/src/lib.rs index 50b8e00f6176..9c0866e4de42 100644 --- a/crates/biome_js_analyze/src/lib.rs +++ b/crates/biome_js_analyze/src/lib.rs @@ -164,8 +164,7 @@ where #[cfg(test)] mod tests { - use biome_analyze::options::RuleOptions; - use biome_analyze::{AnalyzerOptions, Never, RuleCategoriesBuilder, RuleFilter, RuleKey}; + use biome_analyze::{AnalyzerOptions, Never, RuleCategoriesBuilder, RuleFilter}; use biome_console::fmt::{Formatter, Termcolor}; use biome_console::{markup, Markup}; use biome_diagnostics::category; @@ -176,7 +175,6 @@ mod tests { use biome_project::{Dependencies, PackageJson}; use std::slice; - use crate::lint::correctness::use_exhaustive_dependencies::{Hook, HooksOptions}; use crate::{analyze, AnalysisFilter, ControlFlow}; #[ignore] @@ -196,20 +194,9 @@ mod tests { let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default()); let mut error_ranges: Vec = Vec::new(); - let mut options = AnalyzerOptions::default(); - let hook = Hook { - name: "myEffect".to_string(), - closure_index: Some(0), - dependencies_index: Some(1), - stable_result: None, - }; + let options = AnalyzerOptions::default(); 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( diff --git a/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs b/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs index 32e116d7fc37..38519499846a 100644 --- a/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs +++ b/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs @@ -250,12 +250,12 @@ declare_lint_rule! { } #[derive(Debug, Clone)] -pub struct ReactExtensiveDependenciesOptions { +pub struct HookConfigMaps { pub(crate) hooks_config: FxHashMap, pub(crate) stable_config: FxHashSet, } -impl Default for ReactExtensiveDependenciesOptions { +impl Default for HookConfigMaps { fn default() -> Self { let hooks_config = FxHashMap::from_iter([ ("useEffect".to_string(), (0, 1, true).into()), @@ -289,15 +289,33 @@ impl Default for ReactExtensiveDependenciesOptions { } /// Options for the rule `useExhaustiveDependencies` -#[derive(Clone, Debug, Default, Deserialize, Deserializable, Eq, PartialEq, Serialize)] +#[derive(Clone, Debug, Deserialize, Deserializable, Eq, PartialEq, Serialize)] #[cfg_attr(feature = "schemars", derive(JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] -pub struct HooksOptions { +pub struct UseExhaustiveDependenciesOptions { + /// Whether to report an error when a dependency is listed in the dependencies array but isn't used. Defaults to true. + #[serde(default = "report_unnecessary_dependencies_default")] + pub report_unnecessary_dependencies: bool, + /// List of hooks of which the dependencies should be validated. + #[serde(default)] #[deserializable(validate = "non_empty")] pub hooks: Vec, } +impl Default for UseExhaustiveDependenciesOptions { + fn default() -> Self { + Self { + report_unnecessary_dependencies: report_unnecessary_dependencies_default(), + hooks: vec![], + } + } +} + +fn report_unnecessary_dependencies_default() -> bool { + true +} + #[derive(Clone, Debug, Default, Deserialize, Deserializable, Eq, PartialEq, Serialize)] #[cfg_attr(feature = "schemars", derive(JsonSchema))] #[serde(rename_all = "camelCase", deny_unknown_fields)] @@ -356,15 +374,15 @@ impl DeserializableValidator for Hook { } } -impl ReactExtensiveDependenciesOptions { - pub fn new(hooks: HooksOptions) -> Self { - let mut result = ReactExtensiveDependenciesOptions::default(); - for hook in hooks.hooks { - if let Some(stable_result) = hook.stable_result { - if stable_result != StableHookResult::None { +impl HookConfigMaps { + pub fn new(hooks: &UseExhaustiveDependenciesOptions) -> Self { + let mut result = HookConfigMaps::default(); + for hook in &hooks.hooks { + if let Some(stable_result) = &hook.stable_result { + if *stable_result != StableHookResult::None { result.stable_config.insert(StableReactHookConfiguration { hook_name: hook.name.clone(), - result: stable_result, + result: stable_result.clone(), builtin: false, }); } @@ -373,7 +391,7 @@ impl ReactExtensiveDependenciesOptions { (hook.closure_index, hook.dependencies_index) { result.hooks_config.insert( - hook.name, + hook.name.clone(), ReactHookConfiguration { closure_index, dependencies_index, @@ -448,7 +466,7 @@ fn capture_needs_to_be_in_the_dependency_list( capture: &Capture, component_function_range: &TextRange, model: &SemanticModel, - options: &ReactExtensiveDependenciesOptions, + options: &HookConfigMaps, ) -> bool { // Ignore if referenced in TS typeof if capture @@ -712,18 +730,20 @@ impl Rule for UseExhaustiveDependencies { type Query = Semantic; type State = Fix; type Signals = Vec; - type Options = Box; + type Options = Box; fn run(ctx: &RuleContext) -> Vec { let options = ctx.options(); - let options = ReactExtensiveDependenciesOptions::new(options.as_ref().clone()); + let hook_config_maps = HookConfigMaps::new(options); let mut signals = vec![]; let call = ctx.query(); let model = ctx.model(); - if let Some(result) = react_hook_with_dependency(call, &options.hooks_config, model) { + if let Some(result) = + react_hook_with_dependency(call, &hook_config_maps.hooks_config, model) + { let Some(component_function) = function_of_hook_call(call) else { return vec![]; }; @@ -741,7 +761,7 @@ impl Rule for UseExhaustiveDependencies { capture, &component_function_range, model, - &options, + &hook_config_maps, ) }) .map(|capture| { @@ -862,7 +882,7 @@ impl Rule for UseExhaustiveDependencies { }); } - if !excessive_deps.is_empty() { + if options.report_unnecessary_dependencies && !excessive_deps.is_empty() { signals.push(Fix::RemoveDependency { function_name_range: result.function_name_range, component_function, diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.js b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.js new file mode 100644 index 000000000000..b942093ae692 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.js @@ -0,0 +1,13 @@ +import {useEffect} from "react"; + +// should not report errors for the unused `b` when the reportUnnecessaryDependencies option is false +function ReportUnnecessaryDependencies() { + const [b] = useState("hello") + const [a] = useState("world"); + + useEffect(() => { + console.log(a); + }, [a, b]); + + return a; +} diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.js.snap b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.js.snap new file mode 100644 index 000000000000..ae5fa7b74aa5 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.js.snap @@ -0,0 +1,21 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: reportUnnecessaryDependencies.js +--- +# Input +```jsx +import {useEffect} from "react"; + +// should not report errors for the unused `b` when the reportUnnecessaryDependencies option is false +function ReportUnnecessaryDependencies() { + const [b] = useState("hello") + const [a] = useState("world"); + + useEffect(() => { + console.log(a); + }, [a, b]); + + return a; +} + +``` diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.options.json b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.options.json new file mode 100644 index 000000000000..5b2ee6764158 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/reportUnnecessaryDependencies.options.json @@ -0,0 +1,15 @@ +{ + "$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json", + "linter": { + "rules": { + "correctness": { + "useExhaustiveDependencies": { + "level": "error", + "options": { + "reportUnnecessaryDependencies": false + } + } + } + } + } +} diff --git a/crates/biome_service/tests/invalid/hooks_incorrect_options.json.snap b/crates/biome_service/tests/invalid/hooks_incorrect_options.json.snap index 5d69ad53e3dd..197288d87b84 100644 --- a/crates/biome_service/tests/invalid/hooks_incorrect_options.json.snap +++ b/crates/biome_service/tests/invalid/hooks_incorrect_options.json.snap @@ -15,7 +15,5 @@ hooks_incorrect_options.json:9:7 deserialize ━━━━━━━━━━━ i Known keys: + - reportUnnecessaryDependencies - hooks - - - diff --git a/packages/@biomejs/backend-jsonrpc/src/workspace.ts b/packages/@biomejs/backend-jsonrpc/src/workspace.ts index d10410948d90..20ac81aa68a8 100644 --- a/packages/@biomejs/backend-jsonrpc/src/workspace.ts +++ b/packages/@biomejs/backend-jsonrpc/src/workspace.ts @@ -1184,7 +1184,7 @@ export interface Correctness { /** * Enforce all dependencies are correctly specified in a React hook. */ - useExhaustiveDependencies?: RuleConfiguration_for_HooksOptions; + useExhaustiveDependencies?: RuleConfiguration_for_UseExhaustiveDependenciesOptions; /** * Enforce that all React hooks are being called from the Top Level component functions. */ @@ -1985,9 +1985,9 @@ export type RuleFixConfiguration_for_ValidAriaRoleOptions = export type RuleConfiguration_for_ComplexityOptions = | RulePlainConfiguration | RuleWithOptions_for_ComplexityOptions; -export type RuleConfiguration_for_HooksOptions = +export type RuleConfiguration_for_UseExhaustiveDependenciesOptions = | RulePlainConfiguration - | RuleWithOptions_for_HooksOptions; + | RuleWithOptions_for_UseExhaustiveDependenciesOptions; export type RuleConfiguration_for_DeprecatedHooksOptions = | RulePlainConfiguration | RuleWithOptions_for_DeprecatedHooksOptions; @@ -2103,7 +2103,7 @@ export interface RuleWithOptions_for_ComplexityOptions { */ options: ComplexityOptions; } -export interface RuleWithOptions_for_HooksOptions { +export interface RuleWithOptions_for_UseExhaustiveDependenciesOptions { /** * The severity of the emitted diagnostics by the rule */ @@ -2111,7 +2111,7 @@ export interface RuleWithOptions_for_HooksOptions { /** * Rule's options */ - options: HooksOptions; + options: UseExhaustiveDependenciesOptions; } export interface RuleWithOptions_for_DeprecatedHooksOptions { /** @@ -2321,11 +2321,15 @@ export interface ComplexityOptions { /** * Options for the rule `useExhaustiveDependencies` */ -export interface HooksOptions { +export interface UseExhaustiveDependenciesOptions { /** * List of hooks of which the dependencies should be validated. */ - hooks: Hook[]; + hooks?: Hook[]; + /** + * Whether to report an error when a dependency is listed in the dependencies array but isn't used. Defaults to true. + */ + reportUnnecessaryDependencies?: boolean; } /** * Options for the `useHookAtTopLevel` rule have been deprecated, since we now use the React hook naming convention to determine whether a function is a hook. diff --git a/packages/@biomejs/biome/configuration_schema.json b/packages/@biomejs/biome/configuration_schema.json index 9f725c5c0a2e..e028d4f9e51a 100644 --- a/packages/@biomejs/biome/configuration_schema.json +++ b/packages/@biomejs/biome/configuration_schema.json @@ -1051,7 +1051,7 @@ "useExhaustiveDependencies": { "description": "Enforce all dependencies are correctly specified in a React hook.", "anyOf": [ - { "$ref": "#/definitions/HooksConfiguration" }, + { "$ref": "#/definitions/UseExhaustiveDependenciesConfiguration" }, { "type": "null" } ] }, @@ -1485,25 +1485,6 @@ }, "additionalProperties": false }, - "HooksConfiguration": { - "anyOf": [ - { "$ref": "#/definitions/RulePlainConfiguration" }, - { "$ref": "#/definitions/RuleWithHooksOptions" } - ] - }, - "HooksOptions": { - "description": "Options for the rule `useExhaustiveDependencies`", - "type": "object", - "required": ["hooks"], - "properties": { - "hooks": { - "description": "List of hooks of which the dependencies should be validated.", - "type": "array", - "items": { "$ref": "#/definitions/Hook" } - } - }, - "additionalProperties": false - }, "IndentStyle": { "oneOf": [ { "description": "Tab", "type": "string", "enum": ["tab"] }, @@ -2713,21 +2694,6 @@ }, "additionalProperties": false }, - "RuleWithHooksOptions": { - "type": "object", - "required": ["level"], - "properties": { - "level": { - "description": "The severity of the emitted diagnostics by the rule", - "allOf": [{ "$ref": "#/definitions/RulePlainConfiguration" }] - }, - "options": { - "description": "Rule's options", - "allOf": [{ "$ref": "#/definitions/HooksOptions" }] - } - }, - "additionalProperties": false - }, "RuleWithNamingConventionOptions": { "type": "object", "required": ["level"], @@ -2877,6 +2843,23 @@ }, "additionalProperties": false }, + "RuleWithUseExhaustiveDependenciesOptions": { + "type": "object", + "required": ["level"], + "properties": { + "level": { + "description": "The severity of the emitted diagnostics by the rule", + "allOf": [{ "$ref": "#/definitions/RulePlainConfiguration" }] + }, + "options": { + "description": "Rule's options", + "allOf": [ + { "$ref": "#/definitions/UseExhaustiveDependenciesOptions" } + ] + } + }, + "additionalProperties": false + }, "RuleWithUseImportExtensionsOptions": { "type": "object", "required": ["level"], @@ -4008,6 +3991,30 @@ }, "additionalProperties": false }, + "UseExhaustiveDependenciesConfiguration": { + "anyOf": [ + { "$ref": "#/definitions/RulePlainConfiguration" }, + { "$ref": "#/definitions/RuleWithUseExhaustiveDependenciesOptions" } + ] + }, + "UseExhaustiveDependenciesOptions": { + "description": "Options for the rule `useExhaustiveDependencies`", + "type": "object", + "properties": { + "hooks": { + "description": "List of hooks of which the dependencies should be validated.", + "default": [], + "type": "array", + "items": { "$ref": "#/definitions/Hook" } + }, + "reportUnnecessaryDependencies": { + "description": "Whether to report an error when a dependency is listed in the dependencies array but isn't used. Defaults to true.", + "default": true, + "type": "boolean" + } + }, + "additionalProperties": false + }, "UseImportExtensionsConfiguration": { "anyOf": [ { "$ref": "#/definitions/RulePlainConfiguration" },