Skip to content

fix: parse negative and exponent number literals in all four clients - #2575

Open
diegolopezrm wants to merge 14 commits into
a2ui-project:mainfrom
diegolopezrm:fix-number-literals
Open

fix: parse negative and exponent number literals in all four clients#2575
diegolopezrm wants to merge 14 commits into
a2ui-project:mainfrom
diegolopezrm:fix-number-literals

Conversation

@diegolopezrm

Copy link
Copy Markdown
Contributor

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:

'-'? digits ('.' digits?)? ( ('e'|'E') ('+'|'-')? digits )?

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 Infinity for ${1e999} once exponents worked, and that's the same problem as the NaN we fixed in #2497 — a value JSON can't carry and no renderer can show.

Ten new cases in conformance/core/expressions.yaml pin 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 test 342 passing, node --test 511, pytest 24, swift test 24. 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.

…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.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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"):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

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 function

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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.

Suggested change
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'");
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions github-actions Bot added the status: needs-triage auto-managed: https://github.com/a2ui-project/a2ui/blob/main/scripts/triage.mjs label Sep 9, 2026
# 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: needs-triage auto-managed: https://github.com/a2ui-project/a2ui/blob/main/scripts/triage.mjs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant