Skip to content

Commit 16baf61

Browse files
ahoppenahoppen
committed
[RangeInfo] Acknowledge that SemaAnnotator walks the tree in source order *excluding* attributes
`SemaAnnotator` walks the tree in source order with respect to the source ranges *excluding* attributes, but `RangeResolver` considers attributes part of their declaration. Thus they disagree on what “walking in source order means”. `SemaAnnotator` will visit the attributes *before* the decl they are on, while `RangeResolver` as currently implemented expects them *as part of* the decl they are on. Thus, for the purpose of `RangeResolver`, we need to assume that nodes are visited in arbitrary order and we might encounter enclosing nodes after their children. Thus, when we find a new node, remove all nodes that it encloses from `ContainedASTNodes`. Fixes rdar://64140713 [SR-12958]
1 parent 74b5eb7 commit 16baf61

File tree

5 files changed

+98
-8
lines changed

5 files changed

+98
-8
lines changed

‎lib/IDE/IDERequests.cpp

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -815,12 +815,33 @@ struct RangeResolver::Implementation {
815815
analyzeDecl(D);
816816
auto &DCInfo = getCurrentDC();
817817

818-
auto NodeRange = Node.getSourceRange();
819-
820818
// Widen the node's source range to include all attributes to get a range
821819
// match if a function with its attributes has been selected.
822-
if (auto D = Node.dyn_cast<Decl *>())
823-
NodeRange = D->getSourceRangeIncludingAttrs();
820+
auto getSourceRangeIncludingAttrs = [](ASTNode N) -> SourceRange {
821+
if (auto D = N.dyn_cast<Decl *>()) {
822+
return D->getSourceRangeIncludingAttrs();
823+
} else {
824+
return N.getSourceRange();
825+
}
826+
};
827+
828+
auto NodeRange = getSourceRangeIncludingAttrs(Node);
829+
830+
// SemaAnnotator walks the AST in source order, but considers source order
831+
// for declarations to be defined by their range *excluding* attributes.
832+
// In RangeResolver, we attributes as belonging to their decl (see comment
833+
// on getSourceRAngeIncludingAttrs above).
834+
// Thus, for the purpose RangeResolver, we need to assume that SemaAnnotator
835+
// hands us the nodes in arbitrary order.
836+
//
837+
// Remove any nodes that are contained by the newly added one.
838+
auto removeIterator = std::remove_if(
839+
ContainedASTNodes.begin(), ContainedASTNodes.end(),
840+
[&](ASTNode ContainedNode) {
841+
return SM.rangeContains(NodeRange,
842+
getSourceRangeIncludingAttrs(ContainedNode));
843+
});
844+
ContainedASTNodes.erase(removeIterator, ContainedASTNodes.end());
824845

825846
switch (getRangeMatchKind(NodeRange)) {
826847
case RangeMatchKind::NoneMatch: {
@@ -848,11 +869,16 @@ struct RangeResolver::Implementation {
848869

849870
// If no parent is considered as a contained node; this node should be
850871
// a top-level contained node.
872+
// If a node that contains this one is later discovered, this node will be
873+
// removed from ContainedASTNodes again.
851874
if (std::none_of(ContainedASTNodes.begin(), ContainedASTNodes.end(),
852-
[&](ASTNode N) { return SM.rangeContains(N.getSourceRange(),
853-
Node.getSourceRange()); })) {
854-
ContainedASTNodes.push_back(Node);
855-
}
875+
[&](ASTNode ContainedNode) {
876+
return SM.rangeContains(
877+
getSourceRangeIncludingAttrs(ContainedNode),
878+
NodeRange);
879+
})) {
880+
ContainedASTNodes.push_back(Node);
881+
}
856882

857883
if (DCInfo.isMultiStatement()) {
858884
postAnalysis(DCInfo.EndMatches.back());

‎test/IDE/range_info_basics.swift

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,23 @@ func testDefer() {
149149
}
150150
}
151151

152+
func testPropertyWrapper() {
153+
@propertyWrapper
154+
public struct SR12958 {
155+
public var wrappedValue: String
156+
init(_ wrappedValue: String) {
157+
self.wrappedValue = wrappedValue
158+
}
159+
}
160+
161+
public struct MyStruct {
162+
@SR12958("something")
163+
public static var somevar: String
164+
public static var someothervar: Int
165+
}
166+
}
167+
168+
152169
// RUN: %target-swift-ide-test -range -pos=8:1 -end-pos 8:32 -source-filename %s | %FileCheck %s -check-prefix=CHECK1
153170
// RUN: %target-swift-ide-test -range -pos=9:1 -end-pos 9:26 -source-filename %s | %FileCheck %s -check-prefix=CHECK2
154171
// RUN: %target-swift-ide-test -range -pos=10:1 -end-pos 10:27 -source-filename %s | %FileCheck %s -check-prefix=CHECK3
@@ -187,6 +204,8 @@ func testDefer() {
187204
// RUN: %target-swift-ide-test -range -pos=133:1 -end-pos=135:65 -source-filename %s | %FileCheck %s -check-prefix=CHECK-NO-PATTERN
188205
// RUN: %target-swift-ide-test -range -pos=142:12 -end-pos=142:17 -source-filename %s | %FileCheck %s -check-prefix=CHECK-X-Y
189206
// RUN: %target-swift-ide-test -range -pos=147:1 -end-pos=150:1 -source-filename %s | %FileCheck %s -check-prefix=CHECK-INVALID
207+
// RUN: %target-swift-ide-test -range -pos=162:5 -end-pos=163:38 -source-filename %s | %FileCheck %s -check-prefix=CHECK-PROPERTY-WRAPPER
208+
190209

191210
// CHECK-NO-PATTERN: <Kind>MultiStatement</Kind>
192211
// CHECK-NO-PATTERN-NEXT: <Content>for key in parameters.keys.sorted(by: <) {
@@ -483,3 +502,11 @@ func testDefer() {
483502

484503
// CHECK-X-Y: <Referenced>x</Referenced><Type>Int</Type>
485504
// CHECK-X-Y: <Referenced>y</Referenced><Type>Int</Type>
505+
506+
// CHECK-PROPERTY-WRAPPER: <Kind>SingleDecl</Kind>
507+
// CHECK-PROPERTY-WRAPPER-NEXT: <Content>@SR12958("something")
508+
// CHECK-PROPERTY-WRAPPER-NEXT: public static var somevar: String</Content>
509+
// CHECK-PROPERTY-WRAPPER-NEXT: <Context>swift_ide_test.(file).testPropertyWrapper().MyStruct</Context>
510+
// CHECK-PROPERTY-WRAPPER-NEXT: <Declared>somevar</Declared><OutscopeReference>false</OutscopeReference>
511+
// CHECK-PROPERTY-WRAPPER-NEXT: <ASTNodes>1</ASTNodes>
512+
// CHECK-PROPERTY-WRAPPER-NEXT: <end>

‎test/SourceKit/Refactoring/syntactic-rename.swift

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,19 @@ _ = Memberwise1.init(x: 2)
104104
_ = Memberwise2.init(m: 2, n: Memberwise1(x: 34))
105105
#endif
106106

107+
@propertyWrapper
108+
struct Bar<T> {
109+
let wrappedValue: T
110+
init(wrappedValue: T, other: Int) {
111+
self.wrappedValue = wrappedValue
112+
}
113+
}
114+
115+
struct Foo {
116+
@Bar(other: Foo.test)
117+
static var test: Int = 10
118+
}
119+
107120
// RUN: %empty-directory(%t.result)
108121
// RUN: %sourcekitd-test -req=syntactic-rename -rename-spec %S/syntactic-rename/x.in.json %s >> %t.result/x.expected
109122
// RUN: %diff -u %S/syntactic-rename/x.expected %t.result/x.expected

‎test/SourceKit/Refactoring/syntactic-rename/wrappedProperty.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
source.edit.kind.active:
2+
116:19-116:34 "renamedProperty"
3+
source.edit.kind.active:
4+
117:14-117:29 "renamedProperty"

‎test/SourceKit/Refactoring/syntactic-rename/wrappedProperty.in.json

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
[
2+
{
3+
"key.name": "wrappedProperty",
4+
"key.newname": "renamedProperty",
5+
"key.is_function_like": 0,
6+
"key.is_non_protocol_type": 0,
7+
"key.locations": [
8+
{
9+
"key.line": 117,
10+
"key.column": 14,
11+
"key.nametype": source.syntacticrename.definition
12+
},
13+
{
14+
"key.line": 116,
15+
"key.column": 19,
16+
"key.nametype": source.syntacticrename.reference
17+
}
18+
]
19+
}
20+
]

0 commit comments

Comments
 (0)