fix(rust): null is None and .? is unwrap; neither needs an idiom choice - #3261
Merged
Merged
Conversation
The source language spells the empty optional `null` and its unwrap `.?`, and both
reached Rust verbatim: `if (base != null) { base.? }` gives
`cannot find value `null`` and `unexpected token: `?``.
The generated signature is already `base: Option<Vec<u8>>`, so the translation is
direct: `null` is `None`, `.?` is `.unwrap()`. `Option<T>` compares fine, so
`base != None` is the literal rendering of what the spec wrote, and every
occurrence sits behind an `if x != null` the spec put there itself.
Measured: accepts 329 -> 329, zero regressions, but first errors on `?` or `null`
go 8 -> 0 and specs still emitting either go 16 -> 1. Predicted +2 to +6 and
measured +0: the eight move to their next error. Closure is the honest measure.
The one remaining occurrence is `null = 20,` in specs/lsp/schema.t27 -- an enum
variant NAMED null, a legal Rust identifier. The arm is anchored on expression
position and leaves it alone.
Corrects my own pricing: two iterations ago I set this class aside as "needs an
idiom decision, if-let versus unwrap". That decision only exists if you want
idiomatic Rust; the faithful rendering is mechanical, and reading the generated
signature settled it in one command. I had reasoned about the shape of the
expression instead of looking at the type.
Anchored inside expr_to_rust: expr_to_string carries an identical ExprIdentifier
arm, is not a backend, and must keep returning the name.
Closes #3260
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
gHashTag
enabled auto-merge (squash)
September 5, 2026 09:09
Contributor
Contributor
|
📓 NotebookLM Notebook linked to this PR
This notebook contains session context, decisions, and artifacts for this work. |
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
1 participant
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.
The source language spells the empty optional
nulland its unwrap.?. Both reachedRust verbatim:
The generated signature is already
base: Option<Vec<u8>>, so the translation needsno idiom choice:
nullisNone, and.?is.unwrap().Option<T>comparesfine, so
base != Noneis the literal rendering of what the spec wrote, and everyoccurrence measured sits behind an
if x != nullthe spec itself put there.Measured: the class closes, the column does not move
?ornullPredicted +2 to +6 and measured +0: the eight specs move to their next error
rather than to acceptance. Closure is the honest measure here, not yield.
The one remaining occurrence is
null = 20,inspecs/lsp/schema.t27— an enumvariant named
null, which is a legal Rust identifier. The arm is anchored on theexpression position and correctly leaves it alone.
Anchored where it belongs
Inside
expr_to_rust.expr_to_stringcarries an identicalExprIdentifierarm andis not a backend, so it must keep returning the name — the same trap this pass met
when escaping keyword field names, where an assertion on the occurrence count caught
that there were two.
What this corrects in my own reading
I priced this class two iterations ago as "needs an idiom decision (
if letversusunwrap)" and set it aside. That was wrong: the decision only exists if you wantidiomatic Rust. The faithful rendering is mechanical, and checking the generated
signature —
Option<Vec<u8>>— is what settled it in one command.Refs #3249
Closes #3260