Skip to content

fix(functemplate): empty ${} inside a call argument truncates the argument list#6846

Open
chuenchen309 wants to merge 1 commit into
beetbox:masterfrom
chuenchen309:fix/functemplate-empty-braces-in-call-arg
Open

fix(functemplate): empty ${} inside a call argument truncates the argument list#6846
chuenchen309 wants to merge 1 commit into
beetbox:masterfrom
chuenchen309:fix/functemplate-empty-braces-in-call-arg

Conversation

@chuenchen309

Copy link
Copy Markdown

A path format like %upper{a${}b} evaluates to A${b} instead of A${}B, and %if{1,a${}b,c} collapses to a${}b — the rest of the template leaks out as literal text.

Cause

Parser.parse_symbol handles "no closing brace found" and "identifier is empty" in the same branch:

closer = self.string.find(GROUP_CLOSE, self.pos)
if closer == -1 or closer == self.pos:
    # No closing brace found or identifier is empty.
    self.parts.append(self.string[start_pos : self.pos])

Both emit ${ as literal text, but the two cases need different amounts of input consumed. With an empty identifier the closing brace exists and is left unconsumed, so parse_argument_list reads it as the enclosing call's terminator. The argument list ends early and the remainder becomes raw text:

%foo{a${}b}     -> Call('foo', [Expression(['a', '${'])]), 'b}'
%foo{a${}b,baz} -> Call('foo', [Expression(['a', '${'])]), 'b,baz}'

At the top level } is not a terminator, so a ${} b parses correctly — which is why the existing test_empty_braces_symbol passes and this went unnoticed. Only nesting inside a function-call argument triggers it.

Fix

Split the branch so an empty identifier consumes through the closing brace. ${} stays literal text in both contexts; the closer == -1 path is unchanged.

Verification

Both new tests fail on master (assert 2 == 1 — the leaked 'b,baz}' part) and pass with the fix. Full test/util/test_functemplate.py passes (49/49), as does test/test_library.py (170 passed). ruff check, ruff format --check, mypy, and docstrfmt --check on the changelog are all clean.


AI disclosure: drafted with Claude Code (Opus 4.8), including the root-cause trace and tests. I ran the repro and the full test suite locally and reviewed the diff before opening this.

@chuenchen309
chuenchen309 requested a review from a team as a code owner July 16, 2026 12:13
…ument list

`Parser.parse_symbol` handled "no closing brace" and "empty identifier" in
the same branch, emitting `${` as literal text without advancing past the
closing brace. The unconsumed `}` was then read by `parse_argument_list` as
the enclosing call's terminator, so the argument list was cut short and the
remainder of the template leaked out as raw text:

    %foo{a${}b}     -> Call('foo', ['a${']) + 'b}'   (was 'A${b}' evaluated)
    %foo{a${}b,baz} -> parsed as one argument + raw 'b,baz}'

At the top level `}` is not a terminator, which is why `a ${} b` parsed
correctly and the existing test did not catch this.

Split the two cases so an empty identifier consumes through the closing
brace, keeping `${}` as literal text in both contexts.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@chuenchen309
chuenchen309 force-pushed the fix/functemplate-empty-braces-in-call-arg branch from 48af8b8 to e71110a Compare July 16, 2026 12:18
@codecov

codecov Bot commented Jul 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.60%. Comparing base (2cf20aa) to head (e71110a).
⚠️ Report is 14 commits behind head on master.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6846   +/-   ##
=======================================
  Coverage   75.60%   75.60%           
=======================================
  Files         163      163           
  Lines       21299    21302    +3     
  Branches     3358     3359    +1     
=======================================
+ Hits        16103    16106    +3     
  Misses       4406     4406           
  Partials      790      790           
Files with missing lines Coverage Δ
beets/util/functemplate.py 88.76% <100.00%> (+0.12%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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