Skip to content
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.

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

Fix several CLS bugs #25843

Merged
merged 9 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1,376 changes: 689 additions & 687 deletions frontend/lib/parsing/bison-chpl-lib.cpp

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions frontend/lib/parsing/chpl.ypp
Original file line number Diff line number Diff line change
Expand Up @@ -2290,6 +2290,7 @@ enum_item:
auto decl = EnumElement::build(BUILDER, LOC(@$),
context->buildAttributeGroup(@$),
$1);
BUILDER->noteDeclNameLocation(decl.get(), LOC(@1));
$$ = STMT(@$, decl.release());
}
| ident_def TASSIGN expr
Expand All @@ -2298,6 +2299,7 @@ enum_item:
context->buildAttributeGroup(@$),
$1,
toOwned($3));
BUILDER->noteDeclNameLocation(decl.get(), LOC(@1));
$$ = STMT(@$, decl.release());
context->clearCommentsBefore(@3);
}
Expand Down
25 changes: 25 additions & 0 deletions test/chplcheck/IncorrectIndentation.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -358,4 +358,29 @@ module IncorrectIndentation {
writeln("hi");
}
}


if 1 < 2 {
writeln("hi");
if 2 < 3 {
writeln("hi");
writeln("??");
}
}
// since else statements aren't reported correctly only the misaligned child statements should warn
if 1 < 2 {
writeln("hi");
writeln("??");
} else if 2 < 3 {
writeln("hi");
writeln("??");
} else {
writeln("hi");
writeln("??");
}
if 1 < 2 {
if 3 < 4 {

}
}
}
5 changes: 5 additions & 0 deletions test/chplcheck/IncorrectIndentation.good
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,9 @@ IncorrectIndentation.chpl:339: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:349: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:353: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:358: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:363: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:373: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:376: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:379: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:382: node violates rule IncorrectIndentation
[Success matching fixit for IncorrectIndentation]
25 changes: 25 additions & 0 deletions test/chplcheck/IncorrectIndentation.good-fixit
Original file line number Diff line number Diff line change
Expand Up @@ -385,4 +385,29 @@ module IncorrectIndentation {
writeln("hi");
}
}


if 1 < 2 {
writeln("hi");
if 2 < 3 {
writeln("hi");
writeln("??");
}
}
// since else statements aren't reported correctly only the misaligned child statements should warn
if 1 < 2 {
writeln("hi");
writeln("??");
} else if 2 < 3 {
writeln("hi");
writeln("??");
} else {
writeln("hi");
writeln("??");
}
if 1 < 2 {
if 3 < 4 {

}
}
}
28 changes: 28 additions & 0 deletions tools/chapel-py/src/resolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,30 @@ static const resolution::ResolvedExpression* scopeResolveViaModule(Context* cont
return nullptr;
}

static const resolution::ResolvedExpression*
scopeResolveViaAggregate(Context* context,
const AstNode* aggNode,
const AstNode* node) {
if (auto agg = aggNode->toAggregateDecl()) {
auto& byId = resolution::scopeResolveAggregate(context, agg->id());
if (auto res = byId.byAstOrNull(node)) {
return res;
}
}
return nullptr;
}

static const resolution::ResolvedExpression*
scopeResolveViaEnum(Context* context, const AstNode* enumNode, const AstNode* node) {
if (auto enumDecl = enumNode->toEnum()) {
auto& byId = resolution::scopeResolveEnum(context, enumDecl->id());
if (auto res = byId.byAstOrNull(node)) {
return res;
}
}
return nullptr;
}

