Skip to content

Commit

Permalink
Fix spurious warning with misleading indentation
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
DanilaFe committed Jun 10, 2024
1 parent e266bd5 commit 403f863
Showing 1 changed file with 31 additions and 12 deletions.
43 changes: 31 additions & 12 deletions tools/chplcheck/src/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()

Expand Down

0 comments on commit 403f863

Please sign in to comment.