diff --git a/src/tctools/format/format_rules.py b/src/tctools/format/format_rules.py index cc3b234..17fda89 100644 --- a/src/tctools/format/format_rules.py +++ b/src/tctools/format/format_rules.py @@ -368,7 +368,12 @@ def _pad_to_indent_level(self, line: str, tab_index: int): class FormatConditionalParentheses(FormattingRule): - """Formatter to make uses of parentheses inside IF, CASE and WHILE consistent.""" + """Formatter to make uses of parentheses inside IF, CASE and WHILE consistent. + + First regex is used to find potential corrections, which are then investigated + by a loop to make sure parentheses remain matching and no essential parentheses + are removed. + """ def __init__(self, *args): super().__init__(*args) @@ -377,8 +382,7 @@ def __init__(self, *args): "twincat_parentheses_conditionals", value_type=bool ) - # Regex to find conditional inside single lines: - + # Regex to find likely missing parentheses: self._re_needs_parentheses = re.compile( r""" # Look for start of string or new line: @@ -395,11 +399,12 @@ def __init__(self, *args): re.VERBOSE | re.MULTILINE, ) + # Regex to find likely redundant parentheses: self._re_removes_parentheses = re.compile( r""" # Look for start of string or new line: ^ - # Match IF with surrounding ws: + # Match keyword with surrounding ws: \s*(?:IF|WHILE|CASE)\s* # Match any characters within (): \((.+)\) @@ -413,10 +418,11 @@ def format(self, content: List[str], kind: Optional[Kind] = None): if self._parentheses is None: return # Nothing to do - if self._parentheses: - pattern = self._re_needs_parentheses - else: - pattern = self._re_removes_parentheses + pattern = ( + self._re_needs_parentheses + if self._parentheses + else self._re_removes_parentheses + ) for i, line in enumerate(content): # Do a manual match + replace, instead of e.g. subn(), because we might @@ -429,6 +435,13 @@ def format(self, content: List[str], kind: Optional[Kind] = None): if self._parentheses: condition = "(" + condition + ")" else: + # These parentheses could be of importance, check the leading + # "(" is not part of a smaller sub-statement: + if any( + level < 0 for _, level in self.find_and_match_braces(condition) + ): + continue + prefix = prefix[:-1] # Remove parentheses suffix = suffix[1:] @@ -448,3 +461,23 @@ def format(self, content: List[str], kind: Optional[Kind] = None): ) content[i] = prefix + condition + suffix + + @staticmethod + def find_and_match_braces( + text: str, brace_left: str = "(", brace_right: str = ")" + ) -> Tuple[int, int]: + """Step through braces in a string. + + Note that levels can step into negative. + + :return: Tuple of (strpos, level), where strpos is the zero-index position of + the brace itself and level is the nested level it indicates + """ + level = 0 + re_any_brace = re.compile(r"[" + brace_left + brace_right + "]") + for match in re_any_brace.finditer(text): + if match.group() == brace_left: + level += 1 # Nest deeper + else: + level -= 1 # Nest back out + yield match.start(), level diff --git a/tests/test_formatting_rules.py b/tests/test_formatting_rules.py index 46188f4..c30c6bd 100644 --- a/tests/test_formatting_rules.py +++ b/tests/test_formatting_rules.py @@ -274,7 +274,7 @@ def test_variable_align(content, expected, settings): content_parentheses = [ - ( + ( # Regular IF: [ "IF inputs.button = 1 THEN\n", " output.led := 1;\n", @@ -286,7 +286,7 @@ def test_variable_align(content, expected, settings): "END_IF\n", ], ), - ( + ( # Regular IF with comment (comment should be unchanged): [ "IF inputs.button = 1 THEN // comment!\n", ], @@ -294,7 +294,7 @@ def test_variable_align(content, expected, settings): "IF (inputs.button = 1) THEN // comment!\n", ], ), - ( + ( # IF involving a function call (function parentheses should be unchanged): [ "IF func(arg1 := 1, args2 := func2()) THEN\n", ], @@ -302,7 +302,7 @@ def test_variable_align(content, expected, settings): "IF (func(arg1 := 1, args2 := func2())) THEN\n", ], ), - ( + ( # WHILE involving function call (function parentheses should be unchanged): [ "WHILE func() DO // comment!\n", ], @@ -310,7 +310,7 @@ def test_variable_align(content, expected, settings): "WHILE (func()) DO // comment!\n", ], ), - ( + ( # Regular CASE (part of SWITCH): [ "CASE idx OF\n", ], @@ -318,35 +318,43 @@ def test_variable_align(content, expected, settings): "CASE (idx) OF\n", ], ), - # ( # This case fails, because we cannot identify matching parentheses: - # [ - # "IF (1+1)*2 = 3*(x-1) THEN\n", - # ], - # [ - # "IF ((1+1)*2 = 3*(x-1)) THEN\n", - # ], - # ), + ( # IF involving compound statement, parentheses should not be removed at all: + [ + "IF (var1 OR var2) AND (var3 OR var4) THEN\n", + ], + [ + "IF (var1 OR var2) AND (var3 OR var4) THEN\n", + ], + ), + ( # IF involving math statements, parentheses should not be removed at all: + [ + "IF (1+1)*2 = 3*(x-1) THEN\n", + ], + [ + "IF (1+1)*2 = 3*(x-1) THEN\n", + ], + ), ] -@pytest.mark.parametrize("content,expected", content_parentheses) -def test_parentheses_add(content, expected): +@pytest.mark.parametrize("no_parentheses,parentheses", content_parentheses) +def test_parentheses_add(no_parentheses, parentheses): rule = format_rules.FormatConditionalParentheses( {"twincat_parentheses_conditionals": True} ) - content_new = content.copy() + content_new = no_parentheses.copy() rule.format(content_new) - assert content_new == expected + assert content_new == parentheses -@pytest.mark.parametrize("expected,content", content_parentheses) -def test_parentheses_remove(expected, content): +@pytest.mark.parametrize("no_parentheses,parentheses", content_parentheses) +def test_parentheses_remove(no_parentheses, parentheses): rule = format_rules.FormatConditionalParentheses( {"twincat_parentheses_conditionals": False} ) - content_new = content.copy() + content_new = parentheses.copy() rule.format(content_new) - assert content_new == expected + assert content_new == no_parentheses def test_parentheses_remove_no_ws():