Skip to content

Commit 584d20f

Browse files
cblhcblh
committed
refactor(linter): optimize no_useless_undefined rule implementation
- Updated `no_useless_undefined` rule to handle cases where `undefined` is used in parameter initializers, assignment expressions, and return statements in functions with explicit return types. - Added utility function `is_has_function_return_type` to check if a function has an explicit return type. - Added multiple new test cases to cover scenarios including: - Object properties and class fields with `undefined`. - Functions and methods returning `undefined`. - Various valid uses of `undefined` that should not trigger the rule. - Improved handling of nested functions returning `undefined`. - Improved existing test cases to ensure the rule correctly ignores cases where `undefined` is necessary. - Added TODO comments for potential future enhancements.
1 parent a619b20 commit 584d20f

File tree

2 files changed

+391
-38
lines changed

2 files changed

+391
-38
lines changed

‎crates/oxc_linter/src/rules/unicorn/no_useless_undefined.rs‎

Lines changed: 304 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -47,42 +47,42 @@ declare_oxc_lint!(
4747

4848
// Create a static set for all function names
4949
static FUNCTION_NAMES: phf::Set<&'static str> = phf::phf_set! {
50-
// Compare function names
51-
"is",
52-
"equal",
53-
"notEqual",
54-
"strictEqual",
55-
"notStrictEqual",
56-
"propertyVal",
57-
"notPropertyVal",
58-
"not",
59-
"include",
60-
"property",
61-
"toBe",
62-
"toHaveBeenCalledWith",
63-
"toContain",
64-
"toContainEqual",
65-
"toEqual",
66-
"same",
67-
"notSame",
68-
"strictSame",
69-
"strictNotSame",
70-
// `array.push(undefined)`
71-
"push",
72-
// `array.unshift(undefined)`
73-
"unshift",
74-
// `array.includes(undefined)`
75-
"includes",
76-
// `set.add(undefined)`
77-
"add",
78-
// `set.has(undefined)`
79-
"has",
80-
// `map.set(foo, undefined)`
81-
"set",
82-
// `React.createContext(undefined)`
83-
"createContext",
84-
// https://vuejs.org/api/reactivity-core.html#ref
85-
"ref",
50+
// Compare function names
51+
"is",
52+
"equal",
53+
"notEqual",
54+
"strictEqual",
55+
"notStrictEqual",
56+
"propertyVal",
57+
"notPropertyVal",
58+
"not",
59+
"include",
60+
"property",
61+
"toBe",
62+
"toHaveBeenCalledWith",
63+
"toContain",
64+
"toContainEqual",
65+
"toEqual",
66+
"same",
67+
"notSame",
68+
"strictSame",
69+
"strictNotSame",
70+
// `array.push(undefined)`
71+
"push",
72+
// `array.unshift(undefined)`
73+
"unshift",
74+
// `array.includes(undefined)`
75+
"includes",
76+
// `set.add(undefined)`
77+
"add",
78+
// `set.has(undefined)`
79+
"has",
80+
// `map.set(foo, undefined)`
81+
"set",
82+
// `React.createContext(undefined)`
83+
"createContext",
84+
// https://vuejs.org/api/reactivity-core.html#ref
85+
"ref",
8686
};
8787

