Skip to content

Commit dac4db9

Browse files
authored
fix(linter/exhaustive-deps): better diagnostics for missing dependencies (#12337)
1 parent 853ce40 commit dac4db9

File tree

3 files changed

+436
-10
lines changed

3 files changed

+436
-10
lines changed

crates/oxc_linter/src/rules/react/exhaustive_deps.rs

Lines changed: 48 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,9 @@ fn async_effect_diagnostic(span: Span) -> OxcDiagnostic {
6363
.with_error_code_scope(SCOPE)
6464
}
6565

66-
fn missing_dependency_diagnostic(hook_name: &str, deps: &[String], span: Span) -> OxcDiagnostic {
67-
let deps_pretty = if deps.len() == 1 {
66+
fn missing_dependency_diagnostic(hook_name: &str, deps: &[Name<'_>], span: Span) -> OxcDiagnostic {
67+
let single = deps.len() == 1;
68+
let deps_pretty = if single {
6869
format!("'{}'", deps[0])
6970
} else {
7071
let mut iter = deps.iter();
@@ -78,12 +79,25 @@ fn missing_dependency_diagnostic(hook_name: &str, deps: &[String], span: Span) -
7879
format!("{all_but_last}, and '{last}'")
7980
};
8081

81-
OxcDiagnostic::warn(if deps.len() == 1 {
82+
let labels = deps
83+
.iter()
84+
.map(|dep| {
85+
// when multiple dependencies are missing, labels can quickly get noisy,
86+
// so we only add labels when there's only one dependency
87+
if single {
88+
dep.span.label(format!("{hook_name} uses `{dep}` here"))
89+
} else {
90+
dep.span.into()
91+
}
92+
})
93+
.chain(std::iter::once(span.primary()));
94+
95+
OxcDiagnostic::warn(if single {
8296
format!("React Hook {hook_name} has a missing dependency: {deps_pretty}")
8397
} else {
8498
format!("React Hook {hook_name} has missing dependencies: {deps_pretty}")
8599
})
86-
.with_label(span)
100+
.with_labels(labels)
87101
.with_help("Either include it or remove the dependency array.")
88102
.with_error_code_scope(SCOPE)
89103
}
@@ -338,7 +352,7 @@ impl Rule for ExhaustiveDeps {
338352
_ => {
339353
ctx.diagnostic(missing_dependency_diagnostic(
340354
hook_name,
341-
&[ident.name.to_string()],
355+
&[Name::from(ident.as_ref())],
342356
dependencies_node.span(),
343357
));
344358
None
@@ -354,7 +368,7 @@ impl Rule for ExhaustiveDeps {
354368
AstKind::FormalParameter(_) => {
355369
ctx.diagnostic(missing_dependency_diagnostic(
356370
hook_name,
357-
&[ident.name.to_string()],
371+
&[Name::from(ident.as_ref())],
358372
dependencies_node.span(),
359373
));
360374
None
@@ -558,7 +572,7 @@ impl Rule for ExhaustiveDeps {
558572
if undeclared_deps.clone().count() > 0 {
559573
ctx.diagnostic(missing_dependency_diagnostic(
560574
hook_name,
561-
&undeclared_deps.map(Dependency::to_string).collect::<Vec<_>>(),
575+
&undeclared_deps.map(Name::from).collect::<Vec<_>>(),
562576
dependencies_node.span(),
563577
));
564578
}
@@ -731,6 +745,33 @@ fn get_node_name_without_react_namespace<'a, 'b>(expr: &'b Expression<'a>) -> Op
731745
}
732746
}
733747

748+
#[derive(Debug)]
749+
struct Name<'a> {
750+
pub span: Span,
751+
pub name: Cow<'a, str>,
752+
}
753+
impl std::fmt::Display for Name<'_> {
754+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
755+
self.name.fmt(f)
756+
}
757+
}
758+
759+
impl<'a> From<&Dependency<'a>> for Name<'a> {
760+
fn from(dep: &Dependency<'a>) -> Self {
761+
let name = if dep.chain.is_empty() {
762+
Cow::Borrowed(dep.name.as_str())
763+
} else {
764+
Cow::Owned(dep.to_string())
765+
};
766+
Self { name, span: dep.span }
767+
}
768+
}
769+
impl<'a> From<&IdentifierReference<'a>> for Name<'a> {
770+
fn from(id: &IdentifierReference<'a>) -> Self {
771+
Self { name: Cow::Borrowed(id.name.as_str()), span: id.span }
772+
}
773+
}
774+
734775
#[derive(Debug)]
735776
struct Dependency<'a> {
736777
span: Span,

0 commit comments

Comments
 (0)