Skip to content

Commit 14f0159

Browse files
authored
feat(linter/exhaustive-deps): add auto-fixer (#12354)
## What This PR Does Adds auto-fixing capabilities for some scenarios in `react-hooks/exhaustive-deps`. This is a poor man's stacked PR. #12337 must be merged first.
1 parent ed696b5 commit 14f0159

File tree

3 files changed

+147
-14
lines changed

3 files changed

+147
-14
lines changed

crates/oxc_linter/src/context/mod.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -291,6 +291,27 @@ impl<'a> LintContext<'a> {
291291
self.diagnostic_with_fix_of_kind(diagnostic, FixKind::Suggestion, fix);
292292
}
293293

294+
/// Report a lint rule violation and provide a suggestion for fixing it.
295+
///
296+
/// The second argument is a [closure] that takes a [`RuleFixer`] and
297+
/// returns something that can turn into a `CompositeFix`.
298+
///
299+
/// Fixes created this way should not create parse errors, but have the
300+
/// potential to change the code's semantics. If your fix is completely safe
301+
/// and definitely does not change semantics, use [`LintContext::diagnostic_with_fix`].
302+
/// If your fix has the potential to create parse errors, use
303+
/// [`LintContext::diagnostic_with_dangerous_fix`].
304+
///
305+
/// [closure]: <https://doc.rust-lang.org/book/ch13-01-closures.html>
306+
#[inline]
307+
pub fn diagnostic_with_dangerous_suggestion<C, F>(&self, diagnostic: OxcDiagnostic, fix: F)
308+
where
309+
C: Into<RuleFix<'a>>,
310+
F: FnOnce(RuleFixer<'_, 'a>) -> C,
311+
{
312+
self.diagnostic_with_fix_of_kind(diagnostic, FixKind::DangerousSuggestion, fix);
313+
}
314+
294315
/// Report a lint rule violation and provide a potentially dangerous
295316
/// automatic fix for it.
296317
///

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

Lines changed: 119 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,8 @@ declare_oxc_lint!(
242242
/// ```
243243
ExhaustiveDeps,
244244
react,
245-
correctness
245+
correctness,
246+
safe_fixes_and_dangerous_suggestions
246247
);
247248

248249
const HOOKS_USELESS_WITHOUT_DEPENDENCIES: [&str; 2] = ["useCallback", "useMemo"];
@@ -297,10 +298,10 @@ impl Rule for ExhaustiveDeps {
297298

298299
if dependencies_node.is_none() && !is_effect {
299300
if HOOKS_USELESS_WITHOUT_DEPENDENCIES.contains(&hook_name.as_str()) {
300-
ctx.diagnostic(dependency_array_required_diagnostic(
301-
hook_name.as_str(),
302-
call_expr.span(),
303-
));
301+
ctx.diagnostic_with_fix(
302+
dependency_array_required_diagnostic(hook_name.as_str(), call_expr.span()),
303+
|fixer| fixer.insert_text_after(callback_node, ", []"),
304+
);
304305
}
305306
return;
306307
}
@@ -580,11 +581,11 @@ impl Rule for ExhaustiveDeps {
580581
});
581582

582583
if undeclared_deps.clone().count() > 0 {
583-
ctx.diagnostic(missing_dependency_diagnostic(
584-
hook_name,
585-
&undeclared_deps.map(Name::from).collect::<Vec<_>>(),
586-
dependencies_node.span(),
587-
));
584+
let undeclared = undeclared_deps.map(Name::from).collect::<Vec<_>>();
585+
ctx.diagnostic_with_dangerous_suggestion(
586+
missing_dependency_diagnostic(hook_name, &undeclared, dependencies_node.span()),
587+
|fixer| fix::append_dependencies(fixer, &undeclared, dependencies_node.as_ref()),
588+
);
588589
}
589590

590591
// effects are allowed to have extra dependencies
@@ -756,7 +757,7 @@ fn get_node_name_without_react_namespace<'a, 'b>(expr: &'b Expression<'a>) -> Op
756757
}
757758
}
758759

759-
#[derive(Debug)]
760+
#[derive(Debug, Clone)]
760761
struct Name<'a> {
761762
pub span: Span,
762763
pub name: Cow<'a, str>,
@@ -1446,6 +1447,43 @@ fn is_inside_effect_cleanup(stack: &[AstType]) -> bool {
14461447
false
14471448
}
14481449

