Go: add isParenthesized relation to keep track of parentheses around expressions - #22605
Merged
Merged
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
owen-mc
force-pushed
the
go/is-parenthesized
branch
from
September 17, 2026 12:57
feae12a to
431b73a
Compare
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The updated whitespace calculation can produce false positives for upgraded databases and whitespace inside parentheses.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced (auto)
Findings: 1
Note
Copilot is running an experiment and ran this review at Balanced.
Open (1)
What changed in this PR
Adds extraction and schema support for tracking parentheses around Go expressions and uses it in whitespace-precedence analysis.
Changes:
- Records each expression’s parenthesis count.
- Updates whitespace calculations to account for parentheses.
- Adds upgrade/downgrade schemas and normalizes generated schema output.
| File | Description |
|---|---|
go/ql/src/InconsistentCode/WhitespaceContradictsPrecedence.ql |
Uses parenthesis counts in whitespace analysis. |
go/ql/lib/upgrades/d0e7336b491e35a4c890e0b9755a4030d32ee444/upgrade.properties |
Defines the schema upgrade. |
go/ql/lib/upgrades/d0e7336b491e35a4c890e0b9755a4030d32ee444/old.dbscheme |
Captures the previous schema. |
go/ql/lib/upgrades/d0e7336b491e35a4c890e0b9755a4030d32ee444/go.dbscheme |
Captures the upgraded schema. |
go/ql/lib/go.dbscheme |
Adds the parenthesis relation. |
go/extractor/extractor.go |
Counts and emits expression parentheses. |
go/extractor/dbscheme/tables.go |
Defines the new extraction table. |
go/extractor/dbscheme/dbscheme.go |
Normalizes generated schema trailing newlines. |
go/downgrades/b4012cfa6fa7de1878c247155b7dfcd76df2604d/upgrade.properties |
Defines removal of the relation. |
go/downgrades/b4012cfa6fa7de1878c247155b7dfcd76df2604d/old.dbscheme |
Captures the pre-downgrade schema. |
go/downgrades/b4012cfa6fa7de1878c247155b7dfcd76df2604d/go.dbscheme |
Captures the downgraded schema. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| private int getNumberOfParentheses(Expr expr) { | ||
| isParenthesized(expr, result) | ||
| or | ||
| not isParenthesized(expr, _) and result = 0 |
Contributor
Author
There was a problem hiding this comment.
I accept this shortcoming for upgraded databases.
Contributor
Author
|
DCA looks good: this gets rid of 3 FPs on one project and doesn't significantly affect performance. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Builds on #22182 . We no longer extract
ParenExpr, so information about whether an expression had parentheses about it was lost. It is occasionally useful, so we put it in a separate table. The upgrade scripts and downgrade scripts lose this information, but this is not very important.