From 4b26235f2fd2cf8e0b348c9342f2cf94341f5bf2 Mon Sep 17 00:00:00 2001 From: shap-po <50052124+shap-po@users.noreply.github.com> Date: Tue, 5 Aug 2025 21:35:02 +0300 Subject: [PATCH] fix: Formatter adding unnecessary extra parenthesis around expressions inside function calls and arrays closes #305 --- gdtoolkit/formatter/context.py | 1 + gdtoolkit/formatter/expression.py | 56 +++++++++++------- ...05_multiline_lists_extra_parenthesis.in.gd | 21 +++++++ ...5_multiline_lists_extra_parenthesis.out.gd | 59 +++++++++++++++++++ .../bug_334_multiline_lambda.out.gd | 14 ++--- 5 files changed, 122 insertions(+), 29 deletions(-) create mode 100644 tests/formatter/input-output-pairs/bug_305_multiline_lists_extra_parenthesis.in.gd create mode 100644 tests/formatter/input-output-pairs/bug_305_multiline_lists_extra_parenthesis.out.gd diff --git a/gdtoolkit/formatter/context.py b/gdtoolkit/formatter/context.py index 61496df..9157d67 100644 --- a/gdtoolkit/formatter/context.py +++ b/gdtoolkit/formatter/context.py @@ -54,3 +54,4 @@ class ExpressionContext: prefix_line: int # earliest line number of prefix string suffix_string: str suffix_line: int # earliest line number of suffix string + is_inside_list: bool = False diff --git a/gdtoolkit/formatter/expression.py b/gdtoolkit/formatter/expression.py index 3145768..9d21410 100644 --- a/gdtoolkit/formatter/expression.py +++ b/gdtoolkit/formatter/expression.py @@ -437,15 +437,20 @@ def _format_operator_chain_based_expression_to_multiple_lines( ) or ( expression_context.prefix_string.endswith("[") and expression_context.suffix_string.startswith("]") - ) + ) or expression_context.is_inside_list lpar = "" if inside_par else "(" rpar = "" if inside_par else ")" - child_context = context.create_child_context(expression_context.prefix_line) + child_context = ( + context + if expression_context.is_inside_list + else context.create_child_context(expression_context.prefix_line) + ) child_expression_context = ExpressionContext( "", expression_context.prefix_line, - "", + "" if not expression_context.is_inside_list else expression_context.suffix_string, expression_context.suffix_line, + is_inside_list=inside_par, ) fake_meta = Meta() fake_meta.line = expression_context.prefix_line @@ -453,30 +458,34 @@ def _format_operator_chain_based_expression_to_multiple_lines( fake_expression = Tree( "contextless_operator_chain_based_expression", expression.children, fake_meta ) - formatted_lines = [ - ( - expression_context.prefix_line, - "{}{}{}".format( - context.indent_string, expression_context.prefix_string, lpar - ), + formatted_lines = [] # type: FormattedLines + # left and right parentheses are added only if the expression is not inside of a list + if not expression_context.is_inside_list: + formatted_lines.append( + ( + expression_context.prefix_line, + "{}{}{}".format( + context.indent_string, expression_context.prefix_string, lpar + ), + ) ) - ] # type: FormattedLines formatted_lines += _format_concrete_expression( fake_expression, child_expression_context, child_context ) - formatted_lines.append( - ( - get_end_line(expression.children[-1]), - "{}{}{}".format( - context.indent_string, rpar, expression_context.suffix_string - ), + if not expression_context.is_inside_list: + formatted_lines.append( + ( + get_end_line(expression.children[-1]), + "{}{}{}".format( + context.indent_string, rpar, expression_context.suffix_string + ), + ) ) - ) return formatted_lines def _format_contextless_operator_chain_based_expression_to_multiple_lines( - expression: Tree, _: ExpressionContext, context: Context + expression: Tree, expression_context: ExpressionContext, context: Context ) -> FormattedLines: formatted_lines = [] # type: FormattedLines value = expression.children[0] @@ -491,7 +500,7 @@ def _format_contextless_operator_chain_based_expression_to_multiple_lines( ExpressionContext( f"{expression_to_str(operator)} ", get_line(child), - "", + expression_context.suffix_string, get_end_line(child), ), context, @@ -510,6 +519,7 @@ def _format_comma_separated_list_to_multiple_lines( expression_context.prefix_line, "", expression_context.suffix_line, + is_inside_list=True ) fake_meta = Meta() fake_meta.line = expression_context.prefix_line @@ -535,7 +545,7 @@ def _format_comma_separated_list_to_multiple_lines( def _format_contextless_comma_separated_list_to_multiple_lines( - expression: Tree, _: ExpressionContext, context: Context + expression: Tree, expression_context: ExpressionContext, context: Context ) -> FormattedLines: a_list = expression.children elements = [node for node in a_list if not is_any_comma(node)] @@ -544,7 +554,11 @@ def _format_contextless_comma_separated_list_to_multiple_lines( for i, element in enumerate(elements): suffix = "," if i != len(elements) - 1 or trailing_comma_present else "" child_expression_context = ExpressionContext( - "", get_line(element), suffix, get_end_line(element) + "", + get_line(element), + suffix, + get_end_line(element), + is_inside_list=expression_context.is_inside_list ) lines = _format_standalone_expression( element, child_expression_context, context diff --git a/tests/formatter/input-output-pairs/bug_305_multiline_lists_extra_parenthesis.in.gd b/tests/formatter/input-output-pairs/bug_305_multiline_lists_extra_parenthesis.in.gd new file mode 100644 index 0000000..4c450ed --- /dev/null +++ b/tests/formatter/input-output-pairs/bug_305_multiline_lists_extra_parenthesis.in.gd @@ -0,0 +1,21 @@ +func _ready(): + print("Long string with added formatting %d and %s ......................................." % [1, "string"]) + print("Long string with added formatting %d and %s ......................................." % [1, "string"], "Long string with added formatting %d and %s ......................................." % [1, "string"]) + + var very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii: int = 0 + var very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii: int = 1 + + print(very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii) + print(very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii - very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii * very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii / very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii & very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii | very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii) + + var array1 = ["Long string with added formatting %d and %s ......................................." % [1, "string"], very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii] + print(array1) + + var array2 = ["Long string with added formatting %d and %s ......................................." % [1, "string"]] + print(array2) + + var array3 = [very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii] + print(array3) + + var array4 = [very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii,] + print(array4) diff --git a/tests/formatter/input-output-pairs/bug_305_multiline_lists_extra_parenthesis.out.gd b/tests/formatter/input-output-pairs/bug_305_multiline_lists_extra_parenthesis.out.gd new file mode 100644 index 0000000..29a926d --- /dev/null +++ b/tests/formatter/input-output-pairs/bug_305_multiline_lists_extra_parenthesis.out.gd @@ -0,0 +1,59 @@ +func _ready(): + print( + "Long string with added formatting %d and %s ......................................." + % [1, "string"] + ) + print( + "Long string with added formatting %d and %s ......................................." + % [1, "string"], + "Long string with added formatting %d and %s ......................................." + % [1, "string"] + ) + + var very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii: int = 0 + var very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii: int = 1 + + print( + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii + ) + print( + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + - very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + * very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + / very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + & very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + | very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii + ) + + var array1 = [ + "Long string with added formatting %d and %s ......................................." + % [1, "string"], + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii + ] + print(array1) + + var array2 = [ + "Long string with added formatting %d and %s ......................................." + % [1, "string"] + ] + print(array2) + + var array3 = [ + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii + ] + print(array3) + + var array4 = [ + very_long_variable_name1_iiiiiiiiiiiiiiiiiiiiii + + very_long_variable_name2_iiiiiiiiiiiiiiiiiiiiii, + ] + print(array4) diff --git a/tests/formatter/input-output-pairs/bug_334_multiline_lambda.out.gd b/tests/formatter/input-output-pairs/bug_334_multiline_lambda.out.gd index bc82ce6..f0d808d 100644 --- a/tests/formatter/input-output-pairs/bug_334_multiline_lambda.out.gd +++ b/tests/formatter/input-output-pairs/bug_334_multiline_lambda.out.gd @@ -6,12 +6,10 @@ class WeaponSystemBullet: func foo(): var bullet_scene assert( - ( - (func() -> bool: - var test_instance: Node = bullet_scene.instantiate() - var is_needed_class: bool = test_instance is WeaponSystemBullet - test_instance.free() - return is_needed_class) - . call() - ) + (func() -> bool: + var test_instance: Node = bullet_scene.instantiate() + var is_needed_class: bool = test_instance is WeaponSystemBullet + test_instance.free() + return is_needed_class) + . call() )