fix: parse negative and exponent number literals in all four clients - #2575
fix: parse negative and exponent number literals in all four clients#2575diegolopezrm wants to merge 15 commits into
Conversation
…rror
A number literal the scanner accepts but neither language can parse, such
as `${1.2.3}`, left each implementation somewhere different: Dart threw a
`FormatException` from `num.parse` — an error outside the `A2uiError`
hierarchy, which `avoid_catching_errors` discourages catching — while
TypeScript's `Number()` returned NaN and handed it back as a parsed
value, silently placing a non-JSON value in the parse tree.
Both now throw `A2uiExpressionError`, like every other parse failure.
…parser The expression parser behind `formatString` is implemented once per client language, each a port of the others, and nothing compares them. This adds `core/expressions.yaml`: 30 cases covering literals, data bindings, function calls, nested interpolation, escaped markers and the parse errors, expressed once for every implementation to run. Cases are compared with adjacent literal parts joined, so a case fixes what a template means rather than how an implementation splits the literal text around its values. Errors use the suite's language-agnostic categories, with `ParseError` mapping to each SDK's expression error.
Adds the Dart harness for `core/expressions.yaml`. It locates the suite by walking up from the working directory, so it needs no configured path, and maps the suite's error categories onto `A2uiExpressionError`. 30 cases, all passing.
Adds the TypeScript harness for `core/expressions.yaml`, following the same shape as the Dart one: walk up to find the suite, map `ParseError` onto `A2uiExpressionError`, compare with adjacent literals joined. Pulls in js-yaml as a dev dependency to read the suite. 30 cases, all passing.
Both implementations delegated the shape of a number literal to their
platform's parser — `num.parse` and `Number()` — which agree today but
need not: each accepts inputs the grammar never intended, `Number('')`
being 0 among them, and neither is specified anywhere.
The accepted shape is now written out as digits, optionally followed by a
decimal point and more digits, identically in both. `${1.}` moves from
accepted-by-accident (1.0 in Dart, 1 in TypeScript) to rejected in both,
and a conformance case pins that so the answer lives in the suite rather
than in two platform number parsers.
Ran the suite's inputs through the Dart, TypeScript, Python and Swift
parsers. Two cases turned out to encode a choice rather than the shared
behaviour, and both are fixed here.
The number literal grammar rejected a trailing point (`${1.}`). All four
implementations accept it today, so this had turned a unanimous
behaviour into a 2-2 split — the grammar now reads digits, an optional
point, and optional further digits, keeping `${1.2.3}` rejected as all
four already do.
The empty template pinned `[""]`, which only Dart and TypeScript produce;
Python and Swift return `[]`. An empty literal carries no content either
way, so the harnesses now drop empty literals alongside joining adjacent
ones, and the case no longer pins a splitting choice.
All 31 cases now pass in all four implementations.
…ser-conformance # Conflicts: # conformance/README.md # conformance/conformance_schema.json
Requested in review before merging. Both packages get the user-visible half: an invalid number literal now raises the protocol's own error rather than a FormatException in Dart or a NaN in TypeScript, and the accepted shape is stated in each parser instead of inherited from the platform. The web_core entry also notes js-yaml, since it becomes a dev dependency there. a2ui_core's goes under 0.2.0, which is unreleased — pub.dev is still on 0.1.1.
`${-1}` did not fail — it parsed as a data binding to the path `-1`, since
`-` is a valid path character. So `${round(value: -1)}` handed the function
a path where the author wrote a number, and nothing reported it. Exponents
were rejected outright.
The grammar is now the same in Dart, TypeScript, Python and Swift:
'-'? digits ('.' digits?)? (('e'|'E') ('+'|'-')? digits)?
A number starts only when the character is a digit, or a `-` followed by a
digit, so `${a-b}` and `${-a}` are still paths. An exponent that overflows
is rejected rather than returning infinity, which JSON cannot carry — the
same reasoning that made an invalid literal an error rather than NaN.
Nine cases added to conformance/core/expressions.yaml pin all of it,
including the three inputs that have to stay distinguishable: `${-1}`,
`${a-b}` and `${1e}`.
Verified by running the four parsers over the same corpus: 16 inputs
covering negatives, exponents, hyphens in paths, overflow and the
pre-existing forms, agreeing on all 16.
There was a problem hiding this comment.
Code Review
This pull request standardizes the parsing of negative numbers and exponential notation in expression templates across the Python, Dart, TypeScript, and Swift SDKs, and introduces a shared conformance test suite to ensure consistent behavior. Two important issues were identified in the review: first, the Python implementation of parse_number_literal fails to advance past a leading minus sign, which will cause negative numbers to fail parsing; second, the Dart implementation should wrap num.parse in a try-catch block to prevent raw FormatException leaks when parsing extremely large integers that exceed 64-bit limits.
| self.is_digit(scanner.peek()) or scanner.peek() == "." | ||
| ): | ||
| scanner.advance() | ||
| if scanner.peek() in ("e", "E"): |
There was a problem hiding this comment.
In the Python implementation of parse_number_literal, the parser does not advance past the leading minus sign (-) if one is present. This is a critical discrepancy compared to the Dart, TypeScript, and Swift implementations, which all explicitly check for and advance past the minus sign at the start of the function.
Without this advancement, the subsequent while loop (which only consumes digits and dots) will not execute, resulting in an empty num_str (or a string containing only the minus sign if it wasn't consumed), which then fails the NUMBER_LITERAL regex match and raises a ValueError.
To fix this, please add the check to advance past the leading minus sign at the very beginning of parse_number_literal in Python:
def parse_number_literal(self, scanner: Scanner) -> Union[int, float]:
start = scanner.pos
if scanner.peek() == "-":
scanner.advance()
while (
self.is_digit(scanner.peek()) or scanner.peek() == "."
):
scanner.advance()
# ... rest of the functionThere was a problem hiding this comment.
It does advance past it — that's the first thing parse_number_literal does, two lines above the digit loop:
start = scanner.pos
if scanner.peek() == "-":
scanner.advance()Ran the four cases that matter to be sure:
${-1} -> [-1]
${round(value: -1)} -> [{'call': 'round', 'args': {'value': -1}, ...}]
${a-b} -> [{'path': 'a-b'}]
${-a} -> [{'path': '-a'}]
The suggested code matches what's already there.
| if (!_numberLiteral.hasMatch(text)) { | ||
| throw A2uiExpressionError("Invalid number literal: '$text'"); | ||
| } | ||
| final num value = num.parse(text); |
There was a problem hiding this comment.
Although the _numberLiteral regex ensures the string has a valid numeric format, num.parse(text) can still throw a FormatException on some platforms (such as the Dart VM) if the parsed integer is extremely large and exceeds the 64-bit signed integer range (e.g., 99999999999999999999999999999999999999).
Since the CHANGELOG and design goals state that invalid/out-of-range number literals should throw A2uiExpressionError instead of leaking a raw FormatException, we should wrap the num.parse call in a try-catch block to safely catch any FormatException and throw an A2uiExpressionError instead.
| final num value = num.parse(text); | |
| final num value; | |
| try { | |
| value = num.parse(text); | |
| } on FormatException { | |
| throw A2uiExpressionError("Number literal is out of range: '$text'"); | |
| } |
There was a problem hiding this comment.
num.parse falls back to a double when the digits don't fit an int, so it doesn't throw here:
${99999999999999999999999} -> [1e+23]
Same result on the VM and under dart2js (dart test -p chrome), so there's no platform split to guard against. Literals that really are out of range are caught two lines below by the isFinite check — ${1e999} throws A2uiExpressionError.
# Conflicts: # renderers/web_core/CHANGELOG.md
The Dart, web_core and Python changelogs described the a2ui-project#2497 work but not this one. Says what changes for an author: '${-1}' is a number now, '${a-b}' is still a path, and an exponent that overflows is an error.
# Conflicts: # conformance/core/expressions.yaml # dart/a2ui_core/CHANGELOG.md # dart/a2ui_core/lib/src/processing/expressions.dart # renderers/web_core/CHANGELOG.md # renderers/web_core/src/v0_9/basic_catalog/expressions/expression_parser.ts
# Conflicts: # renderers/web_core/CHANGELOG.md
Follow-up to the review discussion in #2497, where you said this deserved its own PR.
The bug isn't that negatives were rejected — it's that
${-1}parsed as a data binding to the path-1, because-is a valid path character.${round(value: -1)}handed the function a path where the author wrote a number, and nothing reported it. Exponents were rejected outright.Same grammar in all four now:
A number starts only on a digit, or on
-followed by a digit, so${a-b}and${-a}stay paths.${1-2}stays an error.One thing I added beyond the ask: an exponent that overflows is now rejected instead of returning infinity. All four produced
Infinityfor${1e999}once exponents worked, and that's the same problem as theNaNwe fixed in #2497 — a value JSON can't carry and no renderer can show.Ten new cases in
conformance/core/expressions.yamlpin the lot, including the three that have to stay distinguishable:${-1},${a-b}and${1e}.I ran the four parsers over the same 16 inputs — negatives, exponents, hyphens in paths, overflow, and the forms that already worked. They agree on all 16. Before the overflow fix they disagreed on exactly one,
${1e999}, which is what turned it up.Suites:
dart test342 passing,node --test511, pytest 24,swift test24. Analyzer, prettier and eslint clean.This branches off #2497, since the conformance suite lands there. Happy to rebase onto main once that merges — or if you'd rather, I can drop the yaml cases here and add them separately.