Skip to content

Commit

Permalink
feat(linter): show dependency variable name by useExhaustiveDependenc…
Browse files Browse the repository at this point in the history
…ies (biomejs#1551)
  • Loading branch information
mehm8128 authored and ematipico committed Jan 24, 2024
1 parent 31dc7ac commit 259a839
Show file tree
Hide file tree
Showing 9 changed files with 60 additions and 49 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ Read our [guidelines for writing a good changelog entry](https://github.com/biom
These cases require the presence of a prototype.
- Add dependency variable names on error message when useExhaustiveDependencies rule shows errors. Contributed by @mehm8128
#### Bug fixes
- The fix of [useArrowFunction](https://biomejs.dev/linter/rules/use-arrow-function) now adds parentheses around the arrow function in more cases where it is needed ([#1524](https://github.com/biomejs/biome/issues/1524)).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ pub enum Fix {
/// When a dependency needs to be added.
AddDependency {
function_name_range: TextRange,
captures: Vec<TextRange>,
captures: (String, Vec<TextRange>),
dependencies_len: usize,
},
/// When a dependency needs to be removed.
Expand All @@ -392,6 +392,7 @@ pub enum Fix {
function_name_range: TextRange,
capture_range: TextRange,
dependency_range: TextRange,
dependency_text: String,
},
}

Expand Down Expand Up @@ -632,6 +633,7 @@ impl Rule for UseExhaustiveDependencies {
function_name_range: result.function_name_range,
capture_range: *capture_range,
dependency_range: dep.syntax().text_trimmed_range(),
dependency_text: dep.syntax().text_trimmed().to_string(),
});
}
_ => {}
Expand Down Expand Up @@ -674,7 +676,7 @@ impl Rule for UseExhaustiveDependencies {
}

// Generate signals
for (_, captures) in add_deps {
for captures in add_deps {
signals.push(Fix::AddDependency {
function_name_range: result.function_name_range,
captures,
Expand All @@ -701,23 +703,22 @@ impl Rule for UseExhaustiveDependencies {
captures,
dependencies_len,
} => {
let (capture_text, captures_range) = captures;
let mut diag = RuleDiagnostic::new(
rule_category!(),
function_name_range,
markup! {
"This hook does not specify all of its dependencies."
},
markup! {"This hook does not specify all of its dependencies: "{capture_text}""},
);

for range in captures {
for range in captures_range {
diag = diag.detail(
range,
"This dependency is not specified in the hook dependency list.",
);
}

if *dependencies_len == 0 {
diag = if captures.len() == 1 {
diag = if captures_range.len() == 1 {
diag.note("Either include it or remove the dependency array")
} else {
diag.note("Either include them or remove the dependency array")
Expand All @@ -731,11 +732,16 @@ impl Rule for UseExhaustiveDependencies {
dependencies,
component_function,
} => {
let deps_joined_with_comma = dependencies
.iter()
.map(|dep| dep.syntax().text_trimmed().to_string())
.collect::<Vec<String>>()
.join(", ");
let mut diag = RuleDiagnostic::new(
rule_category!(),
function_name_range,
markup! {
"This hook specifies more dependencies than necessary."
"This hook specifies more dependencies than necessary: "{deps_joined_with_comma}""
},
);

Expand All @@ -761,12 +767,13 @@ impl Rule for UseExhaustiveDependencies {
function_name_range,
capture_range,
dependency_range,
dependency_text,
} => {
let diag = RuleDiagnostic::new(
rule_category!(),
function_name_range,
markup! {
"This hook specifies a dependency more specific that its captures"
"This hook specifies a dependency more specific that its captures: "{dependency_text}""
},
)
.detail(capture_range, "This capture is more generic than...")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ function MyComponent2() {
```
checkHooksImportedFromReact.js:3:9 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━
! This hook does not specify all of its dependencies.
! This hook does not specify all of its dependencies: a
1 │ function MyComponent1() {
2 │ let a = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ function MyComponent() {
```
customHook.js:5:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This hook does not specify all of its dependencies.
! This hook does not specify all of its dependencies: a
3 │ function MyComponent() {
4 │ let a = 1;
Expand All @@ -53,7 +53,7 @@ customHook.js:5:5 lint/correctness/useExhaustiveDependencies ━━━━━━
```
customHook.js:9:5 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
! This hook does not specify all of its dependencies.
! This hook does not specify all of its dependencies: a
7 │ }, []);
8 │
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ function MyComponent3() {
```
extraDependenciesInvalid.js:5:3 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━━
! This hook specifies more dependencies than necessary.
! This hook specifies more dependencies than necessary: a
3 │ function MyComponent() {
4 │ let a = 1;
Expand All @@ -74,7 +74,7 @@ extraDependenciesInvalid.js:5:3 lint/correctness/useExhaustiveDependencies ━
```
extraDependenciesInvalid.js:12:3 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━
! This hook specifies more dependencies than necessary.
! This hook specifies more dependencies than necessary: a, b
10 │ function MyComponent2() {
11 │ let a = 1, b = 1;
Expand Down Expand Up @@ -107,7 +107,7 @@ extraDependenciesInvalid.js:12:3 lint/correctness/useExhaustiveDependencies ━
```
extraDependenciesInvalid.js:19:3 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━
! This hook specifies more dependencies than necessary.
! This hook specifies more dependencies than necessary: a
17 │ function MyComponent2() {
18 │ const a = 1;
Expand All @@ -131,7 +131,7 @@ extraDependenciesInvalid.js:19:3 lint/correctness/useExhaustiveDependencies ━
```
extraDependenciesInvalid.js:28:3 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━
! This hook specifies a dependency more specific that its captures
! This hook specifies a dependency more specific that its captures: someObj.id
26 │ function MyComponent1() {
27 │ let someObj = getObj();
Expand Down Expand Up @@ -164,7 +164,7 @@ extraDependenciesInvalid.js:28:3 lint/correctness/useExhaustiveDependencies ━
```
extraDependenciesInvalid.js:28:3 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━
! This hook does not specify all of its dependencies.
! This hook does not specify all of its dependencies: someObj
26 │ function MyComponent1() {
27 │ let someObj = getObj();
Expand All @@ -188,7 +188,7 @@ extraDependenciesInvalid.js:28:3 lint/correctness/useExhaustiveDependencies ━
```
extraDependenciesInvalid.js:36:3 lint/correctness/useExhaustiveDependencies ━━━━━━━━━━━━━━━━━━━━━━━━
! This hook specifies more dependencies than necessary.
! This hook specifies more dependencies than necessary: outer
34 │ // Dependencies from outer scope should not be valid
35 │ function MyComponent3() {
Expand Down
Loading

0 comments on commit 259a839

Please sign in to comment.