Air breaks code with a variable called else_*

[lorenzwalthert]

Under certain circumstances, air breaks my code. E.g. running air format test.R

# contents of test.R
function() {

  if (TRUE) {
    1
  }

  else_idx <- 1
  if (FALSE) {
    1
  }
}

I get

function() {
  if (TRUE) {
    1
  } else { # <- this line is modified
    _idx <- 1 # <- this line is modified
  } # <- this line is modified
  if (FALSE) {
    1
  }
}

Which should not happen. Seems like else_idx assignment is somehow mistaken as an else statement. {styler}'s rules are mostly based on the tokenized code and not on the text of a token which would provide a first layer of defense against such problems. I guess you do something similar in air? Always, the token is not else but else_*, so I am surprised this bug still occurs. I admit it's an unusual code snippet, but changing users' code (and in this case, the meaning of the code) is in any case an urgent matter. FWIW, when removing if(TRUE) {1} the problem also seems to vanish. I wonder if there are other similar problems with partial token matching.

In {styler}, we assert that the output is parsable. I don't know if you have such a runtime check, but in the above case, it wouldn't help.

[etiennebacher]

The problem is in the tree-sitter parsed output, slightly shorter reprex:

library(treesitter, warn.conflicts = FALSE)

language <- treesitter.r::language()
parser <- parser(language)
text <- "
{
  if (TRUE) 1
  else_idx <- 1
}
"

parser_parse(parser, text)
#> <tree_sitter_tree>
#> 
#> ── Text ────────────────────────────────────────────────────────────────────────
#> 
#> {
#>   if (TRUE) 1
#>   else_idx <- 1
#> }
#> 
#> 
#> ── S-Expression ────────────────────────────────────────────────────────────────
#> (program [(0, 0), (5, 0)]
#>   (braced_expression [(1, 0), (4, 1)]
#>     open: "{" [(1, 0), (1, 1)]
#>     body: (if_statement [(2, 2), (3, 15)]
#>       "if" [(2, 2), (2, 4)]
#>       open: "(" [(2, 5), (2, 6)]
#>       condition: (true [(2, 6), (2, 10)])
#>       close: ")" [(2, 10), (2, 11)]
#>       consequence: (float [(2, 12), (2, 13)])
#>       "else" [(3, 2), (3, 6)]
#>       alternative: (binary_operator [(3, 6), (3, 15)]
#>         lhs: (identifier [(3, 6), (3, 10)])
#>         operator: "<-" [(3, 11), (3, 13)]
#>         rhs: (float [(3, 14), (3, 15)])
#>       )
#>     )
#>     close: "}" [(4, 0), (4, 1)]
#>   )
#> )

And as a result this also affects Jarl:

{
  if (TRUE) 1 
  elsex = 1
}

> jarl check foo2.R --select assignment
warning: assignment
 --> foo2.R:3:7
  |
3 |   elsex = 1
  |       --- Use `<-` for assignment.
  |


── Summary ──────────────────────────────────────
Found 1 error.
1 fixable with the `--fix` option.

(this should highlight elsex = 1, not just x = 1).

[DavisVaughan]

That's unfortunate! I've opened r-lib/tree-sitter-r#200

[DavisVaughan]

With r-lib/tree-sitter-r#201

(now correctly shows if_statement, binary_operator, if_statement)

It'll take some time to flow back through to Air

text <- '
function() {

  if (TRUE) {
    1
  }

  else_idx <- 1
  if (FALSE) {
    1
  }
}
'

treesitter::node_show_s_expression(treesitter::text_parse(text, language()))
#> (program [(0, 0), (12, 0)]
#>   (function_definition [(1, 0), (11, 1)]
#>     name: "function" [(1, 0), (1, 8)]
#>     parameters: (parameters [(1, 8), (1, 10)]
#>       open: "(" [(1, 8), (1, 9)]
#>       close: ")" [(1, 9), (1, 10)]
#>     )
#>     body: (braced_expression [(1, 11), (11, 1)]
#>       open: "{" [(1, 11), (1, 12)]
#>       body: (if_statement [(3, 2), (5, 3)]
#>         "if" [(3, 2), (3, 4)]
#>         open: "(" [(3, 5), (3, 6)]
#>         condition: (true [(3, 6), (3, 10)])
#>         close: ")" [(3, 10), (3, 11)]
#>         consequence: (braced_expression [(3, 12), (5, 3)]
#>           open: "{" [(3, 12), (3, 13)]
#>           body: (float [(4, 4), (4, 5)])
#>           close: "}" [(5, 2), (5, 3)]
#>         )
#>       )
#>       body: (binary_operator [(7, 2), (7, 15)]
#>         lhs: (identifier [(7, 2), (7, 10)])
#>         operator: "<-" [(7, 11), (7, 13)]
#>         rhs: (float [(7, 14), (7, 15)])
#>       )
#>       body: (if_statement [(8, 2), (10, 3)]
#>         "if" [(8, 2), (8, 4)]
#>         open: "(" [(8, 5), (8, 6)]
#>         condition: (false [(8, 6), (8, 11)])
#>         close: ")" [(8, 11), (8, 12)]
#>         consequence: (braced_expression [(8, 13), (10, 3)]
#>           open: "{" [(8, 13), (8, 14)]
#>           body: (float [(9, 4), (9, 5)])
#>           close: "}" [(10, 2), (10, 3)]
#>         )
#>       )
#>       close: "}" [(11, 0), (11, 1)]
#>     )
#>   )
#> )

[lorenzwalthert]

Great 🥳. FWIW: In styler when scope < tokens (i.e. we only modify line breaks and spaces and indention), we assert if the parse tree is the same( ignoring white spaces, line breaks) before and after styling. Not sure you could also do that, but it would also only work if the bug is not coming from the parser itself like it seems the case here 😀.