Skip to content

fix(no-unnecessary-condition): use static values for literal comparisons#833

Closed
camc314 wants to merge 3 commits into
mainfrom
c/03-25-fix_no-unnecessary-condition_use_static_values_for_literal_comparisons
Closed

fix(no-unnecessary-condition): use static values for literal comparisons#833
camc314 wants to merge 3 commits into
mainfrom
c/03-25-fix_no-unnecessary-condition_use_static_values_for_literal_comparisons

Conversation

@camc314

@camc314 camc314 commented Mar 25, 2026

Copy link
Copy Markdown
Contributor

No description provided.

camc314 commented Mar 25, 2026

Copy link
Copy Markdown
Contributor Author

How to use the Graphite Merge Queue

Add the label 0-merge to this PR to add it to the merge queue.

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.

@camc314 camc314 marked this pull request as ready for review March 25, 2026 17:49
Copilot AI review requested due to automatic review settings March 25, 2026 17:49

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

Updates no-unnecessary-condition to evaluate literal-to-literal comparisons using computed static values, and improves the diagnostic message to state whether the comparison is always true/false.

Changes:

  • Add static value extraction for number and bigint literal types (instead of comparing by string form).
  • Implement constant-folding logic for ==, ===, <, <=, >, >=, !=, !== across static literal values.
  • Update rule snapshots (unit + e2e) to reflect the new, more specific diagnostic message text.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
internal/rules/no_unnecessary_condition/no_unnecessary_condition.go Adds static-value extraction and constant-folding for literal comparisons; updates diagnostic message construction.
internal/rule_tester/snapshots/no-unnecessary-condition.snap Updates expected diagnostics text for literal comparison cases.
e2e/snapshots/snapshot.test.ts.snap Updates expected e2e diagnostics JSON for the rule message change.

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

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

@camc314 camc314 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is too complicated for not much value - we are basically re-implementing JS in go which is not sensible

@camc314 camc314 closed this Mar 25, 2026
graphite-app Bot pushed a commit that referenced this pull request Mar 27, 2026
see #833 for reasoning - we may start using this in future.

In the mean time it makes sense to keep it to align with the upstream impl
@Boshen Boshen deleted the c/03-25-fix_no-unnecessary-condition_use_static_values_for_literal_comparisons branch April 29, 2026 06:47
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