-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
Allow namespace star #8
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
This PR introduces a breaking change to the AST format, allowing the use of '*' as a namespace scope in Thrift files.
- Modified
parseNamespaceScope
insrc/main/parser.ts
to handle '*' and identifier tokens - Added
createNamespaceScope
function insrc/main/factory.ts
for creatingNamespaceScope
objects - Updated
src/main/scanner.ts
to recognize '*' as a StarToken - Introduced
NamespaceScope
interface insrc/main/types.ts
, replacingIdentifier
for namespace scopes - Updated test fixtures and solutions in
src/tests/parser/
to reflect the new AST structure
6 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings
@@ -394,18 +396,32 @@ export function createParser( | |||
return null | |||
} | |||
|
|||
// NamespaceScope → '*' | Identifier |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider adding a comment explaining the purpose of this function
if (currentToken().type === SyntaxType.StarToken) { | ||
const loc = currentToken().loc | ||
advance() | ||
return createNamespaceScope('*', loc) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Check for SyntaxType.StarToken before consuming
`Unable to find scope identifier for namespace`, | ||
) | ||
|
||
const scope: NamespaceScope = parseNamespaceScope() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: Add error handling for invalid namespace scopes
** This is a breaking change, it changes the format of the AST **
Closes creditkarma#65