From 403f863029d166cbdca61002795978c1adc2d930 Mon Sep 17 00:00:00 2001
From: Danila Fedorin <daniel.fedorin@hpe.com>
Date: Mon, 10 Jun 2024 16:11:03 -0700
Subject: [PATCH] Fix spurious warning with misleading indentation

This was caused by the incorrect location reporting for variables.
If the code above is 4-indented, and then location of the name is used,
that name is also 4-indented, which flags as misleading.

The solution is to steal the logic from IncorrectIndentation that avoids
AST nodes that report their locations oddly.

Signed-off-by: Danila Fedorin <daniel.fedorin@hpe.com>
---
 tools/chplcheck/src/rules.py | 43 ++++++++++++++++++++++++++----------
 1 file changed, 31 insertions(+), 12 deletions(-)

diff --git a/tools/chplcheck/src/rules.py b/tools/chplcheck/src/rules.py
index 0de76b5a9267..256a8c369fd9 100644
--- a/tools/chplcheck/src/rules.py
+++ b/tools/chplcheck/src/rules.py
@@ -36,6 +36,32 @@ 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.
+    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 +429,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 +461,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 +670,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()