Skip to content

Prevent unsafe try/catch autofix suggestions for VariableDeclaration in fetch-body and JSON.parse rules - #51017

Merged
pelikhan merged 4 commits into
mainfrom
copilot/require-fetch-response-body-try-catch-fix
Aug 7, 2026
Merged

Prevent unsafe try/catch autofix suggestions for VariableDeclaration in fetch-body and JSON.parse rules#51017
pelikhan merged 4 commits into
mainfrom
copilot/require-fetch-response-body-try-catch-fix

Conversation

CopilotAI commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Both require-fetch-response-body-try-catch and require-json-parse-try-catch could suggest wrapping a const/let declaration in try/catch, which can move the binding into block scope and break later references. This change keeps diagnostics intact while making suggestions scope-safe.

  • Suggestion safety gate for VariableDeclaration

    • Added a guard in both rules to suppress wrap suggestions when any declared identifier is referenced outside the declaration statement range.
    • Behavior for non-VariableDeclaration wrappable statements is unchanged.
  • Rule parity across both implementations

    • Applied the same fix pattern to:
      • require-fetch-response-body-try-catch
      • require-json-parse-try-catch
    • This aligns both rules with the same safety principle already used elsewhere in the rule set.
  • Regression coverage for scope-stranding case

    • Added tests for each rule where a flagged initializer’s variable is used later in the block; expected result is report emitted, suggestion withheld.
    • Existing single-use/safe suggestion cases remain covered and unchanged.
constpayload=awaitresponse.json();constpageArtifacts=Array.isArray(payload?.artifacts) ? payload.artifacts : [];// Before: suggested wrap could strand `payload` inside try block.// Now: diagnostic still reports, but unsafe suggestion is not offered.

@copilot Please triage this PR, refresh the branch if possible, and run the pr-finisher skill before handing back to maintainers.

Branch refresh is requested if GitHub can update it cleanly.

Run context: https://github.com/github/gh-aw/actions/runs/31183485569> Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.6 AIC · ⊞ 5.9K ·

Comment /souschef to run again

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
CopilotAI changed the title [WIP] Fix autofix scope for variable declaration in rulesPrevent unsafe try/catch autofix suggestions for VariableDeclaration in fetch-body and JSON.parse rulesAug 7, 2026
CopilotAI requested a review from pelikhanAugust 7, 2026 06:15
@github-actions

Copy link
Copy Markdown
Contributor

PR Triage

Category: bug (lint rule refinement) · Risk: low · Score: 42/100 (impact 15, urgency 10, quality 17)

Recommendation:batch_review — grouped with #51016, #51002, #50995 (lint/regexp/security-linter cleanup batch: lint-quality-cleanup)

Draft PR narrowing autofix scope for two ESLint rules to avoid unsafe try/catch suggestions. No CI run yet (draft). No reviews posted yet. Small, isolated diff (4 files).

Generated by 🔧 PR Triage Agent · auto · 55.6 AIC · ⌖ 2.45 AIC · ⊞ 7.9K ·

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds scope-aware suggestion guards while preserving diagnostics.

Changes:

  • Suppresses unsafe wrapping when declared bindings are used later.
  • Adds regression tests for both ESLint rules.
Show a summary per file
FileDescription
require-json-parse-try-catch.tsAdds suggestion safety gate.
require-json-parse-try-catch.test.tsTests later variable usage.
require-fetch-response-body-try-catch.tsAdds equivalent safety gate.
require-fetch-response-body-try-catch.test.tsTests fetch-body scope safety.

Review details

Tip

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

  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment on lines +65 to +66
if (stmt.type !== AST_NODE_TYPES.VariableDeclaration || stmt.kind === "var") {
return true;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d64588a. Suggestions are now suppressed for VariableDeclaration nodes unless they are standalone statement-list entries (Program/BlockStatement/SwitchCase), so export const ... and for (...) declaration contexts no longer receive wrap suggestions.

Comment on lines +87 to +88
if (stmt.type !== AST_NODE_TYPES.VariableDeclaration || stmt.kind === "var") {
return true;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d64588a. Added the same standalone-statement guard to this rule, so suggestions are withheld for export const ... and for-header VariableDeclaration contexts where replacing with a TryStatement would be invalid.

@gh-aw-bot

Copy link
Copy Markdown
Collaborator

@copilot Please triage this PR, refresh the branch if possible, and run the pr-finisher skill before handing back to maintainers.

Branch refresh is requested if GitHub can update it cleanly.

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

Generated by 👨🍳 PR Sous Chef
Comment /souschef to run again

Generated by 👨‍🍳 PR Sous Chef · gpt54 · 11.6 AIC · ⊞ 5.9K ·
Comment /souschef to run again

@pelikhan

Copy link
Copy Markdown
Collaborator

/matt

@github-actions

github-actionsBot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

⚠️ Security scanning failed for Matt Pocock Skills Reviewer. Review the logs for details.

…tions
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>

@github-actionsgithub-actionsBot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Skills-Based Review 🧠

Applied /codebase-design and /diagnosing-bugs — approving with minor suggestions.

📋 Key Themes & Highlights

Key Themes

  • Code duplication: canSuggestWrapStatement is identical in both rule files; a shared utility would prevent the two implementations drifting.
  • Range-check edge case: The write-reference for the declaring identifier sits inside the statement range, so it won't false-positive — but worth an explicit test to document that invariant.

Positive Highlights

  • ✅ Correct diagnosis: wrapping a const/let in a try block strands the binding — the fix targets the right abstraction level.
  • var carved out correctly (function-scoped, so wrapping is safe).
  • ✅ Regression tests added for both rules with the exact motivating code pattern.
  • ✅ Diagnostic is preserved; only the suggestion is suppressed — great UX decision.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 21.3 AIC · ⊞ 7.1K
Comment /matt to run again

CopilotAI commented Aug 7, 2026

Copy link
Copy Markdown
ContributorAuthor

@copilot Please triage this PR, refresh the branch if possible, and run the pr-finisher skill before handing back to maintainers.

Branch refresh is requested if GitHub can update it cleanly....

Completed triage and pr-finisher pass. Addressed the open review feedback in d64588a, validated locally (fmt/lint/test-unit/test/recompile), and branch is already up to date with main.

CopilotAI requested a review from gh-aw-botAugust 7, 2026 14:03
@pelikhan
pelikhan merged commit 9e68218 into mainAug 7, 2026
@pelikhan
pelikhan deleted the copilot/require-fetch-response-body-try-catch-fix branch August 7, 2026 14:09
@github-actions

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.86.1

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

Projects

None yet

4 participants

@gh-aw-bot@pelikhan