-
Notifications
You must be signed in to change notification settings - Fork 2
✨ feat(lang): add coalesce operator (??) support #706
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
Conversation
Add support for the null coalescing operator (??) to the mq language. The operator provides a concise way to handle null/none values by returning the right operand when the left operand is null or none. - Add COALESCE constant to ast/constants.rs - Implement TokenKind::Coalesce in lexer - Add coalesce operator parsing with precedence level 6 - Implement coalesce builtin function in eval/builtin.rs - Include comprehensive test cases for operator and function behavior - Add documentation for coalesce function
CodSpeed Performance ReportMerging #706 will not alter performanceComparing Summary
|
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.
Pull Request Overview
This PR adds support for the null coalescing operator (??
) to the mq language, providing a concise way to handle null/none values by returning the right operand when the left operand is null or none.
Key changes:
- Introduces
TokenKind::Coalesce
for lexer support of the??
operator - Implements coalesce operator parsing with precedence level 6 (same as range operator)
- Adds
COALESCE
builtin function that returns the first non-None value from two arguments
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
crates/mq-lang/src/lexer/token.rs | Adds TokenKind::Coalesce variant and its display implementation, reorganizes enum variants alphabetically |
crates/mq-lang/src/lexer.rs | Adds coalesce token parser and includes it in punctuation parsing |
crates/mq-lang/src/eval/builtin.rs | Implements COALESCE builtin function with documentation and hash mapping |
crates/mq-lang/src/eval.rs | Adds comprehensive test cases for coalesce function behavior |
crates/mq-lang/src/cst/parser.rs | Adds coalesce binary operation support to CST parser with test case |
crates/mq-lang/src/cst/node.rs | Adds BinaryOp::Coalesce variant to node types |
crates/mq-lang/src/ast/parser.rs | Implements coalesce operator precedence and AST parsing with extensive test coverage |
crates/mq-lang/src/ast/constants.rs | Defines COALESCE constant for the operator name |
alt(( | ||
and, or, l_paren, r_paren, l_brace, r_brace, comma, colon, semi_colon, l_bracket, | ||
and, or, coalesce, l_paren, r_paren, l_brace, r_brace, comma, colon, semi_colon, l_bracket, | ||
r_bracket, question, pipe, |
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.
The coalesce parser should be ordered by precedence or token length. Since ??
is two characters and ?
is one character, coalesce should come before question in the alt() parser to ensure proper tokenization of ??
before attempting to parse it as two separate ?
tokens.
r_bracket, question, pipe, | |
r_bracket, pipe, question, |
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
fn punctuations(input: Span) -> IResult<Span, Token> { | ||
alt(( | ||
and, or, l_paren, r_paren, l_brace, r_brace, comma, colon, semi_colon, l_bracket, | ||
and, or, coalesce, l_paren, r_paren, l_brace, r_brace, comma, colon, semi_colon, l_bracket, |
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.
The coalesce parser should be placed after question
in the alt() chain to ensure proper parsing precedence. Since ??
starts with ?
, the single ?
parser might consume the first character before ??
can be matched.
Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
[a, b] => { | ||
if a.is_none() { | ||
Ok(std::mem::take(b)) | ||
} else { | ||
Ok(std::mem::take(a)) | ||
} | ||
} |
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.
The coalesce function implementation lacks documentation explaining its null coalescing behavior. Add a doc comment describing that it returns the first non-None value from the two arguments.
Copilot uses AI. Check for mistakes.
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.
Pull Request Overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Add support for the null coalescing operator (??) to the mq language. The operator provides a concise way to handle null/none values by returning the right operand when the left operand is null or none.