Make no-unsafe-catch-error-property flow-sensitive to branch dominance and source order

[Copilot]

no-unsafe-catch-error-property used a catch-frame boolean guard, so any recognized guard could suppress all unsafe accesses in the catch block, including sibling branches and earlier accesses. This produced false negatives whenever guard recognition was not control-flow aligned with the reported access.

• Rule behavior: replace frame-wide suppression with per-access guard evaluation

  • Removed hasGuard/hasNonNullGuard frame state and evaluate each collected unsafe member access independently at CatchClause:exit.
  • Suppress only when the access is actually guarded by an enclosing truthy branch (if consequent, conditional consequent, && RHS) that narrows the caught variable.

• Guard semantics preserved, but scoped correctly

  • instanceof Error continues to guard accesses inside dominated branches.
  • typeof err === "object" still requires non-null evidence (err, err != null, or prior if (!err) return; in the same block).
  • getErrorMessage(err) is now order/scope-aware: it only suppresses accesses that occur later in source order within the same enclosing block.

• Regression coverage added

  • Added cases for:
    • sibling-branch guard does not suppress unguarded access,
    • guard-after-access does not suppress earlier access,
    • nested dominated guard remains valid.

// now reported (sibling-branch guard no longer suppresses this)
catch (err) {
  if (flag) core.info(err.message);
  if (err instanceof Error) core.info(err.stack);
}

• Fixes [eslint-refiner] no-unsafe-catch-error-property: flow-insensitive guard suppresses unguarded accesses (false negative) #47496

[github-actions]

Great work! 🎉 This PR refines the guard flow sensitivity analysis in the no-unsafe-catch-error-property ESLint rule with a cleaner, more context-aware approach.

The changes are solid:

• Improved logic — The refactored guard detection now correctly handles nested scopes, sibling branches, and call-ordering semantics instead of relying on flat global flags.
• Better test coverage — New test cases cover important edge cases like sibling-branch guards not suppressing access and guards that come after unsafe access.
• Focused scope — Changes are confined to the ESLint rule implementation and its tests.

The PR looks ready for merge!

Generated by ✅ Contribution Check · sonnet46 67.1 AIC · ⌖ 10.4 AIC · ⊞ 6.2K · ◷

[Copilot]

Pull request overview

Refines catch-error property validation to evaluate guards per access using branch dominance and source order.

Changes:

• Adds branch-aware guard detection.
• Makes getErrorMessage suppression order- and block-sensitive.
• Adds sibling-branch and guard-order regression tests.

Show a summary per file

File	Description
eslint-factory/src/rules/no-unsafe-catch-error-property.ts	Implements per-access guard evaluation.
eslint-factory/src/rules/no-unsafe-catch-error-property.test.ts	Adds flow-sensitivity regression tests.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (2)

eslint-factory/src/rules/no-unsafe-catch-error-property.ts:111

• Nested && guards lose combined narrowing evidence because each ancestor's left operand is evaluated independently. For example, typeof err === "object" && (err !== null && err.status) is safe, but the inner guard has only the non-null check and the outer guard has only the typeof check, so this now reports err.status. Accumulate all dominating left operands before evaluating the combined guard.

    if (ancestor.type === AST_NODE_TYPES.LogicalExpression && ancestor.operator === "&&" && ancestor.right === current) {
      if (isTruthyBranchGuarded(sourceCode, ancestor.left, varName, ancestor)) return true;

eslint-factory/src/rules/no-unsafe-catch-error-property.ts:132

• Source order of block entries does not imply that the call executes. In if (flag) getErrorMessage(err); console.log(err.stack);, the call maps to the earlier IfStatement entry, so this returns true and suppresses err.stack even when flag is false. Require the call's execution path to dominate the access, excluding conditional, loop, short-circuit, and deferred contexts.

    if (callEntryIndex < nodeEntryIndex) return true;
    if (callEntryIndex === nodeEntryIndex && callNode.range[1] <= node.range[0]) return true;

• Files reviewed: 2/2 changed files
• Comments generated: 1
• Review effort level: Medium

[gh-aw-bot]

@copilot please run the pr-finisher skill, address the outstanding review feedback on this PR, and rerun the relevant checks once the branch is up to date.

Run: https://github.com/github/gh-aw/actions/runs/29998171888

Generated by 👨‍🍳 PR Sous Chef · gpt54 8.92 AIC · ⌖ 7.98 AIC · ⊞ 7K · ◷
_{Comment /souschef to run again}

[Copilot]

@copilot please run the pr-finisher skill, address the outstanding review feedback on this PR, and rerun the relevant checks once the branch is up to date.

Run: https://github.com/github/gh-aw/acti...

Addressed in commit Recognize early-exit instanceof guards (negated and else-return patterns).

Added hasPriorEarlyExitInstanceofGuard which scans preceding statements in the same block for two early-exit patterns before the unsafe access:

• if (!(err instanceof Error)) return/throw; — negated instanceof with terminating consequent (direct or block form)
• if (err instanceof Error) { ... } else return/throw; — instanceof with terminating alternate

The new check is wired into CatchClause:exit alongside the existing isGuardedByAncestorBranch and isCallOrderingGuarded checks. Tests cover all new valid patterns plus an invalid counterexample (non-terminating consequent).

[github-actions]

🎉 This pull request is included in a new release.

Release: v0.83.1