Skip to content

fix(prefer-nullish-coalescing): emit suggestion over fix#951

Merged
graphite-app[bot] merged 1 commit into
mainfrom
codex/prefer-nullish-suggestions
May 11, 2026
Merged

fix(prefer-nullish-coalescing): emit suggestion over fix#951
graphite-app[bot] merged 1 commit into
mainfrom
codex/prefer-nullish-suggestions

Conversation

@camc314

@camc314 camc314 commented May 11, 2026

Copy link
Copy Markdown
Contributor

related to #950

this is technically unsafe in certain scenarios

const x: boolean | null | undefined;
const y: true;

// for `x = false`, `y = true`: `result = true`
const result = x || y

// for `x = false`, `y = true`: `result = false`
const result = x ?? y

@camc314 camc314 changed the title fix: emit nullish coalescing suggestions fix(prefer-nullish-coalescing): emit suggestion over fix May 11, 2026
@camc314 camc314 self-assigned this May 11, 2026
@camc314 camc314 force-pushed the codex/prefer-nullish-suggestions branch from a7737af to 64ca39d Compare May 11, 2026 20:09
@camc314 camc314 marked this pull request as ready for review May 11, 2026 20:12
Copilot AI review requested due to automatic review settings May 11, 2026 20:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR adjusts the prefer-nullish-coalescing rule to stop emitting auto-fixes and instead emit fix suggestions, reducing the risk of semantics-changing edits being applied implicitly (notably when replacing || with ?? can change behavior for falsy-but-non-nullish values like false).

Changes:

  • Switched prefer-nullish-coalescing from ReportNodeWithFixes to ReportNodeWithSuggestions for ||/||=, ternary, and assignment patterns.
  • Added a dedicated suggestion message id (suggestNullishCoalescing) for the rule’s suggested fixes.
  • Updated rule tests and snapshots to assert suggestions (instead of fixed output) are produced.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
internal/rules/prefer_nullish_coalescing/prefer_nullish_coalescing.go Converts previously auto-applied fixes into explicit suggestions and adds a suggestion message.
internal/rules/prefer_nullish_coalescing/prefer_nullish_coalescing_test.go Updates test expectations from Output autofix results to per-error Suggestions.
internal/rule_tester/snapshots/prefer-nullish-coalescing.snap Refreshes snapshots to include the new suggestion entries in diagnostics output.

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

Comment thread internal/rules/prefer_nullish_coalescing/prefer_nullish_coalescing.go Outdated

camc314 commented May 11, 2026

Copy link
Copy Markdown
Contributor Author

Merge activity

related to #950

this is technically unsafe in certain scenarios

```
const x: boolean | null | undefined;
const y: true;

// for `x = false`, `y = true`: `result = true`
const result = x || y

// for `x = false`, `y = true`: `result = false`
const result = x ?? y
```
@graphite-app graphite-app Bot force-pushed the codex/prefer-nullish-suggestions branch from 0cdd8ca to 74e59a8 Compare May 11, 2026 20:40
@graphite-app graphite-app Bot merged commit 74e59a8 into main May 11, 2026
9 checks passed
@graphite-app graphite-app Bot removed the 0-merge label May 11, 2026
@graphite-app graphite-app Bot deleted the codex/prefer-nullish-suggestions branch May 11, 2026 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants