Skip to content

Commit 3d5db4a

Browse files
camc314camc314
committed
feat(linter/unicorn): implement no-useless-error-capture-stack-trace (#14222)
1 parent b3b482a commit 3d5db4a

File tree

4 files changed

+371
-0
lines changed

4 files changed

+371
-0
lines changed

‎crates/oxc_linter/src/generated/rule_runner_impls.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2534,6 +2534,13 @@ impl RuleRunner for crate::rules::unicorn::no_unreadable_iife::NoUnreadableIife
25342534
Some(&AstTypesBitset::from_types(&[AstType::CallExpression]));
25352535
}
25362536

2537+
impl RuleRunner
2538+
for crate::rules::unicorn::no_useless_error_capture_stack_trace::NoUselessErrorCaptureStackTrace
2539+
{
2540+
const NODE_TYPES: Option<&AstTypesBitset> =
2541+
Some(&AstTypesBitset::from_types(&[AstType::CallExpression]));
2542+
}
2543+
25372544
impl RuleRunner
25382545
for crate::rules::unicorn::no_useless_fallback_in_spread::NoUselessFallbackInSpread
25392546
{

‎crates/oxc_linter/src/rules.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -436,6 +436,7 @@ pub(crate) mod unicorn {
436436
pub mod no_unnecessary_slice_end;
437437
pub mod no_unreadable_array_destructuring;
438438
pub mod no_unreadable_iife;
439+
pub mod no_useless_error_capture_stack_trace;
439440
pub mod no_useless_fallback_in_spread;
440441
pub mod no_useless_length_check;
441442
pub mod no_useless_promise_resolve_reject;
@@ -1130,6 +1131,7 @@ oxc_macros::declare_all_lint_rules! {
11301131
unicorn::filename_case,
11311132
unicorn::new_for_builtins,
11321133
unicorn::no_array_callback_reference,
1134+
unicorn::no_useless_error_capture_stack_trace,
11331135
unicorn::no_array_sort,
11341136
unicorn::no_array_reverse,
11351137
unicorn::no_instanceof_builtins,

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

Lines changed: 306 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,306 @@
1+
use oxc_ast::{
2+
AstKind,
3+
ast::{CallExpression, Expression},
4+
};
5+
use oxc_diagnostics::OxcDiagnostic;
6+
use oxc_macros::declare_oxc_lint;
7+
use oxc_semantic::{IsGlobalReference, SymbolId};
8+
use oxc_span::Span;
9+
10+
use crate::{
11+
AstNode, ast_util::is_method_call, context::LintContext, rule::Rule, utils::BUILT_IN_ERRORS,
12+
};
13+
14+
fn no_useless_error_capture_stack_trace_diagnostic(span: Span) -> OxcDiagnostic {
15+
OxcDiagnostic::warn("Do not use `Error.captureStackTrace(…)` in the constructor of an Error subclass")
16+
.with_help("The Error constructor already calls `captureStackTrace` internally, so calling it again is unnecessary.")
17+
.with_label(span)
18+
}
19+
20+
#[derive(Debug)]
21+
enum ClassInfo {
22+
None,
23+
Named(SymbolId),
24+
Anonymous,
25+
}
26+
27+
#[derive(Debug, Default, Clone)]
28+
pub struct NoUselessErrorCaptureStackTrace;
29+
30+
declare_oxc_lint!(
31+
/// ### What it does
32+
///
33+
/// Disallows unnecessary `Error.captureStackTrace(…)` in error constructors.
34+
///
35+
/// ### Why is this bad?
36+
///
37+
/// Calling `Error.captureStackTrace(…)` inside the constructor of a built-in `Error` subclass
38+
/// is unnecessary, since the `Error` constructor calls it automatically.
39+
///
40+
/// ### Examples
41+
///
42+
/// Examples of **incorrect** code for this rule:
43+
/// ```js
44+
/// class MyError extends Error {
45+
/// constructor() {
46+
/// Error.captureStackTrace(this, MyError);
47+
/// }
48+
/// }
49+
/// ```
50+
///
51+
/// Examples of **correct** code for this rule:
52+
/// ```js
53+
/// class MyError extends Error {
54+
/// constructor() {
55+
/// // No need to call Error.captureStackTrace
56+
/// }
57+
/// }
58+
/// ```
59+
NoUselessErrorCaptureStackTrace,
60+
unicorn,
61+
restriction,
62+
pending
63+
);
64+
65+
impl Rule for NoUselessErrorCaptureStackTrace {
66+
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
67+
let AstKind::CallExpression(call_expr) = node.kind() else {
68+
return;
69+
};
70+
71+
if !is_method_call(call_expr, Some(&["Error"]), Some(&["captureStackTrace"]), None, None) {
72+
return;
73+
}
74+
75+
if let Some(member) = call_expr.callee.as_member_expression()
76+
&& let Expression::Identifier(error_ident) = member.object()
77+
&& !error_ident.is_global_reference(ctx.scoping())
78+
{
79+
return;
80+
}
81+
82+
match get_error_subclass_if_in_constructor(node, ctx) {
83+
ClassInfo::None => return,
84+
ClassInfo::Named(class_id) => {
85+
if !is_referencing_class(call_expr, Some(class_id), ctx) {
86+
return;
87+
}
88+
}
89+
ClassInfo::Anonymous => {
90+
if !is_referencing_class(call_expr, None, ctx) {
91+
return;
92+
}
93+
}
94+
}
95+
96+
ctx.diagnostic(no_useless_error_capture_stack_trace_diagnostic(call_expr.span));
97+
}
98+
}
99+
100+
fn get_error_subclass_if_in_constructor<'a>(
101+
node: &AstNode<'a>,
102+
ctx: &LintContext<'a>,
103+
) -> ClassInfo {
104+
let mut found_constructor = false;
105+
106+
for ancestor in ctx.nodes().ancestors(node.id()) {
107+
match ancestor.kind() {
108+
AstKind::StaticBlock(_) => return ClassInfo::None,
109+
AstKind::Function(func) if func.id.is_some() => return ClassInfo::None,
110+
AstKind::MethodDefinition(method) if method.kind.is_constructor() => {
111+
found_constructor = true;
112+
}
113+
AstKind::Class(class) => {
114+
if found_constructor && is_error_class(class, ctx) {
115+
if let Some(id) = class.id.as_ref()
116+
&& let Some(symbol_id) = id.symbol_id.get()
117+
{
118+
return ClassInfo::Named(symbol_id);
119+
}
120+
return ClassInfo::Anonymous;
121+
}
122+
break;
123+
}
124+
_ => {}
125+
}
126+
}
127+
128+
ClassInfo::None
129+
}
130+
131+
fn is_error_class(class: &oxc_ast::ast::Class, ctx: &LintContext) -> bool {
132+
// Check if the class extends a built-in Error type
133+
let Some(super_class) = &class.super_class else {
134+
return false;
135+
};
136+
137+
// Check if super_class is one of the built-in error types
138+
if let Expression::Identifier(ident) = super_class.get_inner_expression() {
139+
// Check that the name is a built-in error type
140+
if !BUILT_IN_ERRORS.contains(&ident.name.as_str()) {
141+
return false;
142+
}
143+
144+
if !ident.is_global_reference(ctx.scoping()) {
145+
return false;
146+
}
147+
148+
return true;
149+
}
150+
151+
false
152+
}
153+
154+
fn is_referencing_class(
155+
call_expr: &CallExpression,
156+
class_id: Option<SymbolId>,
157+
ctx: &LintContext,
158+
) -> bool {
159+
let Some(expr) = call_expr
160+
.arguments
161+
.get(1)
162+
.and_then(|arg| arg.as_expression())
163+
.map(Expression::get_inner_expression)
164+
else {
165+
return false;
166+
};
167+
168+
match expr {
169+
Expression::Identifier(ident) => {
170+
if let Some(expected_symbol) = class_id
171+
&& let Some(reference_id) = ident.reference_id.get()
172+
&& let Some(symbol_id) = ctx.scoping().get_reference(reference_id).symbol_id()
173+
{
174+
return symbol_id == expected_symbol;
175+
}
176+
177+
false
178+
}
179+
Expression::MetaProperty(meta)
180+
if meta.meta.name == "new" && meta.property.name == "target" =>
181+
{
182+
true
183+
}
184+
_ => {
185+
if let Some(member) = expr.as_member_expression()
186+
&& let Expression::ThisExpression(_) = member.object().get_inner_expression()
187+
&& let Some(prop_name) = member.static_property_name()
188+
&& prop_name == "constructor"
189+
{
190+
return true;
191+
}
192+
193+
false
194+
}
195+
}
196+
}
197+
198+
#[test]
199+
fn test() {
200+
use crate::tester::Tester;
201+
202+
let pass = vec![
203+
"class MyError {constructor() {Error.captureStackTrace(this, MyError)}}",
204+
"class MyError extends NotABuiltinError {constructor() {Error.captureStackTrace(this, MyError)}}",
205+
"class MyError extends Error {
206+
notConstructor() {
207+
Error.captureStackTrace(this, MyError)
208+
}
209+
}",
210+
"class MyError extends Error {
211+
constructor() {
212+
function foo() {
213+
Error.captureStackTrace(this, MyError)
214+
}
215+
}
216+
}",
217+
"class MyError extends Error {
218+
constructor(MyError) {
219+
Error.captureStackTrace(this, MyError)
220+
}
221+
}",
222+
"class MyError extends Error {
223+
static {
224+
Error.captureStackTrace(this, MyError)
225+
function foo() {
226+
Error.captureStackTrace(this, MyError)
227+
}
228+
}
229+
}",
230+
"class MyError extends Error {
231+
constructor() {
232+
class NotAErrorSubclass {
233+
constructor() {
234+
Error.captureStackTrace(this, new.target)
235+
}
236+
}
237+
}
238+
}",
239+
"class Error {}
240+
class MyError extends Error {
241+
constructor() {
242+
Error.captureStackTrace(this, MyError)
243+
}
244+
}",
245+
"class Error {}
246+
class MyError extends RangeError {
247+
constructor() {
248+
Error.captureStackTrace(this, MyError)
249+
}
250+
}",
251+
"class MyError extends Error {
252+
constructor(): void;
253+
static {
254+
Error.captureStackTrace(this, MyError)
255+
function foo() {
256+
Error.captureStackTrace(this, MyError)
257+
}
258+
}
259+
}",
260+
];
261+
262+
let fail = vec![
263+
"class MyError extends Error {
264+
constructor() {
265+
const foo = () => {
266+
Error.captureStackTrace(this, MyError)
267+
}
268+
}
269+
}",
270+
"class MyError extends Error {
271+
constructor() {
272+
if (a) Error.captureStackTrace(this, MyError)
273+
}
274+
}",
275+
"class MyError extends Error {
276+
constructor() {
277+
const x = () => Error.captureStackTrace(this, MyError)
278+
}
279+
}",
280+
"class MyError extends Error {
281+
constructor() {
282+
void Error.captureStackTrace(this, MyError)
283+
}
284+
}",
285+
"export default class extends Error {
286+
constructor() {
287+
Error.captureStackTrace(this, new.target)
288+
}
289+
}",
290+
"export default (
291+
class extends Error {
292+
constructor() {
293+
Error.captureStackTrace(this, new.target)
294+
}
295+
}
296+
)",
297+
];
298+
299+
Tester::new(
300+
NoUselessErrorCaptureStackTrace::NAME,
301+
NoUselessErrorCaptureStackTrace::PLUGIN,
302+
pass,
303+
fail,
304+
)
305+
.test_and_snapshot();
306+
}