// For scope resolution, we handle AST nodes that don't necessarily get
// their own ResolvedExpression (specificlaly, uses/imports), so return an ID
// instead of a ResolvedExpression.
Expand All @@ -130,6 +154,10 @@ static const ID scopeResolveToIdForNode(Context* context, const AstNode* node) {
while (search) {
if (auto rr = scopeResolveViaFunction(context, search, node)) {
return rr->toId();
} else if (auto rr = scopeResolveViaAggregate(context, search, node)) {
return rr->toId();
} else if (auto rr = scopeResolveViaEnum(context, search, node)) {
return rr->toId();
} else if (auto rr = scopeResolveViaModule(context, search, node)) {
return rr->toId();
} else if(auto id = scopeResolveViaVisibilityStmt(context, search, node)) {
Expand Down
70 changes: 70 additions & 0 deletions tools/chpl-language-server/test/basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -320,3 +320,73 @@ async def test_list_references_standard(client: LanguageClient):

assert references is not None
assert len(references) > 10


@pytest.mark.asyncio
async def test_go_to_def_inherit(client: LanguageClient):
"""
Test cases for go-to-definition on inherit expression.
"""

file = """
module foo {
interface II { }
record R: II { }

class CC { }
class C: CC, II { }

module M {
class CC {}
}
class C2: M.CC { }
}
"""

async with source_file(client, file) as doc:
# Definitions link to themselves
await check_goto_decl_def(client, doc, pos((0, 7)), pos((0, 7)))
await check_goto_decl_def(client, doc, pos((1, 10)), pos((1, 10)))
await check_goto_decl_def(client, doc, pos((2, 7)), pos((2, 7)))
await check_goto_decl_def(client, doc, pos((4, 6)), pos((4, 6)))
await check_goto_decl_def(client, doc, pos((5, 6)), pos((5, 6)))
await check_goto_decl_def(client, doc, pos((7, 7)), pos((7, 7)))
await check_goto_decl_def(client, doc, pos((8, 8)), pos((8, 8)))
await check_goto_decl_def(client, doc, pos((10, 6)), pos((10, 6)))

# check the inherit expressions
await check_goto_decl_def(client, doc, pos((2, 10)), pos((1, 10)))
await check_goto_decl_def(client, doc, pos((5, 9)), pos((4, 6)))
await check_goto_decl_def(client, doc, pos((5, 13)), pos((1, 10)))
await check_goto_decl_def(client, doc, pos((10, 10)), pos((7, 7)))
await check_goto_decl_def(client, doc, pos((10, 12)), pos((8, 8)))


@pytest.mark.asyncio
async def test_go_to_enum(client: LanguageClient):
"""
Test cases for go-to-definition on enums.
"""

file = """
proc bar param do return 1;
enum A {
a = bar,
b = 1,
}
var e = A.a;
"""

async with source_file(client, file) as doc:
# Definitions link to themselves
await check_goto_decl_def(client, doc, pos((0, 5)), pos((0, 5)))
await check_goto_decl_def(client, doc, pos((1, 5)), pos((1, 5)))
await check_goto_decl_def(client, doc, pos((2, 2)), pos((2, 2)))
await check_goto_decl_def(client, doc, pos((3, 2)), pos((3, 2)))
await check_goto_decl_def(client, doc, pos((5, 4)), pos((5, 4)))

# check bar
await check_goto_decl_def(client, doc, pos((2, 6)), pos((0, 5)))
# check A.a
await check_goto_decl_def(client, doc, pos((5, 8)), pos((1, 5)))
await check_goto_decl_def(client, doc, pos((5, 10)), pos((2, 2)))
22 changes: 21 additions & 1 deletion tools/chplcheck/src/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,20 @@ def might_incorrectly_report_location(node: AstNode) -> bool:
elif isinstance(node, (Function, Use, Import)) and node.visibility() != "":
return True

# 'else if' statements do not have proper locations
#
# https://github.com/chapel-lang/chapel/issues/25256
elif 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


Expand Down Expand Up @@ -813,7 +827,13 @@ def unwrap_intermediate_block(node: AstNode) -> Optional[AstNode]:
# var x: int;
# }
elif parent_depth and depth == parent_depth:
yield AdvancedRuleResult(child, anchor=parent_for_indentation)
# conditionals do not support attributes
anchor = (
parent_for_indentation
if not isinstance(parent_for_indentation, Conditional)
else None
)
yield AdvancedRuleResult(child, anchor=anchor)

prev_depth = depth
prev = child
Expand Down