Skip to content

fix(noNestedTernary): detect nested ternaries wrapped in parentheses #8086

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
ematipico merged 2 commits into from
Nov 12, 2025
Merged

fix(noNestedTernary): detect nested ternaries wrapped in parentheses #8086

ematipico merged 2 commits into from
Nov 12, 2025

Conversation

@matanshavit
Copy link
Contributor

@matanshavit matanshavit commented Nov 12, 2025 •
edited
Loading

AI Assistance Notice

This PR was written primarily with Claude Code. See Contribution Guidelines - AI Assistance.

Summary

Fixes #8045

The noNestedTernary rule now correctly detects nested ternary expressions even when they are wrapped in parentheses.

Previously, the rule would not flag nested ternaries like foo ? (bar ? 1 : 2) : 3 because the parentheses prevented detection.

Test Plan

I added valid and invalid tests with ternary operators in parenthesis.

Docs

Changeset included

Implementation notes

This is a two line change to production code, using consequent.omit_parentheses() in place of consequent.

@changeset-bot
Copy link

changeset-bot bot commented Nov 12, 2025 •
edited
Loading

🦋 Changeset detected

Latest commit: 2fdaf13

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 13 packages
Name Type
@biomejs/biome Patch
@biomejs/cli-win32-x64 Patch
@biomejs/cli-win32-arm64 Patch
@biomejs/cli-darwin-x64 Patch
@biomejs/cli-darwin-arm64 Patch
@biomejs/cli-linux-x64 Patch
@biomejs/cli-linux-arm64 Patch
@biomejs/cli-linux-x64-musl Patch
@biomejs/cli-linux-arm64-musl Patch
@biomejs/wasm-web Patch
@biomejs/wasm-bundler Patch
@biomejs/wasm-nodejs Patch
@biomejs/backend-jsonrpc Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added A-Linter Area: linter L-JavaScript Language: JavaScript and super languages labels Nov 12, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Nov 12, 2025 •
edited
Loading

CodSpeed Performance Report

Merging #8086 will not alter performance

Comparing matanshavit:fix/8045-noNestedTernary-parentheses (2fdaf13) with main (5cd3d27)

Summary

✅ 58 untouched
⏩ 95 skipped1

Footnotes

  1. 95 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

@matanshavit matanshavit marked this pull request as ready for review November 12, 2025 16:17
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 12, 2025 •
edited
Loading

Walkthrough

This change updates the noNestedTernary lint rule to unwrap surrounding parentheses on the consequent and alternate expressions before checking for nested conditional expressions. The rule now calls omit_parentheses() (or equivalent) when matching JsConditionalExpression, enabling detection of nested ternaries wrapped in parentheses. Tests were expanded with multiple parenthesised and non‑parenthesised nested ternary cases, and a changelog entry documenting the patch release was added. No exported or public API declarations were changed.

Suggested reviewers

  • ematipico
  • dyc3

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely describes the fix: enabling detection of nested ternaries wrapped in parentheses, which is the main change across all files.
Description check ✅ Passed The description clearly explains the problem, solution, test coverage, and implementation approach, all directly relevant to the changeset.
Linked Issues check ✅ Passed The PR successfully addresses issue #8045 by modifying the noNestedTernary rule to detect nested ternaries wrapped in parentheses using omit_parentheses(), with comprehensive test coverage added.
Out of Scope Changes check ✅ Passed All changes are directly within scope: production logic fix, changeset documentation, invalid test cases, and valid test cases for the parenthesised ternary detection feature.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d2de9f and 2fdaf13.

📒 Files selected for processing (1)
  • .changeset/fix-nested-ternary-parentheses.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • .changeset/fix-nested-ternary-parentheses.md

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

dyc3
dyc3 approved these changes Nov 12, 2025
View reviewed changes
Copy link
Contributor

@dyc3 dyc3 left a comment

Choose a reason for hiding this comment

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

Love to see an easy fix

@ematipico ematipico merged commit 2b41e82 into biomejs:main Nov 12, 2025
18 checks passed
@github-actions github-actions bot mentioned this pull request Nov 12, 2025
Merged
ematipico pushed a commit to hamirmahal/biome that referenced this pull request Nov 19, 2025
@matanshavit @dyc3 @ematipico
fix(noNestedTernary): detect nested ternaries wrapped in parentheses (b…
ac86030 
…iomejs#8086)

Co-authored-by: Carson McManus <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dyc3 dyc3 dyc3 approved these changes

Assignees

No one assigned

Labels

A-Linter Area: linter L-JavaScript Language: JavaScript and super languages

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

💅 lint/style/noNestedTernary doesn't work with parentheses

3 participants

@matanshavit @dyc3 @ematipico