‎crates/oxc_linter/src/snapshots/unicorn_no_useless_error_capture_stack_trace.snap‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
source: crates/oxc_linter/src/tester.rs
3+
---
4+
⚠ eslint-plugin-unicorn(no-useless-error-capture-stack-trace): Do not use `Error.captureStackTrace(…)` in the constructor of an Error subclass
5+
╭─[no_useless_error_capture_stack_trace.tsx:4:7]
6+
3 │ const foo = () => {
7+
4 │ Error.captureStackTrace(this, MyError)
8+
· ──────────────────────────────────────
9+
5 │ }
10+
╰────
11+
help: The Error constructor already calls `captureStackTrace` internally, so calling it again is unnecessary.
12+
13+
⚠ eslint-plugin-unicorn(no-useless-error-capture-stack-trace): Do not use `Error.captureStackTrace(…)` in the constructor of an Error subclass
14+
╭─[no_useless_error_capture_stack_trace.tsx:3:13]
15+
2 │ constructor() {
16+
3 │ if (a) Error.captureStackTrace(this, MyError)
17+
· ──────────────────────────────────────
18+
4 │ }
19+
╰────
20+
help: The Error constructor already calls `captureStackTrace` internally, so calling it again is unnecessary.
21+
22+
⚠ eslint-plugin-unicorn(no-useless-error-capture-stack-trace): Do not use `Error.captureStackTrace(…)` in the constructor of an Error subclass
23+
╭─[no_useless_error_capture_stack_trace.tsx:3:22]
24+
2 │ constructor() {
25+
3 │ const x = () => Error.captureStackTrace(this, MyError)
26+
· ──────────────────────────────────────
27+
4 │ }
28+
╰────
29+
help: The Error constructor already calls `captureStackTrace` internally, so calling it again is unnecessary.
30+
31+
⚠ eslint-plugin-unicorn(no-useless-error-capture-stack-trace): Do not use `Error.captureStackTrace(…)` in the constructor of an Error subclass
32+
╭─[no_useless_error_capture_stack_trace.tsx:3:11]
33+
2 │ constructor() {
34+
3 │ void Error.captureStackTrace(this, MyError)
35+
· ──────────────────────────────────────
36+
4 │ }
37+
╰────
38+
help: The Error constructor already calls `captureStackTrace` internally, so calling it again is unnecessary.
39+
40+
⚠ eslint-plugin-unicorn(no-useless-error-capture-stack-trace): Do not use `Error.captureStackTrace(…)` in the constructor of an Error subclass
41+
╭─[no_useless_error_capture_stack_trace.tsx:3:6]
42+
2 │ constructor() {
43+
3 │ Error.captureStackTrace(this, new.target)
44+
· ─────────────────────────────────────────
45+
4 │ }
46+
╰────
47+
help: The Error constructor already calls `captureStackTrace` internally, so calling it again is unnecessary.
48+
49+
⚠ eslint-plugin-unicorn(no-useless-error-capture-stack-trace): Do not use `Error.captureStackTrace(…)` in the constructor of an Error subclass
50+
╭─[no_useless_error_capture_stack_trace.tsx:4:7]
51+
3 │ constructor() {
52+
4 │ Error.captureStackTrace(this, new.target)
53+
· ─────────────────────────────────────────
54+
5 │ }
55+
╰────
56+
help: The Error constructor already calls `captureStackTrace` internally, so calling it again is unnecessary.

0 commit comments

Comments
 (0)