1450+
mod fix {
1451+
use super::Name;
1452+
use itertools::Itertools;
1453+
use oxc_ast::ast::ArrayExpression;
1454+
use oxc_span::GetSpan;
1455+
1456+
use crate::fixer::{RuleFix, RuleFixer};
1457+
1458+
pub fn append_dependencies<'c, 'a: 'c>(
1459+
fixer: RuleFixer<'c, 'a>,
1460+
names: &[Name<'a>],
1461+
deps: &ArrayExpression<'a>,
1462+
) -> RuleFix<'a> {
1463+
let names_as_deps = names.iter().map(|n| n.name.as_ref()).join(", ");
1464+
let Some(last) = deps.elements.last() else {
1465+
return fixer.replace(deps.span, format!("[{names_as_deps}]"));
1466+
};
1467+
// look for a trailing comma. we'll need to add one if its not there already
1468+
let mut needs_comma = true;
1469+
let last_span = last.span();
1470+
for c in fixer.source_text()[(last_span.end as usize)..].chars() {
1471+
match c {
1472+
',' => {
1473+
needs_comma = false;
1474+
break;
1475+
}
1476+
']' => break,
1477+
_ => {} // continue
1478+
}
1479+
}
1480+
fixer.insert_text_after_range(
1481+
last_span,
1482+
if needs_comma { format!(", {names_as_deps}") } else { format!(" {names_as_deps}") },
1483+
)
1484+
}
1485+
}
1486+
14491487
#[test]
14501488
fn test() {
14511489
use crate::tester::Tester;
@@ -3996,11 +4034,81 @@ fn test() {
39964034
Some(serde_json::json!([{ "additionalHooks": "useSpecialEffect" }])),
39974035
)];
39984036

4037+
let fix = vec![
4038+
(
4039+
"const useHook = x => useCallback(() => x)",
4040+
"const useHook = x => useCallback(() => x, [])",
4041+
// None,
4042+
// FixKind::SafeFix,
4043+
),
4044+
(
4045+
"const useHook = x => useCallback(() => { return x; })",
4046+
"const useHook = x => useCallback(() => { return x; }, [])",
4047+
// None,
4048+
// FixKind::SafeFix,
4049+
),
4050+
(
4051+
r"const useHook = () => {
4052+
const [state, setState] = useState(0);
4053+
const foo = useCallback(() => state, []);
4054+
}",
4055+
r"const useHook = () => {
4056+
const [state, setState] = useState(0);
4057+
const foo = useCallback(() => state, [state]);
4058+
}",
4059+
// None,
4060+
// FixKind::DangerousSuggestion,
4061+
),
4062+
(
4063+
r"const useHook = () => {
4064+
const [x] = useState(0);
4065+
const [y] = useState(0);
4066+
const foo = useCallback(() => x + y, []);
4067+
}",
4068+
r"const useHook = () => {
4069+
const [x] = useState(0);
4070+
const [y] = useState(0);
4071+
const foo = useCallback(() => x + y, [x, y]);
4072+
}",
4073+
// None,
4074+
// FixKind::DangerousSuggestion,
4075+
),
4076+
(
4077+
r"const useHook = () => {
4078+
const [x] = useState(0);
4079+
const [y] = useState(0);
4080+
const [z] = useState(0);
4081+
const foo = useCallback(() => x + y + z, [x]);
4082+
}",
4083+
r"const useHook = () => {
4084+
const [x] = useState(0);
4085+
const [y] = useState(0);
4086+
const [z] = useState(0);
4087+
const foo = useCallback(() => x + y + z, [x, y, z]);
4088+
}",
4089+
// None,
4090+
// FixKind::DangerousSuggestion,
4091+
),
4092+
// (
4093+
// r#"const useHook = () => {
4094+
// const [state, setState] = useState(0);
4095+
// const foo = useCallback(() => state);
4096+
// }"#,
4097+
// r#"const useHook = () => {
4098+
// const [state, setState] = useState(0);
4099+
// const foo = useCallback(() => state, [state]);
4100+
// }"#,
4101+
// // None,
4102+
// // FixKind::DangerousSuggestion,
4103+
// ),
4104+
];
4105+
39994106
Tester::new(
40004107
ExhaustiveDeps::NAME,
40014108
ExhaustiveDeps::PLUGIN,
40024109
pass.iter().map(|&code| (code, None)).chain(pass_additional_hooks).collect::<Vec<_>>(),
40034110
fail.iter().map(|&code| (code, None)).chain(fail_additional_hooks).collect::<Vec<_>>(),
40044111
)
4112+
.expect_fix(fix)
40054113
.test_and_snapshot();
40064114
}

crates/oxc_macros/src/declare_oxc_lint.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,11 @@ fn parse_fix(s: &str) -> proc_macro2::TokenStream {
256256
is_conditional = true;
257257
false
258258
}
259-
"and" | "or" => false, // e.g. fix_or_suggestion
259+
// e.g. "safe_fix". safe is implied
260+
"safe"
261+
// e.g. fix_or_suggestion
262+
| "and" | "or"
263+
=> false,
260264
_ => true,
261265
})
262266
.unique()
@@ -273,8 +277,8 @@ fn parse_fix(s: &str) -> proc_macro2::TokenStream {
273277

274278
fn parse_fix_kind(s: &str) -> proc_macro2::TokenStream {
275279
match s {
276-
"fix" => quote! { FixKind::Fix },
277-
"suggestion" => quote! { FixKind::Suggestion },
280+
"fix" | "fixes" => quote! { FixKind::Fix },
281+
"suggestion" | "suggestions" => quote! { FixKind::Suggestion },
278282
"dangerous" => quote! { FixKind::Dangerous },
279283
_ => panic!("invalid fix kind: {s}. Valid fix kinds are fix, suggestion, or dangerous."),
280284
}

0 commit comments

Comments
 (0)