Scony / godot-gdscript-toolkit

Independent set of GDScript tools - parser, linter, formatter, and more
MIT License
1.01k stars 70 forks source link

GDformat: error on ternary operator #70

Closed NathanLovato closed 4 years ago

NathanLovato commented 4 years ago

Here's a line that causes errors:

var direction = 1 if i % 2 == 0 else - 1

Traceback

Traceback (most recent call last):
  File "/home/gdquest/.local/bin/gdformat", line 8, in <module>
    sys.exit(main())
  File "/home/gdquest/.local/lib/python3.7/site-packages/gdtoolkit/formatter/__main__.py", line 56, in main
    given_code_comment_parse_tree=comment_parse_tree,
  File "/home/gdquest/.local/lib/python3.7/site-packages/gdtoolkit/formatter/__init__.py", line 36, in check_formatting_safety
    formatted_code_parse_tree=formatted_code_parse_tree,
  File "/home/gdquest/.local/lib/python3.7/site-packages/gdtoolkit/formatter/safety_checks.py", line 49, in check_tree_invariant
    raise TreeInvariantViolation
AngelOnFira commented 4 years ago

I don't believe this is an issue with the ternary operator in particular, but rather the space between the - and the 1 at the end. Here are is the tree for your line var direction = 1 if i % 2 == 0 else - 1

start
  class_var_stmt
    var_assigned
      direction
      expr
        test_expr
          1
          if
          comparison
            mdr_expr
              i
              %
              2
            ==
            0
          else
          neg_expr
            -
            1

And the tree for var direction = 1 if i % 2 == 0 else - 1

start
  class_var_stmt
    var_assigned
      direction
      expr
        test_expr
          1
          if
          comparison
            mdr_expr
              i
              %
              2
            ==
            0
          else
          -1

And the error is thrown because these aren't the same, specifically at the very end. This is still an issue, however. In the engine, var a = - 1 would set a to -1. On the other hand, the formatter throws the original error with just this snippet.

Scony commented 4 years ago

Soon I'm going to clean up backlog a bit, I'll cover that.

AngelOnFira commented 4 years ago

@Scony just want to check, was this moved to another issue?

Scony commented 4 years ago

@AngelOnFira no, it was closed in b64cf43 - it's not released yet tho. If want to test it, you can pip-install straight from master's HEAD like this: https://adamj.eu/tech/2019/03/11/pip-install-from-a-git-repository/