diff --git a/test/chplcheck/MisleadingIndentation.chpl b/test/chplcheck/MisleadingIndentation.chpl index 92e3abce8efc..7be23f8276a0 100644 --- a/test/chplcheck/MisleadingIndentation.chpl +++ b/test/chplcheck/MisleadingIndentation.chpl @@ -23,4 +23,9 @@ writeln("second thing"); writeln(i); writeln("second thing"); + proc f() { + for 1..10 do + writeln("Hello, world!"); + var unrelated = "hi"; + } } diff --git a/test/chplcheck/MisleadingIndentation.good-fixit b/test/chplcheck/MisleadingIndentation.good-fixit index da921566216f..4930cbb9d44e 100644 --- a/test/chplcheck/MisleadingIndentation.good-fixit +++ b/test/chplcheck/MisleadingIndentation.good-fixit @@ -25,4 +25,9 @@ writeln(i); writeln(i); writeln("second thing"); + proc f() { + for 1..10 do + writeln("Hello, world!"); + var unrelated = "hi"; + } } diff --git a/tools/chplcheck/src/rules.py b/tools/chplcheck/src/rules.py index 0de76b5a9267..9921bc94c6c9 100644 --- a/tools/chplcheck/src/rules.py +++ b/tools/chplcheck/src/rules.py @@ -36,6 +36,31 @@ def variables(node: AstNode): yield from variables(child) +def might_incorrectly_report_location(node: AstNode) -> bool: + """ + Some Dyno AST nodes do not return locations in the way that we expect. + For instance, some NamedDecl nodes currently use the name as the location, + which does not indicate their actual indentation. Rules that depend on + indentation should leave these variables alone. + """ + + # some NamedDecl nodes currently use the name as the location, which + # does not indicate their actual indentation. + # + # https://github.com/chapel-lang/chapel/issues/25208 + if isinstance(node, (VarLikeDecl, TupleDecl, ForwardingDecl)): + return True + + # private function locations are bugged and don't include the 'private' + # keyword. + # + # https://github.com/chapel-lang/chapel/issues/24818 + elif isinstance(node, (Function, Use, Import)) and node.visibility() != "": + return True + + return False + + def fixit_remove_unused_node( node: AstNode, lines: Optional[List[str]] = None, @@ -403,6 +428,8 @@ def MisleadingIndentation(context: Context, root: AstNode): for child in root: if isinstance(child, Comment): continue + if might_incorrectly_report_location(child): + continue yield from MisleadingIndentation(context, child) if prev is not None: @@ -433,6 +460,8 @@ def MisleadingIndentation(context: Context, root: AstNode): for blockchild in reversed(list(grandchildren[-1])): if isinstance(blockchild, Comment): continue + if might_incorrectly_report_location(blockchild): + continue prev = blockchild prevloop = child break @@ -640,22 +669,11 @@ def unwrap_intermediate_block(node: AstNode) -> Optional[AstNode]: if isinstance(child, Comment): continue - # some NamedDecl nodes currently use the name as the location, which - # does not indicate their actual indentation. - if isinstance(child, (VarLikeDecl, TupleDecl, ForwardingDecl)): + if might_incorrectly_report_location(child): continue # Empty statements get their own warnings, no need to warn here. elif isinstance(child, EmptyStmt): continue - # private function locations are bugged and don't include the 'private' - # keyword. - # - # https://github.com/chapel-lang/chapel/issues/24818 - elif ( - isinstance(child, (Function, Use, Import)) - and child.visibility() != "" - ): - continue line, depth = child.location().start()