Fix miss-match in operator precedence in TS vs GO - #63907
Fix miss-match in operator precedence in TS vs GO#63907Titian Cernicova-Dragomir (dragomirtitian) wants to merge 2 commits into
Conversation
typescript-automation
Bot
added
the
For Uncommitted Bug
There was a problem hiding this comment.
Pull request overview
Corrects SymbolFlags.All generation so TypeScript matches Go.
Changes:
- Parenthesizes the Go expression before TypeScript generation.
- Updates generated enum values.
- Adds synchronous and asynchronous API regression tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
tsc/internal/ast/symbolflags.go |
Clarifies operator precedence. |
packages/typescript/src/enums/symbolFlags.enum.ts |
Corrects the generated enum expression. |
packages/typescript/src/enums/symbolFlags.ts |
Updates the runtime value to 1073741823. |
packages/typescript/test/sync/api.test.ts |
Adds synchronous regression coverage. |
packages/typescript/test/async/api.test.ts |
Adds asynchronous regression coverage. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
I also added a script to validate that the GO and TS values match. I can remove from the PR if it's not wanted. |
| SymbolFlagsGlobalLookup SymbolFlags = 1 << 30 // Flag to signal this is a global lookup | ||
| SymbolFlagsAll SymbolFlags = 1<<30 - 1 // All flags except SymbolFlagsGlobalLookup | ||
| SymbolFlagsGlobalLookup SymbolFlags = 1 << 30 // Flag to signal this is a global lookup | ||
| SymbolFlagsAll SymbolFlags = (1 << 30) - 1 // All flags except SymbolFlagsGlobalLookup. Do not remove () they are needed when the expression is copied to TS. |
There was a problem hiding this comment.
Can we just fix the code gen? We'll totally make this mistake again
There was a problem hiding this comment.
I did add a check in the code gen to ensure that the values from GO and the ones in TS agree. Not sure what you mean by fix. We could look for this case in particular, but feels like there might be others. Checking that at the end both JS and GO see the same value seems like the simplest way to ensure this does not happen again, regardless of expression
There was a problem hiding this comment.
Maybe both, since this pattern is common in bit flags?
There was a problem hiding this comment.
gofumpt will remove things, I'm pretty sure; I'm just asking if we can parse this in the enum code gen and then generate it with correct precedence in TS
There was a problem hiding this comment.
That's also what I'm suggesting, while still keeping the verification script added for hypothetical other future parsing differences
Add
()to make sureSymbolFlags.Allhas the same value in TS as it does in GOFixes #63906