Surprising behaviour when evaluating syntactically incorrect code

[lionel-]

Take this long function which has a syntax error:

f <- function() {
  (  # Syntactically invalid
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
  NULL
}

Try to evaluate it with cmd+enter. Only f gets sent at the console, which is surprising.

Screen.Recording.2024-10-15.at.15.16.23.mov

Now select the function and evaluate with cmd+enter:

• The selection is written to the console but not sent to the kernel.

• The code sent to the console is not scrolled down. Scrolling with the trackpad/mousewheel doesn't work.

Screen.Recording.2024-10-15.at.15.08.56.mov

This behaviour makes it seem like Positron is stuck. In reality the user can interrupt, and can fix the code before sending it, but it's really not obvious what is expected of the user.

I'm thinking we should avoid sending any code to the kernel or console when there is a syntax error and that instead we should notify the user that the code must be fixed before being evaluated. Either with a notification or with a dialog. While a dialog is more intrusive, I think it wouldn't be too bad here, to make it very clear that the user is responsible for fixing the syntax error.

[DavisVaughan]

I've seen this before and I think we aren't correctly distinguishing between these 3 in the Console:

• Valid
• Syntax error
• Incomplete

Incomplete should result in the behavior you see here, so the user can keep typing

But with syntax errors I think we should either:

• Push it through anyways and let the kernel throw a syntax error (this is what rstudio does for your 2nd case)
• Do what you said and not even write it to the console, instead pushing a dialog

---

I'm not entirely sure how to fix the Statement Range issue in your first case. I wonder what's happening internally there, is ark saying that f is the next thing to run? Or is this some fallback behavior?

It's possible that on the ark side we have parsed this file with tree-sitter and it succeeded with error recovery, but is really still quite broken, and somehow f looks like the next selection to run. Maybe we should bail in Statement Range if we detect any ERROR nodes. Then in theory I think the Statement Range provider would fall back to only sending the current line, which I think is also what RStudio does here.

Update: Yea I think that is probably what is happening, note the "successful" (identifier [(0, 0), (0, 1)]) even though we have an ERROR immediately after it. We should probably not send anything back here.

treesitter::text_parse(x, treesitter.r::language()) |>
  treesitter::node_show_s_expression()
#> (program [(0, 0), (20, 1)]
#>   (identifier [(0, 0), (0, 1)])
#>   (ERROR [(0, 2), (20, 1)]
#>     "<-" [(0, 2), (0, 4)]
#>     "function" [(0, 5), (0, 13)]
#>     (parameters [(0, 13), (0, 15)]
#>       open: "(" [(0, 13), (0, 14)]
#>       close: ")" [(0, 14), (0, 15)]
#>     )
#>     "{" [(0, 16), (0, 17)]
#>     "(" [(1, 2), (1, 3)]
#>     (comment [(1, 5), (1, 28)])
#>     body: (null [(2, 4), (2, 8)])
#>     body: (null [(3, 4), (3, 8)])
#>     body: (null [(4, 4), (4, 8)])
#>     body: (null [(5, 4), (5, 8)])
#>     body: (null [(6, 4), (6, 8)])
#>     body: (null [(7, 4), (7, 8)])
#>     body: (null [(8, 4), (8, 8)])
#>     body: (null [(9, 4), (9, 8)])
#>     body: (null [(10, 4), (10, 8)])
#>     body: (null [(11, 4), (11, 8)])
#>     body: (null [(12, 4), (12, 8)])
#>     body: (null [(13, 4), (13, 8)])
#>     body: (null [(14, 4), (14, 8)])
#>     body: (null [(15, 4), (15, 8)])
#>     body: (null [(16, 4), (16, 8)])
#>     body: (null [(17, 4), (17, 8)])
#>     body: (null [(18, 4), (18, 8)])
#>     body: (null [(19, 4), (19, 8)])
#>     (ERROR [(20, 0), (20, 1)])
#>   )
#> )

[lionel-]

oh good idea regarding sending broken code to the kernel

[lionel-]

This tripped up @hadley yesterday

[lionel-]

Another weirdness: Because the code doesn't parse the statement range provider doesn't work and we fall back to line-by-line eval. That's expected but when you repeatedly evaluate incomplete statements in this way, the previous input gets replaced:

Screen.Recording.2025-07-03.at.21.05.54.mov

Might be related to #4150