Skip to content

Commit a4dc56c

Browse files
cblhcblh
authoredJul 14, 2024··
feat(linter): add fixer for unicorn/no_useless_promise_resolve_reject (#4244)
Part of #4179 add fixer for unicorn/no_useless_promise_resolve_reject
·
oxlint_v0.16.7crates_v0.21.0
1 parent ace4f1f commit a4dc56c

File tree

1 file changed

+579
-15
lines changed

1 file changed

+579
-15
lines changed
 

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

+579-15
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,18 @@
1-
use oxc_ast::{ast::MemberExpression, AstKind};
1+
use oxc_ast::{
2+
ast::{Argument, CallExpression, MemberExpression},
3+
AstKind,
4+
};
25
use oxc_diagnostics::OxcDiagnostic;
36
use oxc_macros::declare_oxc_lint;
47
use oxc_span::{GetSpan, Span};
58

6-
use crate::{ast_util::outermost_paren_parent, context::LintContext, rule::Rule, AstNode};
9+
use crate::{
10+
ast_util::outermost_paren_parent,
11+
context::LintContext,
12+
fixer::{Fix, RuleFixer},
13+
rule::Rule,
14+
AstNode,
15+
};
716

817
fn resolve(span0: Span, x1: &str) -> OxcDiagnostic {
918
OxcDiagnostic::warn(format!("eslint-plugin-unicorn(no-useless-promise-resolve-reject): Prefer `{x1} value` over `{x1} Promise.resolve(value)`."))
@@ -75,7 +84,9 @@ impl Rule for NoUselessPromiseResolveReject {
7584
}
7685
}
7786

78-
let Some((is_async, function_node)) = get_function_like_node(node, ctx) else {
87+
let Some((is_async, function_node, is_in_try_statement)) =
88+
get_function_like_node(node, ctx)
89+
else {
7990
return;
8091
};
8192

@@ -85,16 +96,36 @@ impl Rule for NoUselessPromiseResolveReject {
8596

8697
match static_member_expr.property.name.as_str() {
8798
"resolve" => {
88-
ctx.diagnostic(resolve(
89-
node.kind().span(),
90-
if is_yield { "yield" } else { "return" },
91-
));
99+
ctx.diagnostic_with_fix(
100+
resolve(node.kind().span(), if is_yield { "yield" } else { "return" }),
101+
|fixer| {
102+
generate_fix(
103+
call_expr,
104+
false,
105+
is_yield,
106+
is_in_try_statement,
107+
fixer,
108+
ctx,
109+
node,
110+
)
111+
},
112+
);
92113
}
93114
"reject" => {
94-
ctx.diagnostic(reject(
95-
node.kind().span(),
96-
if is_yield { "yield" } else { "return" },
97-
));
115+
ctx.diagnostic_with_fix(
116+
reject(node.kind().span(), if is_yield { "yield" } else { "return" }),
117+
|fixer| {
118+
generate_fix(
119+
call_expr,
120+
true,
121+
is_yield,
122+
is_in_try_statement,
123+
fixer,
124+
ctx,
125+
node,
126+
)
127+
},
128+
);
98129
}
99130
_ => unreachable!(),
100131
}
@@ -104,23 +135,29 @@ impl Rule for NoUselessPromiseResolveReject {
104135
fn get_function_like_node<'a, 'b>(
105136
node: &'a AstNode<'b>,
106137
ctx: &'a LintContext<'b>,
107-
) -> Option<(bool, &'a AstNode<'b>)> {
138+
) -> Option<(bool, &'a AstNode<'b>, bool)> {
108139
let mut parent = node;
140+
let mut is_in_try_statement = false;
109141

110142
let fnx = loop {
111143
if let Some(grand_parent) = ctx.nodes().parent_node(parent.id()) {
112144
parent = grand_parent;
113145
if parent.kind().is_function_like() {
114146
break parent;
115147
}
148+
if matches!(parent.kind(), AstKind::TryStatement(_)) {
149+
is_in_try_statement = true;
150+
}
116151
} else {
117152
return None;
118153
}
119154
};
120155

121156
match fnx.kind() {
122-
AstKind::ArrowFunctionExpression(arrow_expr) => Some((arrow_expr.r#async, parent)),
123-
AstKind::Function(func) => Some((func.r#async, parent)),
157+
AstKind::ArrowFunctionExpression(arrow_expr) => {
158+
Some((arrow_expr.r#async, parent, is_in_try_statement))
159+
}
160+
AstKind::Function(func) => Some((func.r#async, parent, is_in_try_statement)),
124161
_ => None,
125162
}
126163
}
@@ -163,6 +200,133 @@ fn is_promise_callback<'a, 'b>(node: &'a AstNode<'b>, ctx: &'a LintContext<'b>)
163200
false
164201
}
165202

203+
fn match_arrow_function_body<'a>(ctx: &LintContext<'a>, parent: &AstNode<'a>) -> bool {
204+
match ctx.nodes().parent_node(parent.id()) {
205+
Some(arrow_function_body) => match arrow_function_body.kind() {
206+
AstKind::FunctionBody(_) => match ctx.nodes().parent_node(arrow_function_body.id()) {
207+
Some(arrow_function) => {
208+
matches!(arrow_function.kind(), AstKind::ArrowFunctionExpression(_))
209+
}
210+
None => false,
211+
},
212+
_ => false,
213+
},
214+
None => false,
215+
}
216+
}
217+
218+
fn generate_fix<'a>(
219+
call_expr: &CallExpression,
220+
is_reject: bool,
221+
is_yield: bool,
222+
is_in_try_statement: bool,
223+
fixer: RuleFixer<'_, 'a>,
224+
ctx: &LintContext<'a>,
225+
node: &AstNode<'a>,
226+
) -> Fix<'a> {
227+
if call_expr.arguments.len() > 1 {
228+
return fixer.noop();
229+
}
230+
231+
let arg_text = if call_expr.arguments.is_empty() {
232+
&String::new()
233+
} else {
234+
let arg = &call_expr.arguments[0];
235+
if arg.is_spread() {
236+
return fixer.noop();
237+
}
238+
fixer.source_range(arg.span())
239+
};
240+
241+
if is_reject {
242+
if is_in_try_statement {
243+
return fixer.noop();
244+
}
245+
if is_yield {
246+
if let Some(parent) = ctx.nodes().parent_node(node.id()) {
247+
if let Some(grand_parent) = ctx.nodes().parent_node(parent.id()) {
248+
if !matches!(
249+
grand_parent.kind(),
250+
AstKind::ExpressionStatement(_) | AstKind::ParenthesizedExpression(_)
251+
) {
252+
return fixer.noop();
253+
}
254+
};
255+
};
256+
};
257+
}
258+
259+
let node = get_parenthesized_node(node, ctx);
260+
261+
let Some(parent) = ctx.nodes().parent_node(node.id()) else {
262+
return fixer.noop();
263+
};
264+
265+
let is_arrow_function_body = match parent.kind() {
266+
AstKind::ExpressionStatement(_) => match_arrow_function_body(ctx, parent),
267+
_ => false,
268+
};
269+
270+
let mut replace_range = if is_reject { parent.kind().span() } else { call_expr.span() };
271+
let replacement_text = if is_reject {
272+
let mut text =
273+
if arg_text.is_empty() { "undefined".to_string() } else { arg_text.to_string() };
274+
text = format!("throw {text}");
275+
276+
if is_yield {
277+
replace_range = get_parenthesized_node(parent, ctx).kind().span();
278+
text
279+
} else {
280+
text = format!("{text};");
281+
// `=> Promise.reject(error)` -> `=> { throw error; }`
282+
if is_arrow_function_body {
283+
replace_range = get_parenthesized_node(parent, ctx).kind().span();
284+
text = format!("{{ {text} }}");
285+
}
286+
text
287+
}
288+
} else {
289+
let mut text = arg_text.to_string();
290+
if text.is_empty() {
291+
// `=> Promise.resolve()` -> `=> {}`
292+
if is_arrow_function_body {
293+
text = "{}".to_string();
294+
text
295+
} else {
296+
if matches!(parent.kind(), AstKind::ReturnStatement(_)) {
297+
replace_range.start = parent.kind().span().start + 6;
298+
}
299+
if is_yield {
300+
replace_range.start = parent.kind().span().start + 5;
301+
}
302+
text
303+
}
304+
} else {
305+
if matches!(&call_expr.arguments[0], Argument::ObjectExpression(_)) {
306+
text = format!("({text})");
307+
}
308+
text
309+
}
310+
};
311+
312+
fixer.replace(replace_range, replacement_text)
313+
}
314+
315+
fn get_parenthesized_node<'a, 'b>(
316+
node: &'a AstNode<'b>,
317+
ctx: &'a LintContext<'b>,
318+
) -> &'a AstNode<'b> {
319+
let mut node = node;
320+
while let Some(parent_node) = ctx.nodes().parent_node(node.id()) {
321+
if let AstKind::ParenthesizedExpression(_) = parent_node.kind() {
322+
node = parent_node;
323+
} else {
324+
break;
325+
}
326+
}
327+
node
328+
}
329+
166330
#[test]
167331
fn test() {
168332
use crate::tester::Tester;
@@ -459,5 +623,405 @@ fn test() {
459623
r"promise.then(() => {}, async () => { return Promise.reject(bar); })",
460624
];
461625

462-
Tester::new(NoUselessPromiseResolveReject::NAME, pass, fail).test_and_snapshot();
626+
let fix = vec![
627+
(
628+
r"
629+
const main = async foo => {
630+
if (foo > 4) {
631+
return Promise.reject(new Error('🤪'));
632+
}
633+
634+
return Promise.resolve(result);
635+
};
636+
",
637+
r"
638+
const main = async foo => {
639+
if (foo > 4) {
640+
throw new Error('🤪');
641+
}
642+
643+
return result;
644+
};
645+
",
646+
None,
647+
),
648+
// Async function returning Promise.resolve
649+
("async () => Promise.resolve(bar);", "async () => bar;", None),
650+
(
651+
r"
652+
async () => {
653+
return Promise.resolve(bar);
654+
};
655+
",
656+
r"
657+
async () => {
658+
return bar;
659+
};
660+
",
661+
None,
662+
),
663+
(
664+
r"
665+
async function foo() {
666+
return Promise.resolve(bar);
667+
}
668+
",
669+
r"
670+
async function foo() {
671+
return bar;
672+
}
673+
",
674+
None,
675+
),
676+
(
677+
r"
678+
(async function() {
679+
return Promise.resolve(bar);
680+
});
681+
",
682+
r"
683+
(async function() {
684+
return bar;
685+
});
686+
",
687+
None,
688+
),
689+
(
690+
r"
691+
async function * foo() {
692+
return Promise.resolve(bar);
693+
}
694+
",
695+
r"
696+
async function * foo() {
697+
return bar;
698+
}
699+
",
700+
None,
701+
),
702+
(
703+
r"
704+
(async function*() {
705+
return Promise.resolve(bar);
706+
});
707+
",
708+
r"
709+
(async function*() {
710+
return bar;
711+
});
712+
",
713+
None,
714+
),
715+
// Async function returning Promise.reject
716+
("async () => Promise.reject(bar);", "async () => { throw bar; };", None),
717+
(
718+
r"
719+
async () => {
720+
return Promise.reject(bar);
721+
};
722+
",
723+
r"
724+
async () => {
725+
throw bar;
726+
};
727+
",
728+
None,
729+
),
730+
(
731+
r"
732+
async function foo() {
733+
return Promise.reject(bar);
734+
}
735+
",
736+
r"
737+
async function foo() {
738+
throw bar;
739+
}
740+
",
741+
None,
742+
),
743+
(
744+
r"
745+
(async function() {
746+
return Promise.reject(bar);
747+
});
748+
",
749+
r"
750+
(async function() {
751+
throw bar;
752+
});
753+
",
754+
None,
755+
),
756+
(
757+
r"
758+
async function * foo() {
759+
return Promise.reject(bar);
760+
}
761+
",
762+
r"
763+
async function * foo() {
764+
throw bar;
765+
}
766+
",
767+
None,
768+
),
769+
(
770+
r"
771+
(async function*() {
772+
return Promise.reject(bar);
773+
});
774+
",
775+
r"
776+
(async function*() {
777+
throw bar;
778+
});
779+
",
780+
None,
781+
),
782+
// Async generator yielding Promise.resolve
783+
(
784+
r"
785+
async function * foo() {
786+
yield Promise.resolve(bar);
787+
}
788+
",
789+
r"
790+
async function * foo() {
791+
yield bar;
792+
}
793+
",
794+
None,
795+
),
796+
(
797+
r"
798+
(async function * () {
799+
yield Promise.resolve(bar);
800+
});
801+
",
802+
r"
803+
(async function * () {
804+
yield bar;
805+
});
806+
",
807+
None,
808+
),
809+
// Async generator yielding Promise.reject
810+
(
811+
r"
812+
async function * foo() {
813+
yield Promise.reject(bar);
814+
}
815+
",
816+
r"
817+
async function * foo() {
818+
throw bar;
819+
}
820+
",
821+
None,
822+
),
823+
(
824+
r"
825+
(async function * () {
826+
yield Promise.reject(bar);
827+
});
828+
",
829+
r"
830+
(async function * () {
831+
throw bar;
832+
});
833+
",
834+
None,
835+
),
836+
// No arguments
837+
(r"async () => Promise.resolve();", r"async () => {};", None),
838+
(
839+
r"
840+
async function foo() {
841+
return Promise.resolve();
842+
}
843+
",
844+
r"
845+
async function foo() {
846+
return;
847+
}
848+
",
849+
None,
850+
),
851+
(r"async () => Promise.reject();", r"async () => { throw undefined; };", None),
852+
(
853+
r"
854+
async function foo() {
855+
return Promise.reject();
856+
}
857+
",
858+
r"
859+
async function foo() {
860+
throw undefined;
861+
}
862+
",
863+
None,
864+
),
865+
(
866+
r"
867+
async function * foo() {
868+
yield Promise.resolve();
869+
}
870+
",
871+
r"
872+
async function * foo() {
873+
yield;
874+
}
875+
",
876+
None,
877+
),
878+
// Sequence expressions
879+
(
880+
r"
881+
async function * foo() {
882+
yield Promise.resolve((bar, baz));
883+
}
884+
",
885+
r"
886+
async function * foo() {
887+
yield (bar, baz);
888+
}
889+
",
890+
None,
891+
),
892+
(r"async () => Promise.resolve((bar, baz))", r"async () => (bar, baz)", None),
893+
// Arrow function returning an object
894+
(r"async () => Promise.resolve({})", r"async () => ({})", None),
895+
// Try statements
896+
(
897+
r"
898+
async function foo() {
899+
try {
900+
return Promise.resolve(1);
901+
} catch {}
902+
}
903+
",
904+
r"
905+
async function foo() {
906+
try {
907+
return 1;
908+
} catch {}
909+
}
910+
",
911+
None,
912+
),
913+
// Try statements reject case: don't change
914+
(
915+
r"
916+
async function foo() {
917+
try {
918+
return Promise.reject(1);
919+
} catch {}
920+
}
921+
",
922+
r"
923+
async function foo() {
924+
try {
925+
return Promise.reject(1);
926+
} catch {}
927+
}
928+
",
929+
None,
930+
),
931+
// Spread arguments can not modify the original array
932+
(r"async () => Promise.resolve(...bar);", r"async () => Promise.resolve(...bar);", None),
933+
(r"async () => Promise.reject(...bar);", r"async () => Promise.reject(...bar);", None),
934+
// Yield not in an ExpressionStatement
935+
(
936+
r"
937+
async function * foo() {
938+
const baz = yield Promise.resolve(bar);
939+
}
940+
",
941+
r"
942+
async function * foo() {
943+
const baz = yield bar;
944+
}
945+
",
946+
None,
947+
),
948+
// Yield reject not in an ExpressionStatement not modify
949+
(
950+
r"
951+
async function * foo() {
952+
const baz = yield Promise.reject(bar);
953+
}
954+
",
955+
r"
956+
async function * foo() {
957+
const baz = yield Promise.reject(bar);
958+
}
959+
",
960+
None,
961+
),
962+
// Parenthesized Promise.resolve/reject
963+
(r"async () => (Promise.resolve(bar));", r"async () => (bar);", None),
964+
(r"async () => (Promise.reject(bar));", r"async () => { throw bar; };", None),
965+
(r"async () => ((Promise.reject(bar)));", r"async () => { throw bar; };", None),
966+
(
967+
r"
968+
async function * foo() {
969+
(yield Promise.reject(bar));
970+
}
971+
",
972+
r"
973+
async function * foo() {
974+
throw bar;
975+
}
976+
",
977+
None,
978+
),
979+
(
980+
r"
981+
async function * foo() {
982+
((yield Promise.reject(bar)));
983+
}
984+
",
985+
r"
986+
async function * foo() {
987+
throw bar;
988+
}
989+
",
990+
None,
991+
),
992+
// Promise#then/catch/finally callbacks returning Promise.resolve/reject
993+
(
994+
r"promise.then(() => {}, () => Promise.resolve(bar))",
995+
r"promise.then(() => {}, () => bar)",
996+
None,
997+
),
998+
(
999+
r"promise.then(() => Promise.resolve(bar), () => Promise.resolve(baz))",
1000+
r"promise.then(() => bar, () => baz)",
1001+
None,
1002+
),
1003+
(
1004+
r"promise.then(() => {}, () => { return Promise.resolve(bar); })",
1005+
r"promise.then(() => {}, () => { return bar; })",
1006+
None,
1007+
),
1008+
(
1009+
r"promise.then(() => {}, async () => Promise.reject(bar))",
1010+
r"promise.then(() => {}, async () => { throw bar; })",
1011+
None,
1012+
),
1013+
(
1014+
r"promise.then(() => {}, async () => { return Promise.reject(bar); })",
1015+
r"promise.then(() => {}, async () => { throw bar; })",
1016+
None,
1017+
),
1018+
(
1019+
r"promise.then(() => {}, async () => { return Promise.reject(bar); })",
1020+
r"promise.then(() => {}, async () => { throw bar; })",
1021+
None,
1022+
),
1023+
];
1024+
Tester::new(NoUselessPromiseResolveReject::NAME, pass, fail)
1025+
.expect_fix(fix)
1026+
.test_and_snapshot();
4631027
}

0 commit comments

Comments
 (0)
Please sign in to comment.