Skip to content

Fix function tokens terminating at a nested closing parenthesis#190

Open
jhaygood86 wants to merge 1 commit into
TylerBrinks:masterfrom
jhaygood86:bugfix/lexer-nested-parentheses-in-functions
Open

Fix function tokens terminating at a nested closing parenthesis#190
jhaygood86 wants to merge 1 commit into
TylerBrinks:masterfrom
jhaygood86:bugfix/lexer-nested-parentheses-in-functions

Conversation

@jhaygood86

Copy link
Copy Markdown
Contributor

Problem

Lexer.NewFunction consumes argument tokens until the first RoundBracketClose:

while (token.Type != TokenType.EndOfFile)
{
    function.AddArgumentToken(token);
    if (token.Type == TokenType.RoundBracketClose) break;
    token = Get();
}

A bare parenthesized group inside a function's arguments therefore ends the function early. calc((1px + 2px) * 3) tokenizes as a calc function containing only (1px + 2px), and * 3) is left behind as stray top-level tokens that corrupt the remainder of the value.

Grouping parentheses are explicitly part of the calc() grammar (CSS Values and Units 3 §10.1), and CSS Syntax 3 §5.4.9 "Consume a function" requires consuming until the matching ), not the first one.

Fix

Track the parenthesis depth and terminate only on the ) that matches the function's own (.

A nested function call is already fully consumed by its own recursive NewFunction call before it is added here as a single Function token, so its parentheses never surface as bare RoundBracketOpen/RoundBracketClose tokens at this level and cannot skew the count. min(10px, (2px + 3px)) nested inside calc(...) is covered by a test for exactly that reason.

Unbalanced input still terminates at EOF, unchanged.

Tests

Four tests in Tokenization.cs, asserting both that the function token round-trips to the full source text via ToValue() and that no stray tokens follow it:

  • calc((1px + 2px) * 3) — the base case
  • calc(((1px + 2px) * (3 - 1)) / 2) — multiple nesting levels
  • calc(min(10px, (2px + 3px)) * 2) — nested function alongside a bare group
  • calc((1px + 2px — unbalanced input still stops at EOF

Three of the four fail on master. The full suite (1263 existing tests) stays green, and all seven target frameworks build with no new warnings.

@jhaygood86
jhaygood86 marked this pull request as ready for review July 22, 2026 21:14
@jhaygood86
jhaygood86 marked this pull request as draft July 22, 2026 21:40
@jhaygood86
jhaygood86 marked this pull request as ready for review July 22, 2026 21:43
Lexer.NewFunction consumed argument tokens until the first
RoundBracketClose, so a bare parenthesized group inside a function's
arguments ended the function early. "calc((1px + 2px) * 3)" tokenized as
a calc function containing only "(1px + 2px)", leaving "* 3)" behind as
stray top-level tokens and corrupting the rest of the value.

Track the parenthesis depth instead, so only the ')' matching the
function's own '(' terminates it. A nested function call's parens are
already consumed by its own recursive call before being added here as a
single token, so they never surface as bare bracket tokens at this level
and cannot skew the count.
@jhaygood86
jhaygood86 force-pushed the bugfix/lexer-nested-parentheses-in-functions branch from d83fd3e to 7d88742 Compare July 23, 2026 22:22
@jhaygood86

Copy link
Copy Markdown
Contributor Author

Rebased onto current master.

Note on scope after the rebase: the actual lexer fix (tracking paren depth in NewFunction) already landed on master as part of #213, which carried it as a prerequisite. So this branch no longer changes lexer behaviour — what remains is:

  • the standalone tokenizer regression tests for nested parentheses in a function's arguments, and
  • a comment tidy-up in NewFunction (dropping the now-stale "also submitted standalone…if that merges first" cross-reference, which pointed at this PR).

Happy to close this instead if you'd rather not take the extra tests — the behaviour is already covered on master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant