Skip to content

Fix IncorrectIndentation checking for else/if chains #28361

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
jabraham17 wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Fix IncorrectIndentation checking for else/if chains #28361

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6aaa90c
add more uast methods
jabraham17 Jan 30, 2026
eb08b88
fix checking for conditionals
jabraham17 Jan 30, 2026
2a2ccb7
add new tests
jabraham17 Jan 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions test/chplcheck/IncorrectIndentation.chpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -420,4 +420,38 @@ if 1 < 2 {
var a = 10;
var b = 20;
}

proc ifElse() {
if 1 < 5 {
writeln("whee");
} else if 2 > 3 {
writeln("whoopie");
} else if 11 > 3 {
writeln("wooooow");
}

if 1 < 5 then
writeln("whee");
else if 2 > 3 then
writeln("whoopie");
else if 11 > 3 then
writeln("wooooow");

if 1 < 5 {
writeln("whee");
} else if 11 > 3 {
var x = 10;
var y = 20;
}

if 1 < 5 {
writeln("whee");
} else if 11 > 3 {
var x = 10;
var y = 20;
if 1 {
writeln("nested");
}
}
}
}
5 changes: 5 additions & 0 deletions test/chplcheck/IncorrectIndentation.good
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,9 @@ IncorrectIndentation.chpl:416: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:420: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:421: node violates rule ConsecutiveDecls
IncorrectIndentation.chpl:421: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:444: node violates rule ConsecutiveDecls
IncorrectIndentation.chpl:444: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:451: node violates rule ConsecutiveDecls
IncorrectIndentation.chpl:452: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:453: node violates rule IncorrectIndentation
[Success matching fixit for IncorrectIndentation]
34 changes: 34 additions & 0 deletions test/chplcheck/IncorrectIndentation.good-fixit
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -440,4 +440,38 @@ if 1 < 2 {
var a = 10;
var b = 20;
}

proc ifElse() {
if 1 < 5 {
writeln("whee");
} else if 2 > 3 {
writeln("whoopie");
} else if 11 > 3 {
writeln("wooooow");
}

if 1 < 5 then
writeln("whee");
else if 2 > 3 then
writeln("whoopie");
else if 11 > 3 then
writeln("wooooow");

if 1 < 5 {
writeln("whee");
} else if 11 > 3 {
var x = 10;
var y = 20;
}

if 1 < 5 {
writeln("whee");
} else if 11 > 3 {
var x = 10;
var y = 20;
if 1 {
writeln("nested");
}
}
}
}
30 changes: 26 additions & 4 deletions tools/chapel-py/src/method-tables/uast-methods.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,12 +178,30 @@ CLASS_END(Cobegin)
CLASS_BEGIN(Conditional)
PLAIN_GETTER(Conditional, condition, "Get the condition of this Conditional node",
const chpl::uast::AstNode*, return node->condition())
PLAIN_GETTER(Conditional, then_block, "Get the then block of this Conditional node",
const chpl::uast::Block*, return node->thenBlock())
PLAIN_GETTER(Conditional, then_stmts, "Get the statements in the then block of this Conditional node",
IterAdapterBase*, return mkIterPair(node->thenStmts()))
PLAIN_GETTER(Conditional, num_then_stmts, "Get the number of statements in the then block of this Conditional node",
int, return node->numThenStmts())
METHOD(Conditional, then_stmt, "Get the i'th statement in the then block of this Conditional node",
const chpl::uast::AstNode*(int), return node->thenStmt(std::get<0>(args)))
PLAIN_GETTER(Conditional, then_block_style, "Get the block style of the then block of this Conditional node",
const char*, return blockStyleToString(node->thenBlockStyle()))
PLAIN_GETTER(Conditional, has_else_block, "Check if this Conditional node has an else block",
bool, return node->hasElseBlock())
PLAIN_GETTER(Conditional, else_block, "Get the else block of this Conditional node or None if no else block",
Nilable<const chpl::uast::Block*>, return node->elseBlock())
PLAIN_GETTER(Conditional, else_stmts, "Get the statements in the else block of this Conditional node",
IterAdapterBase*, return mkIterPair(node->elseStmts()))
PLAIN_GETTER(Conditional, num_else_stmts, "Get the number of statements in the else block of this Conditional node",
int, return node->numElseStmts())
METHOD(Conditional, else_stmt, "Get the i'th statement in the else block of this Conditional node",
const chpl::uast::AstNode*(int), return node->elseStmt(std::get<0>(args)))
PLAIN_GETTER(Conditional, else_block_style, "Get the block style of the else block of this Conditional node",
const char*, return blockStyleToString(node->elseBlockStyle()))
PLAIN_GETTER(Conditional, is_expression_level, "Checks if this Conditional node is expression-level",
bool, return node->isExpressionLevel())
PLAIN_GETTER(Conditional, then_block, "Get the then block of this Conditional node",
const chpl::uast::Block*, return node->thenBlock())
CLASS_END(Conditional)

CLASS_BEGIN(Comment)
Expand Down Expand Up @@ -369,10 +387,14 @@ CLASS_BEGIN(Yield)
CLASS_END(Yield)

CLASS_BEGIN(SimpleBlockLike)
PLAIN_GETTER(SimpleBlockLike, block_style, "Get the block style of this SimpleBlockLike node",
const char*, return blockStyleToString(node->blockStyle()))
PLAIN_GETTER(SimpleBlockLike, stmts, "Get the statements contained in this SimpleBlockLike.",
IterAdapterBase*, return mkIterPair(node->stmts()))
PLAIN_GETTER(SimpleBlockLike, num_stmts, "Get the number of statements contained in this SimpleBlockLike.",
int, return node->numStmts())
METHOD(SimpleBlockLike, stmt, "Get the i'th statement contained in this SimpleBlockLike.",
const chpl::uast::AstNode*(int), return node->stmt(std::get<0>(args)))
PLAIN_GETTER(SimpleBlockLike, block_style, "Get the block style of this SimpleBlockLike node",
const char*, return blockStyleToString(node->blockStyle()))
CLASS_END(SimpleBlockLike)

CLASS_BEGIN(Begin)
Expand Down
45 changes: 13 additions & 32 deletions tools/chplcheck/src/rules.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,31 +41,6 @@ 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.
"""

# 'else if' statements do not have proper locations
#
# https://github.com/chapel-lang/chapel/issues/25256
if isinstance(node, Conditional):
parent = node.parent()
grandparent = parent.parent() if parent else None
if (
isinstance(parent, Block)
and parent.block_style() == "implicit"
and grandparent
and isinstance(grandparent, Conditional)
):
return True

return False


def fixit_remove_unused_node(
node: AstNode,
lines: Optional[List[str]] = None,
Expand Down Expand Up @@ -677,8 +652,6 @@ 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 and any(
Expand All @@ -703,8 +676,6 @@ def append_nested_single_stmt(node, prev: List[AstNode]):
for stmt in inblock:
if isinstance(stmt, Comment):
continue
if might_incorrectly_report_location(stmt):
continue
prev.append(stmt)
append_nested_single_stmt(stmt, prev)
return node # Return the outermost on to use an anchor
Expand Down Expand Up @@ -1045,10 +1016,8 @@ def unwrap_intermediate_block(node: AstNode) -> Optional[AstNode]:
if isinstance(child, Comment):
continue

if might_incorrectly_report_location(child):
continue
# Empty statements get their own warnings, no need to warn here.
elif isinstance(child, EmptyStmt):
if isinstance(child, EmptyStmt):
continue

line, depth = child.location().start()
Expand Down Expand Up @@ -1084,6 +1053,18 @@ def unwrap_intermediate_block(node: AstNode) -> Optional[AstNode]:
# var x: int;
# }
elif parent_depth and depth == parent_depth:
if (
isinstance(parent_for_indentation, Conditional)
and parent_for_indentation.has_else_block()
and parent_for_indentation.num_else_stmts() == 1
and parent_for_indentation.else_stmt(0).unique_id()
== child.unique_id()
and parent_for_indentation.else_block_style() == "implicit"
):
# don't warn if the child is the only statement in an else implicit block
prev_line = line
prev = child
continue
# only loops and NamedDecls can be anchors for indentation
anchor = (
parent_for_indentation
Expand Down