japhib / pico8-ls

PICO-8 Language Server
MIT License
64 stars 8 forks source link

[Bug] Formatter sometimes breaks single-line `if`s #48

Open scambier opened 9 months ago

scambier commented 9 months ago

Under certain conditions, when using the single-line if() shorthand, the formatter incorrectly removes the line return right after

Examples:

-- This works as expected
function foo()
  if (bar) baz()
  lorem = "ipsum"
end

-- Example 1 - Doesn't work when the if() is nested into another block

function foo()
  if true then
    if (bar) baz()
    lorem = "ipsum"
  end
end

-- formatted to:
function foo()
  if true then
    if (bar) baz() lorem = "ipsum"
  end
end

-- Example 2

function foo()
  if true then
    if (bar) baz()
    if 1 == 2 then
      print("hello")
    end
  end
end

-- formatted to
function foo()
  if true then
    if (bar) baz() if 1 == 2 then
      print("hello")
    end
  end
end

A more convoluted example

function update_map()
  for x = 0, 15 do
    for y = 0, 15 do
      local tile = mget(x, y)
      -- bonfire
      if (tile == 56) mset(x, y, 57)
      if (tile == 57) mset(x, y, 56)
      -- water
      local r = r()<.1
      if (tile == 10 and r) mset(x, y, 11)
      if (tile == 11 and r) mset(x, y, 10)
    end
  end
end

is formatted to

function update_map()
  for x = 0, 15 do
    for y = 0, 15 do
      local tile = mget(x, y)
      -- bonfire
      if (tile == 56) mset(x, y, 57) if (tile == 57) mset(x, y, 56)
      -- water local r = r()<.1
      if (tile == 10 and r) mset(x, y, 11) if (tile == 11 and r) mset(x, y, 10)
    end
  end
end

Note the -- water comment that got merged with the line right after

japhib commented 8 months ago

Thanks for reporting! I especially appreciate your specific and in-depth examples 🙂

Single-line ifs are tricky to handle since it's pretty different from the standard Lua grammar. I'll have to add these examples as test cases and make sure the formatter handles them correctly.

For anyone else that might want to take a stab at this, the logic for formatting single-line if statements is here in the visitIfStatement method of formatter.ts, specifically the section inside if (node.oneLine) {.

icegoat9 commented 7 months ago

Thanks for this program. And-- I was about to report a bug with the autoformatter joining commands onto a comment line before them, but I found this bug and I realize everywhere I see this happening in a current program it's adjacent to an if() line, so it sounds like the same root cause. It's odd that having an if() line would cause two different lines to join together... this can at least be another test suite case...

Example:

if (wound) hp-=1
--now check if dead
if hp <= 0 then
  print("dead")
end

autoformats to this:

if (wound) hp -= 1
--now check if dead if hp <= 0 then
  print("dead")
end