8888
fn is_match_ignore_func_name(name: &str) -> bool {
@@ -118,6 +118,19 @@ fn is_undefined(arg: &Argument) -> bool {
118118
false
119119
}
120120

121+
fn is_has_function_return_type(node: &AstNode, ctx: &LintContext<'_>) -> bool {
122+
let Some(parent_node) = ctx.nodes().parent_node(node.id()) else {
123+
return false;
124+
};
125+
match parent_node.kind() {
126+
AstKind::ArrowFunctionExpression(arrow_func_express) => {
127+
arrow_func_express.return_type.is_some()
128+
}
129+
AstKind::Function(func) => func.return_type.is_some(),
130+
_ => is_has_function_return_type(parent_node, ctx),
131+
}
132+
}
133+
121134
impl Rule for NoUselessUndefined {
122135
fn from_configuration(value: serde_json::Value) -> Self {
123136
let check_arguments =
@@ -139,6 +152,9 @@ impl Rule for NoUselessUndefined {
139152
match parent_node_kind {
140153
// `return undefined`
141154
AstKind::ReturnStatement(ret_stmt) => {
155+
if is_has_function_return_type(parent_node, ctx) {
156+
return;
157+
}
142158
ctx.diagnostic_with_fix(
143159
no_useless_undefined_diagnostic(undefined_literal.span),
144160
|fixer| {
@@ -190,6 +206,10 @@ impl Rule for NoUselessUndefined {
190206
return;
191207
};
192208

209+
if is_has_function_return_type(parent_node, ctx) {
210+
return;
211+
}
212+
193213
ctx.diagnostic_with_fix(
194214
no_useless_undefined_diagnostic(undefined_literal.span),
195215
|fixer| fixer.replace(func_body.span, "{}"),
@@ -208,6 +228,9 @@ impl Rule for NoUselessUndefined {
208228
if variable_declarator.kind == VariableDeclarationKind::Const {
209229
return;
210230
}
231+
if is_has_function_return_type(parent_node, ctx) {
232+
return;
233+
}
211234
return ctx.diagnostic_with_fix(
212235
no_useless_undefined_diagnostic(undefined_literal.span),
213236
|fixer| {
@@ -222,6 +245,9 @@ impl Rule for NoUselessUndefined {
222245
AstKind::AssignmentPattern(assign_pattern) => {
223246
let left = &assign_pattern.left;
224247
let delete_span = Span::new(left.span().end, undefined_literal.span.end);
248+
if is_has_function_return_type(parent_node, ctx) {
249+
return;
250+
}
225251
return ctx.diagnostic_with_fix(
226252
no_useless_undefined_diagnostic(undefined_literal.span),
227253
|fixer| fixer.delete_range(delete_span),
@@ -353,9 +379,223 @@ fn test() {
353379
// `checkArrowFunctionBody: false`
354380
(r"const foo = () => undefined", options_ignore_arrow_function_body()),
355381
(r"const x = { a: undefined }", None),
382+
// https://github.com/zeit/next.js/blob/3af0fe5cf2542237f34d106872d104c3606b1858/packages/next/build/utils.ts#L620
383+
(r"prerenderPaths?.add(entry)", None),
384+
(
385+
r#"
386+
function getThing(): string | undefined {
387+
if (someCondition) {
388+
return "hello world";
389+
}
390+
391+
return undefined;
392+
}
393+
"#,
394+
None,
395+
),
396+
(
397+
r#"
398+
function getThing(): string | undefined {
399+
if (someCondition) {
400+
return "hello world";
401+
} else if (anotherCondition) {
402+
return undefined;
403+
}
404+
405+
return undefined;
406+
}
407+
"#,
408+
None,
409+
),
410+
(r"const foo = (): undefined => {return undefined;}", None),
411+
(r"const foo = (): undefined => undefined;", None),
412+
(r"const foo = (): string => undefined;", None),
413+
(r"const foo = function (): undefined {return undefined}", None),
414+
(r"export function foo(): undefined {return undefined}", None),
415+
(
416+
r"
417+
const object = {
418+
method(): undefined {
419+
return undefined;
420+
}
421+
}
422+
",
423+
None,
424+
),
425+
(
426+
r"
427+
class A {
428+
method(): undefined {
429+
return undefined;
430+
}
431+
}
432+
",
433+
None,
434+
),
435+
(
436+
r"
437+
const A = class A {
438+
method(): undefined {
439+
return undefined
440+
}
441+
};
442+
",
443+
None,
444+
),
445+
(
446+
r"
447+
class A {
448+
static method(): undefined {
449+
return undefined
450+
}
451+
}
452+
",
453+
None,
454+
),
455+
(
456+
r"
457+
class A {
458+
get method(): undefined {
459+
return undefined;
460+
}
461+
}
462+
",
463+
None,
464+
),
465+
(
466+
r"
467+
class A {
468+
static get method(): undefined {
469+
return undefined;
470+
}
471+
}
472+
",
473+
None,
474+
),
475+
(
476+
r"
477+
class A {
478+
#method(): undefined {
479+
return undefined;
480+
}
481+
}
482+
",
483+
None,
484+
),
485+
(
486+
r"
487+
class A {
488+
private method(): undefined {
489+
return undefined;
490+
}
491+
}
492+
",
493+
None,
494+
),
495+
(r"createContext<T>(undefined);", None),
496+
(r"React.createContext<T>(undefined);", None),
497+
(r"const x = { a: undefined }", None),
498+
(
499+
r"
500+
const y: any = {}
501+
y.foo = undefined
502+
",
503+
None,
504+
),
505+
(
506+
r"
507+
class Foo {
508+
public x: number | undefined = undefined
509+
}
510+
",
511+
None,
512+
),
356513
];
357514

358-
let fail = vec![(r"let foo = undefined;", None)];
515+
let fail = vec![
516+
(
517+
r"
518+
foo(
519+
undefined,
520+
bar,
521+
undefined,
522+
undefined,
523+
undefined,
524+
undefined,
525+
)
526+
",
527+
None,
528+
),
529+
(r"function foo([bar = undefined] = []) {}", None),
530+
(
531+
r"
532+
foo(
533+
undefined,
534+
bar,
535+
undefined,
536+
undefined,
537+
undefined,
538+
undefined,
539+
)
540+
",
541+
None,
542+
),
543+
("function foo([bar = undefined] = []) {}", None),
544+
("foo(bar, undefined, undefined);", None),
545+
("let a = undefined, b = 2;", None),
546+
// TODO: Handle parenthesized undefined
547+
/* (
548+
r#"
549+
function foo() {
550+
return /* */ (
551+
/* */
552+
(
553+
/* */
554+
undefined
555+
/* */
556+
)
557+
/* */
558+
) /* */ ;
559+
}
560+
"#,
561+
None,
562+
),
563+
(
564+
r#"
565+
function * foo() {
566+
yield /* */ (
567+
/* */
568+
(
569+
/* */
570+
undefined
571+
/* */
572+
)
573+
/* */
574+
) /* */ ;
575+
}
576+
"#,
577+
None,
578+
),
579+
(
580+
r#"
581+
const foo = () => /* */ (
582+
/* */
583+
(
584+
/* */
585+
undefined
586+
/* */
587+
)
588+
/* */
589+
);
590+
"#,
591+
None,
592+
), */
593+
("foo.bind(undefined)", None),
594+
("bind(foo, undefined)", None),
595+
("foo.bind?.(bar, undefined)", None),
596+
("foo[bind](bar, undefined)", None),
597+
("foo.notBind(bar, undefined)", None),
598+
];
359599

360600
let fix = vec![
361601
(r"function foo() {return undefined;}", r"function foo() {return;}", None),
@@ -396,7 +636,34 @@ fn test() {
396636
("function foo({bar = undefined} = {}) {}", "function foo({bar} = {}) {}", None),
397637
("function foo([bar = undefined]) {}", "function foo([bar]) {}", None),
398638
("function foo([bar = undefined] = []) {}", "function foo([bar] = []) {}", None),
639+
// TODO: maybe fix into `x?: string`?
640+
(
641+
"function foo(x: string | undefined = undefined) { }",
642+
"function foo(x: string | undefined) { }",
643+
None,
644+
),
399645
("return undefined;", "return;", None),
646+
(
647+
r"
648+
function foo():undefined {
649+
function nested() {
650+
return undefined;
651+
}
652+
653+
return nested();
654+
}
655+
",
656+
r"
657+
function foo():undefined {
658+
function nested() {
659+
return;
660+
}
661+
662+
return nested();
663+
}
664+
",
665+
None,
666+
),
400667
];
401668

402669
Tester::new(NoUselessUndefined::NAME, pass, fail).expect_fix(fix).test_and_snapshot();

0 commit comments

Comments
 (0)