feat: implement BEP-020 optional chaining (?.) and null coalescing (??)#3267
feat: implement BEP-020 optional chaining (?.) and null coalescing (??)#3267antoniosarosi wants to merge 1 commit intocanaryfrom
Conversation
Implements the optional chaining and null coalescing operators from BEP-020 across the compiler 2 pipeline (lexer through type inference). The ??= assignment operator is excluded per scope. Lexer: Added ?. as a single token. ?? is handled at parser level (two ? tokens combined) to avoid ambiguity with int?? (double optional). Parser: ?. dispatches to optional field access, index, or call based on the following token. ?? uses Pratt parsing with binding power between assignment and logical OR. AST: Added OptionalFieldAccess, OptionalIndex, OptionalCall expr variants and BinaryOp::NullCoalesce. Type inference: a?.b unwraps Optional, resolves member, re-wraps in Optional. a ?? b unwraps Optional from LHS and joins with RHS type.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds optional chaining ( Changes
Sequence DiagramsequenceDiagram
participant Source as Source Code
participant Lexer as Lexer
participant Parser as Parser
participant AST as AST/Lowering
participant TIR as Type Inference
participant Viz as Visualization
Source->>Lexer: `user?.name ?? "default"`
Lexer->>Lexer: Tokenize: IDENTIFIER, QuestionDot, WORD, QuestionQuestion, STRING
Lexer->>Parser: Token Stream
Parser->>Parser: Parse QuestionDot as infix postfix operator
Parser->>Parser: Parse QuestionQuestion as binary operator
Parser->>AST: OPTIONAL_FIELD_ACCESS_EXPR, BINARY_EXPR(NullCoalesce)
AST->>TIR: Lower to Expr::OptionalFieldAccess & Expr::Binary(NullCoalesce)
TIR->>TIR: Infer OptionalFieldAccess: remove null from base, wrap result in Optional
TIR->>TIR: Infer NullCoalesce: join non-null LHS type with RHS type
TIR->>Viz: Type-inferred expressions
Viz->>Viz: Render `user?.name ?? "default"` for CFG/diagnostic labels
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/tools_onionskin/src/compiler.rs (1)
1938-1995:⚠️ Potential issue | 🟡 MinorUse symbol rendering in the compact TIR2 tree path.
Line 1965 still formats binaries as
{op:?}, so expressions likeu?.name ?? fallbackcollapse to... NullCoalesce ...in the tree output even though the richer renderers print??. That makes the new operator look unsupported in one of the main onionskin views.🛠️ Minimal fix
- Expr::Binary { op, .. } => format!("... {op:?} ..."), + Expr::Binary { op, .. } => format!("... {} ...", binop_sym(op)),
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 16546e43-ca05-4880-884a-9f3a106fa33c
⛔ Files ignored due to path filters (11)
baml_language/crates/baml_tests/snapshots/optional_chaining/baml_tests__optional_chaining__01_lexer__optional_chaining.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/optional_chaining/baml_tests__optional_chaining__02_parser__optional_chaining.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/optional_chaining/baml_tests__optional_chaining__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/optional_chaining/baml_tests__optional_chaining__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/optional_chaining/baml_tests__optional_chaining__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/optional_chaining/baml_tests__optional_chaining__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/optional_chaining/baml_tests__optional_chaining__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/optional_chaining/baml_tests__optional_chaining__10_formatter__optional_chaining.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__chained_optional_field_access.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_chaining_with_null_coalesce.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/src/compiler2_tir/snapshots/baml_tests__compiler2_tir__phase3a__optional_field_access.snapis excluded by!**/*.snap
📒 Files selected for processing (13)
baml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_visualization/src/control_flow/from_ast.rsbaml_language/crates/baml_compiler_lexer/src/tokens.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/syntax_kind.rsbaml_language/crates/baml_tests/projects/optional_chaining/optional_chaining.bamlbaml_language/crates/baml_tests/src/compiler2_tir/inference.rsbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase3a.rsbaml_language/crates/baml_tests/src/compiler2_tir/phase7.rsbaml_language/crates/tools_onionskin/src/compiler.rs
| fn lower_optional_field_access_expr(&mut self, node: &SyntaxNode) -> ExprId { | ||
| // OPTIONAL_FIELD_ACCESS_EXPR: <base_expr> QUESTION_DOT WORD | ||
| let mut base = None; | ||
| let mut field = None; | ||
|
|
||
| for elem in node.children_with_tokens() { | ||
| match elem { | ||
| rowan::NodeOrToken::Node(child) => { | ||
| if base.is_none() { | ||
| base = Some(self.lower_expr(&child)); | ||
| } | ||
| } | ||
| rowan::NodeOrToken::Token(token) => { | ||
| if token.kind() == SyntaxKind::WORD && base.is_some() { | ||
| field = Some(Name::new(token.text())); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| let base = base.unwrap_or_else(|| self.alloc_expr(Expr::Missing, node.text_range())); | ||
| let field = field.unwrap_or_else(|| Name::new("_")); | ||
|
|
||
| self.alloc_expr(Expr::OptionalFieldAccess { base, field }, node.text_range()) |
There was a problem hiding this comment.
foo?.bar currently loses its base during lowering.
For simple bases, the parser hands this node to lowering as WORD QUESTION_DOT WORD with no child expression node. This implementation only fills base from child nodes, so foo?.bar ends up as Expr::OptionalFieldAccess { base: Missing, field: "_" }.
🛠️ Proposed fix
fn lower_optional_field_access_expr(&mut self, node: &SyntaxNode) -> ExprId {
// OPTIONAL_FIELD_ACCESS_EXPR: <base_expr> QUESTION_DOT WORD
let mut base = None;
let mut field = None;
+ let mut seen_question_dot = false;
for elem in node.children_with_tokens() {
match elem {
rowan::NodeOrToken::Node(child) => {
- if base.is_none() {
+ if !seen_question_dot && base.is_none() {
base = Some(self.lower_expr(&child));
}
}
rowan::NodeOrToken::Token(token) => {
- if token.kind() == SyntaxKind::WORD && base.is_some() {
+ if token.kind() == SyntaxKind::QUESTION_DOT {
+ seen_question_dot = true;
+ } else if !seen_question_dot && base.is_none() {
+ base = self.try_lower_bare_token(&token);
+ } else if seen_question_dot
+ && token.kind() == SyntaxKind::WORD
+ && field.is_none()
+ {
field = Some(Name::new(token.text()));
}
}
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn lower_optional_field_access_expr(&mut self, node: &SyntaxNode) -> ExprId { | |
| // OPTIONAL_FIELD_ACCESS_EXPR: <base_expr> QUESTION_DOT WORD | |
| let mut base = None; | |
| let mut field = None; | |
| for elem in node.children_with_tokens() { | |
| match elem { | |
| rowan::NodeOrToken::Node(child) => { | |
| if base.is_none() { | |
| base = Some(self.lower_expr(&child)); | |
| } | |
| } | |
| rowan::NodeOrToken::Token(token) => { | |
| if token.kind() == SyntaxKind::WORD && base.is_some() { | |
| field = Some(Name::new(token.text())); | |
| } | |
| } | |
| } | |
| } | |
| let base = base.unwrap_or_else(|| self.alloc_expr(Expr::Missing, node.text_range())); | |
| let field = field.unwrap_or_else(|| Name::new("_")); | |
| self.alloc_expr(Expr::OptionalFieldAccess { base, field }, node.text_range()) | |
| fn lower_optional_field_access_expr(&mut self, node: &SyntaxNode) -> ExprId { | |
| // OPTIONAL_FIELD_ACCESS_EXPR: <base_expr> QUESTION_DOT WORD | |
| let mut base = None; | |
| let mut field = None; | |
| let mut seen_question_dot = false; | |
| for elem in node.children_with_tokens() { | |
| match elem { | |
| rowan::NodeOrToken::Node(child) => { | |
| if !seen_question_dot && base.is_none() { | |
| base = Some(self.lower_expr(&child)); | |
| } | |
| } | |
| rowan::NodeOrToken::Token(token) => { | |
| if token.kind() == SyntaxKind::QUESTION_DOT { | |
| seen_question_dot = true; | |
| } else if !seen_question_dot && base.is_none() { | |
| base = self.try_lower_bare_token(&token); | |
| } else if seen_question_dot | |
| && token.kind() == SyntaxKind::WORD | |
| && field.is_none() | |
| { | |
| field = Some(Name::new(token.text())); | |
| } | |
| } | |
| } | |
| } | |
| let base = base.unwrap_or_else(|| self.alloc_expr(Expr::Missing, node.text_range())); | |
| let field = field.unwrap_or_else(|| Name::new("_")); | |
| self.alloc_expr(Expr::OptionalFieldAccess { base, field }, node.text_range()) |
| Expr::OptionalCall { callee, args } => { | ||
| self.collect_effective_throws_from_expr(*callee, body, out); | ||
| for arg in args { | ||
| self.collect_effective_throws_from_expr(*arg, body, out); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing transitive throw propagation for OptionalCall.
The collect_effective_throws_from_expr handler for Expr::OptionalCall only collects throws from the callee and argument expressions, but unlike Expr::Call (lines 1553-1567), it doesn't look up the call target's transitive throw set via function_throw_sets. If the callee resolves to a throwing function, those throws won't be propagated.
🐛 Proposed fix
Expr::OptionalCall { callee, args } => {
self.collect_effective_throws_from_expr(*callee, body, out);
for arg in args {
self.collect_effective_throws_from_expr(*arg, body, out);
}
+ // Propagate transitive throws from the callee function (if resolvable).
+ if let Some(target) = self.call_target_name(*callee, body) {
+ let throws = crate::throw_inference::function_throw_sets(
+ self.context.db(),
+ self.package_id,
+ );
+ if let Some(transitive) = throws.transitive_for(&target) {
+ out.extend(transitive.iter().cloned());
+ }
+ }
}| Expr::OptionalCall { callee, args } => { | ||
| self.collect_throw_facts_from_expr(*callee, body, out); | ||
| for arg in args { | ||
| self.collect_throw_facts_from_expr(*arg, body, out); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing transitive throw lookup in collect_throw_facts_from_expr for OptionalCall.
Same issue as collect_effective_throws_from_expr: the Expr::Call handler (lines 1711-1725) includes transitive throw lookup via call_target_name and function_throw_sets, but OptionalCall doesn't.
🐛 Proposed fix
Expr::OptionalCall { callee, args } => {
self.collect_throw_facts_from_expr(*callee, body, out);
for arg in args {
self.collect_throw_facts_from_expr(*arg, body, out);
}
+ if let Some(target) = self.call_target_name(*callee, body) {
+ let throws = crate::throw_inference::function_throw_sets(
+ self.context.db(),
+ self.package_id,
+ );
+ if let Some(transitive) = throws.transitive_for(&target) {
+ out.extend(transitive.iter().cloned());
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Expr::OptionalCall { callee, args } => { | |
| self.collect_throw_facts_from_expr(*callee, body, out); | |
| for arg in args { | |
| self.collect_throw_facts_from_expr(*arg, body, out); | |
| } | |
| } | |
| Expr::OptionalCall { callee, args } => { | |
| self.collect_throw_facts_from_expr(*callee, body, out); | |
| for arg in args { | |
| self.collect_throw_facts_from_expr(*arg, body, out); | |
| } | |
| if let Some(target) = self.call_target_name(*callee, body) { | |
| let throws = crate::throw_inference::function_throw_sets( | |
| self.context.db(), | |
| self.package_id, | |
| ); | |
| if let Some(transitive) = throws.transitive_for(&target) { | |
| out.extend(transitive.iter().cloned()); | |
| } | |
| } | |
| } |
| // Functions using optional chaining and null coalescing. | ||
| // These test that the parser correctly handles the new syntax and | ||
| // that TIR type inference produces correct types. | ||
|
|
||
| // Optional field access: obj?.field | ||
| function OptionalFieldAccess(user: User?) -> string? { | ||
| user?.name | ||
| } | ||
|
|
||
| // Null coalescing: expr ?? default | ||
| function NullCoalesceBasic(x: int?, y: int) -> int { | ||
| x ?? y | ||
| } | ||
|
|
||
| // Optional chaining with null coalescing | ||
| function OptionalChainWithDefault(user: User?, fallback: string) -> string { | ||
| user?.name ?? fallback | ||
| } | ||
|
|
||
| // Chained optional access: obj?.field1?.field2 | ||
| function ChainedOptionalAccess(user: User?) -> string? { | ||
| user?.address?.street | ||
| } | ||
|
|
||
| // Nested optional chaining on nullable field | ||
| function NestedOptionalChain(user: User?) -> string? { | ||
| user?.address?.zip | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Extend this project fixture to hit ?.[ and ?.( too.
Lines 19-42 currently smoke-test ?.field and ??, but the end-to-end project snapshot never exercises OPTIONAL_INDEX_EXPR or OPTIONAL_CALL_EXPR. Adding one array-index case and one optional-call case would keep this fixture aligned with the full BEP-020 surface.
Merging this PR will not alter performance
|
|
No description provided. |
|
|
||
| // Optional field access: obj?.field | ||
| function OptionalFieldAccess(user: User?) -> string? { | ||
| user?.name |
There was a problem hiding this comment.
i think this PR is missing optional chaining assignment:
user?.name?.first = "hi"
It's discussed in the BEP as well (and whether we short circuit the RHS if the LHS is null).
There was a problem hiding this comment.
add a few mroe test cases from the BEP like
user?.(myFunc())?.hi
and (user?.hello).myProp
which tests parentheses as precedence
2 participants
Summary
?.) and null coalescing (??) across the compiler 2 pipeline??=excluded per scope?.lexed as a single token;??handled at parser level (two?tokens) to avoid ambiguity withint??(double optional)obj?.field,obj?.[expr],obj?.(args)a?.bunwraps Optional, resolves member, re-wraps;a ?? bunwraps LHS Optional and joins with RHSTest plan
?.and??tokenization??unwrapping,?.field/index/callprojects/optional_chaining/) across all compiler phasesSummary by CodeRabbit
Release Notes
?.) enables safe property access on nullable objects, supporting field access (obj?.field), indexing (obj?.[expr]), and function calls (func?.(args))??) provides fallback values when expressions evaluate to null, allowingexpr ?? defaultsyntax