Skip to content

fix: do.call with non-syntactic function name breaks parsing (#335) - #357

Merged
bburns632 merged 2 commits into
mainfrom
fix/335-do-call-nonsyntactic-name
Sep 1, 2026
Merged

fix: do.call with non-syntactic function name breaks parsing (#335)#357
bburns632 merged 2 commits into
mainfrom
fix/335-do-call-nonsyntactic-name

Conversation

@bburns632

Copy link
Copy Markdown
Collaborator

Fixes #335.

The bug

The error reported in #335the condition has length > 1 — was fixed by #331, which added the [1] subscript to deparse(x[[1]]). But {mlt} still fails to report, one line further down:

# R/FunctionReporter.R
if (deparse(x[[1]])[1] == "do.call" & is.character(x[[2]])){
    x[[2]] <- parse(text=x[[2]])
}

mlt::tmlt and mlt::dmlt call do.call("[<-", ...). parse(text = "[<-") is a syntax error, since a bare replacement operator is not parseable as an expression:

Error in parse(text = x[[2]]) : <text>:1:1: unexpected '['
1: [
    ^

.parse_function() recurses over every expression in the package, so one such call aborts node extraction for the whole package. Those two functions are the only ones in mlt's namespace that trip it.

The fix

Use as.name() instead of parse(text = ). The goal of this block is to turn a string naming a function into a symbol so the recursion can match it against the package namespace, and as.name() does that for any name, syntactic or not. For an ordinary name like do.call("at_bats", ...) the result is unchanged.

Tests

Two unit tests next to the existing .parse_function tests:

  • do.call with a plain string name — guards the Handle functions referenced by name as a string when using do.call() #302 behavior this block exists for, which had no direct unit test (only the baseballstats network fixture).
  • do.call with a non-syntactic name — the regression test for this issue. Confirmed non-vacuous: against unmodified R/FunctionReporter.R it errors with exactly the unexpected '[' message above.

Verification

The reproducer from the issue now completes:

CreatePackageReport('mlt', pkg_reporters = DefaultReporters()[3])
# INFO Done creating package report!

FunctionReporter on mlt yields 154 nodes / 152 edges, where it previously errored. Full default reports for {mlt} and {ggplot2} also render cleanly.

Full local devtools::test(): 250 pass, 2 fail. Both failures are covr::package_coverage() errors on baseballstats (test-FunctionReporter-class.R:91, test-plotting.R:12). They are pre-existing and unrelated — I stashed the diff and reproduced the identical failures on clean main, and CI is green on main at 3c3457e (the commit this branches from). Local environment only; deferring to CI.

🤖 Generated with Claude Code

@bburns632
bburns632 requested review from jameslamb and jayqi September 1, 2026 04:00
.parse_function converted a do.call() string function argument into a
call with parse(text = ...). That fails outright when the string names a
non-syntactic function, e.g. do.call("[<-", ...) in mlt::tmlt, aborting
node extraction for the whole package.

Use as.name() instead, which accepts any name.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@bburns632
bburns632 force-pushed the fix/335-do-call-nonsyntactic-name branch from 7bcd872 to 0affccc Compare September 1, 2026 04:29

@jameslamb jameslamb left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

TIL as.name(), cool!

@bburns632
bburns632 merged commit ce59c97 into main Sep 1, 2026
10 checks passed
@bburns632
bburns632 deleted the fix/335-do-call-nonsyntactic-name branch September 1, 2026 16:09
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.

Failing to generate graph for {mlt}

2 participants