Skip to content

[WIP] Fix expressionReferencesCatchVar to handle LogicalExpression#43951

Closed
pelikhan with Copilot wants to merge 1 commit into
mainfrom
copilot/eslint-factory-require-error-cause-in-rethrow
Closed

[WIP] Fix expressionReferencesCatchVar to handle LogicalExpression#43951
pelikhan with Copilot wants to merge 1 commit into
mainfrom
copilot/eslint-factory-require-error-cause-in-rethrow

Conversation

Copilot AI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.


This section details on the original issue you should resolve

<issue_title>eslint-factory: require-error-cause-in-rethrow's expressionReferencesCatchVar skips LogicalExpression/ConditionalExpression (idi
[Content truncated due to length]</issue_title>
<issue_description>### Summary

In require-error-cause-in-rethrow, expressionReferencesCatchVar (src/rules/require-error-cause-in-rethrow.ts:27-56) decides whether the rethrown new Error(...) message references the caught variable. It recurses through Identifier, CallExpression, TemplateLiteral, BinaryExpression, and MemberExpression — but it does not handle several node types that routinely wrap error values in defensive message-building:

  • LogicalExpression (err.message || "unknown", err ?? fallback, err.message && ...) — note ESTree models ||/&&/?? as LogicalExpression, not BinaryExpression, so the existing BinaryExpression branch does not cover them.
  • ConditionalExpression (cond ? err.message : "n/a").
  • SequenceExpression and array/object literals holding the catch var (new Error([err].join('')), less common).

Any rethrow whose reference to the caught variable lives only inside one of these unhandled nodes returns false → the rule does not fire → the cause chain is silently dropped.

Grounding

actions/setup/js/create_pull_request.cjs:589 builds its message with a LogicalExpression fallback:

throw new Error(`Failed to delete existing remote branch "${branchName}" for reuse with recreate-ref: ${message || String(err)}`);

The ${message || String(err)} template expression is a LogicalExpression; expressionReferencesCatchVar bails out at the unhandled node type and never reaches the String(err) reference. (This site is also affected by the alias blindspot filed separately — both gaps independently hide it; fixing only one is insufficient.)

The x.message || "fallback" and cond ? x.message : default idioms are standard defensive error-formatting in JS, so this is a broad precision gap even where no aliasing is involved.

Acceptance criteria

  1. Extend expressionReferencesCatchVar to traverse LogicalExpression (both operands), ConditionalExpression (test/consequent/alternate), and SequenceExpression (all expressions). Consider array/object literals for completeness if cheap.
  2. Add RuleTester coverage:
    • invalid: catch (err) { throw new Error(\x: ${err.message || 'unknown'}`); }`
    • invalid: catch (err) { throw new Error(cond ? err.message : 'n/a'); }
    • valid: same shapes but with { cause: err } present.
    • valid (regression guard): a logical expression that does NOT reference the catch var, e.g. throw new Error(a || b);, must remain un-flagged.
  3. Confirm no new false positives on the existing valid fixtures (the "Something went wrong" / unrelated-message cases).

Notes

Keep the traversal purely syntactic (no type info / no cross-statement dataflow), consistent with the rest of the rule. Recursion is already bounded by AST depth.

Generated by 🤖 ESLint Refiner · 197.1 AIC · ⌖ 13.3 AIC · ⊞ 4.6K ·

  • expires on Jul 13, 2026, 10:39 PM UTC-08:00

Comments on the Issue (you are @copilot in this section)

@github-actions

github-actions Bot commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

🤖 PR Triage — §28848998303

Field Value
Category bug
Risk low
Score 20/100 (impact:10 urgency:5 quality:5)
Action ⏸️ defer

📝 DRAFT/WIP — Fix expressionReferencesCatchVar to handle LogicalExpression/ConditionalExpression in eslint-factory. 0 files committed. Await draft completion.

Generated by 🔧 PR Triage Agent · 84.1 AIC · ⌖ 6.65 AIC · ⊞ 5.4K ·

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants