diff --git a/test/chplcheck/IncorrectIndentation.chpl b/test/chplcheck/IncorrectIndentation.chpl index da21d30c2820..c688c84d76a7 100644 --- a/test/chplcheck/IncorrectIndentation.chpl +++ b/test/chplcheck/IncorrectIndentation.chpl @@ -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"); + } + } + } } diff --git a/test/chplcheck/IncorrectIndentation.good b/test/chplcheck/IncorrectIndentation.good index 57de755a7b97..85cce6d31857 100644 --- a/test/chplcheck/IncorrectIndentation.good +++ b/test/chplcheck/IncorrectIndentation.good @@ -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] diff --git a/test/chplcheck/IncorrectIndentation.good-fixit b/test/chplcheck/IncorrectIndentation.good-fixit index 33df2510df51..027ac5422d96 100644 --- a/test/chplcheck/IncorrectIndentation.good-fixit +++ b/test/chplcheck/IncorrectIndentation.good-fixit @@ -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"); + } + } + } } diff --git a/tools/chapel-py/src/method-tables/uast-methods.h b/tools/chapel-py/src/method-tables/uast-methods.h index a2c98ebe6f68..fc5adf74cd15 100644 --- a/tools/chapel-py/src/method-tables/uast-methods.h +++ b/tools/chapel-py/src/method-tables/uast-methods.h @@ -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, 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) @@ -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) diff --git a/tools/chplcheck/src/rules.py b/tools/chplcheck/src/rules.py index 5e50b015401c..8a57f492c5e3 100644 --- a/tools/chplcheck/src/rules.py +++ b/tools/chplcheck/src/rules.py @@ -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, @@ -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( @@ -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 @@ -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() @@ -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