Skip to content

Fix strict mode for nested calls and align the parser with R operators - #7

Merged
ymarcon merged 3 commits into
masterfrom
fix/parser-r-compat
Nov 6, 2026
Merged

ymarcon merged 3 commits into
masterfrom
fix/parser-r-compat

Conversation

@ymarcon

@ymarcon ymarcon commented Nov 6, 2026

Copy link
Copy Markdown
Member

Follows up on obiba/datashield4j#8, which fixes operator handling in the Java parsers. DSLite has the same grammar bugs, but with different effects: it writes the AST back as infix text and R re-parses it, so wrong grouping never changed a result. The bugs only caused valid R to be rejected. Checking this also turned up a separate strict mode bypass.

Changes

Strict mode (R/DSLiteServer.R): replaceFunctionNode recursed through self$children, where self is the server rather than the node, so only the outermost call was checked and mapped. With f allowed, f(system('id')), 1 + system('id') and x = system('id') all passed. Nested calls are now checked and mapped.

Lexer and grammar (R/lex-yacc.R):

  • - is no longer part of names, and signs are no longer part of numbers. a-f(1), a-(b), 1-1, x -1 and -1L now parse.
  • ^, %*%, %%, %/% and unary + are supported in expressions.
  • Precedence follows ?Syntax.
  • The separate formula_term grammar is removed and ~ is an ordinary low-precedence operator. y-1 ~ x depended on - being part of names, so it would otherwise stop parsing. y ~ -x and y ~ x -1 now parse. Strings and function calls are still rejected in formulas.

Behaviour changes

  • Scripts with nested calls that are not in the configured methods now fail in strict mode. This is the change most likely to affect downstream tests.
  • -1 is now a UnaryOpNode rather than a NumericNode, and abc-DEF is a subtraction rather than a symbol. The evaluated result is unchanged, but code inspecting the AST would see the difference.
  • a-b is emitted as a - b.

Testing

  • The testthat suite passes, with new tests for nested strict mode checks, minus handling, precedence, % operators, formulas and the round trip through R's parser.
  • 3,768 random arithmetic expressions: all now match parse() after the round trip. Before, 1,923 matched and 1,845 were rejected. None differed silently, before or after.
  • 2,969 random formulas: all match parse(), run after the change only. These comparisons were one-off scripts and are not part of this PR.
  • Not run: R CMD check and the dsBaseClient test suite.

Not addressed

These are valid R and are still rejected: comparison and logical operators, subsetting (a[1]), one-sided formulas (~ x) and .5. f(D$x = 1) still gets past the DSLite parser and is rejected later by parse().

Minus is no longer part of names and signs are no longer part of numbers,
so a-f(1), 1-1 and x -1 parse as in R. Add ^, %*%, %% and %/%, unary plus,
and follow R precedence. Formulas use the expression grammar, so the
response can be an expression, and still reject strings and function calls.
@ymarcon
ymarcon merged commit 14199e8 into master Nov 6, 2026
3 checks passed
@ymarcon
ymarcon deleted the fix/parser-r-compat branch November 6, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant