feat(linter/unicorn): implement no-useless-error-capture-stack-trace

[camc314]

No description provided.

[camc314]

• feat(linter/unicorn): implement no-useless-error-capture-stack-trace #14222 👈 (View in Graphite)
• feat(linter/unicorn): implement no-array-callback-reference #14230
• main

---

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #14222 will not alter performance

_{Comparing c/09-28-feat_linter_unicorn_implement_no-useless-error-capture-stack-trace_2 (3d5db4a) with main (b3b482a)¹}

Summary

✅ 4 untouched
⏩ 33 skipped²

Footnotes

1. No successful run was found on main (3d5db4a) during the generation of this report, so b3b482a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

2. 33 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[Copilot]

Pull Request Overview

Adds support for the no-useless-error-capture-stack-trace rule from eslint-plugin-unicorn, which detects unnecessary calls to Error.captureStackTrace() inside constructors of Error subclasses.

• Implements rule logic to detect Error.captureStackTrace() calls within Error subclass constructors
• Adds comprehensive test cases covering various scenarios including arrow functions and static blocks
• Updates rule count in snapshot files across the codebase

Reviewed Changes

Copilot reviewed 48 out of 49 changed files in this pull request and generated no comments.

File	Description
crates/oxc_linter/src/rules/unicorn/no_useless_error_capture_stack_trace.rs	Main rule implementation with AST traversal and Error subclass detection
crates/oxc_linter/src/snapshots/unicorn_no_useless_error_capture_stack_trace.snap	Test output snapshots showing diagnostic messages
crates/oxc_linter/src/rules.rs	Rule registration and module declaration
apps/oxlint/src/snapshots/*.snap	Updated rule count from 89/90/91 to 90/91/92 reflecting new rule addition

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[camc314]

@Sysix or @camchenry do you mind giving this a quick review, and a sanity check that this should be a correctness rule? Thanks

[Sysix]

The main use case for Error.captureStackTrace() is to install a stack trace on a custom error object. Typically, you define custom errors by extending the Error class, which automatically makes the stack property available via inheritance. However, the problem with the default stack trace is that it includes the constructor call itself, which leaks implementation details. You can avoid this by using Error.captureStackTrace(), which allows the stack trace to be installed even for custom errors that do not inherit from Error.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/captureStackTrace#installing_stack_trace_on_a_custom_error_object

class MyError extends Error {
  constructor(message, options) {
    super(message, options);
  }
}

const myError = new MyError("Something went wrong");
console.log(myError.stack);

// output:
MyError@debugger eval code:3:5
@debugger eval code:7:17

class MyError extends Error {
  constructor(message, options) {
    super(message, options);
    if ("captureStackTrace" in Error) {
      // Avoid MyError itself in the stack trace
      Error.captureStackTrace(this, MyError);
    }
  }
}

const myError = new MyError("Something went wrong");
console.log(myError.stack);

// output:
@debugger eval code:11:17

---

Reading the docs, this should not be correctness, because it could want to ignore it stack trace output for MyError.
Maybe Restriction or Suspicious ? :)

[camc314]

hmm let me test this later. unicorn refers to the call as useless, which to me implies correctness.

i wonder if calling super before captureStackYrace is correct, but if you skip super it isn't?

thanks for looking

[camc314]

Merge activity

• Sep 30, 1:45 PM UTC: The merge label '0-merge' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
• Sep 30, 1:50 PM UTC: camc314 added this pull request to the Graphite merge queue.
• Sep 30, 1:53 PM UTC: Merged by the Graphite merge queue.