Skip to content

Commit

Permalink
feat(useExhaustiveDependencies): add option to disable errors for une…
Browse files Browse the repository at this point in the history
…cessary dependencies (#4135)
  • Loading branch information
simon-paris authored Oct 1, 2024
1 parent fc9c4cf commit 10a1371
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 78 deletions.
17 changes: 2 additions & 15 deletions crates/biome_js_analyze/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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]
Expand All @@ -196,20 +194,9 @@ mod tests {
let parsed = parse(SOURCE, JsFileSource::tsx(), JsParserOptions::default());

let mut error_ranges: Vec<TextRange> = 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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,12 +250,12 @@ declare_lint_rule! {
}

#[derive(Debug, Clone)]
pub struct ReactExtensiveDependenciesOptions {
pub struct HookConfigMaps {
pub(crate) hooks_config: FxHashMap<String, ReactHookConfiguration>,
pub(crate) stable_config: FxHashSet<StableReactHookConfiguration>,
}

impl Default for ReactExtensiveDependenciesOptions {
impl Default for HookConfigMaps {
fn default() -> Self {
let hooks_config = FxHashMap::from_iter([
("useEffect".to_string(), (0, 1, true).into()),
Expand Down Expand Up @@ -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<Hook>,
}

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)]
Expand Down Expand Up @@ -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,
});
}
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -712,18 +730,20 @@ impl Rule for UseExhaustiveDependencies {
type Query = Semantic<JsCallExpression>;
type State = Fix;
type Signals = Vec<Self::State>;
type Options = Box<HooksOptions>;
type Options = Box<UseExhaustiveDependenciesOptions>;

fn run(ctx: &RuleContext<Self>) -> Vec<Self::State> {
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![];
};
Expand All @@ -741,7 +761,7 @@ impl Rule for UseExhaustiveDependencies {
capture,
&component_function_range,
model,
&options,
&hook_config_maps,
)
})
.map(|capture| {
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
Original file line number Diff line number Diff line change
@@ -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;
}

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"$schema": "../../../../../../packages/@biomejs/biome/configuration_schema.json",
"linter": {
"rules": {
"correctness": {
"useExhaustiveDependencies": {
"level": "error",
"options": {
"reportUnnecessaryDependencies": false
}
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,5 @@ hooks_incorrect_options.json:9:7 deserialize ━━━━━━━━━━━

i Known keys:

- reportUnnecessaryDependencies
- hooks



18 changes: 11 additions & 7 deletions packages/@biomejs/backend-jsonrpc/src/workspace.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 10a1371

Please sign in to comment.