[SemaAnnotator] Fix bug causing custom attributes to be walked out of order

lib/IDE/IDERequests.cpp

@@ -815,12 +815,33 @@ struct RangeResolver::Implementation {
     analyzeDecl(D);
     auto &DCInfo = getCurrentDC();
 
-    auto NodeRange = Node.getSourceRange();
-
     // Widen the node's source range to include all attributes to get a range
     // match if a function with its attributes has been selected.
-    if (auto D = Node.dyn_cast<Decl *>())
-      NodeRange = D->getSourceRangeIncludingAttrs();
+    auto getSourceRangeIncludingAttrs = [](ASTNode N) -> SourceRange {
+      if (auto D = N.dyn_cast<Decl *>()) {
+        return D->getSourceRangeIncludingAttrs();
+      } else {
+        return N.getSourceRange();
+      }
+    };
+
+    auto NodeRange = getSourceRangeIncludingAttrs(Node);
+
+    // SemaAnnotator walks the AST in source order, but considers source order
+    // for declarations to be defined by their range *excluding* attributes.
+    // In RangeResolver, we attributes as belonging to their decl (see comment
+    // on getSourceRAngeIncludingAttrs above).
+    // Thus, for the purpose RangeResolver, we need to assume that SemaAnnotator
+    // hands us the nodes in arbitrary order.
+    //
+    // Remove any nodes that are contained by the newly added one.
+    auto removeIterator = std::remove_if(
+        ContainedASTNodes.begin(), ContainedASTNodes.end(),
+        [&](ASTNode ContainedNode) {
+          return SM.rangeContains(NodeRange,
+                                  getSourceRangeIncludingAttrs(ContainedNode));
+        });
+    ContainedASTNodes.erase(removeIterator, ContainedASTNodes.end());
 
     switch (getRangeMatchKind(NodeRange)) {
       case RangeMatchKind::NoneMatch: {
@@ -848,11 +869,16 @@ struct RangeResolver::Implementation {
 
     // If no parent is considered as a contained node; this node should be
     // a top-level contained node.
+    // If a node that contains this one is later discovered, this node will be
+    // removed from ContainedASTNodes again.
     if (std::none_of(ContainedASTNodes.begin(), ContainedASTNodes.end(),
-      [&](ASTNode N) { return SM.rangeContains(N.getSourceRange(),
-                                               Node.getSourceRange()); })) {
-        ContainedASTNodes.push_back(Node);
-      }
+                     [&](ASTNode ContainedNode) {
+                       return SM.rangeContains(
+                           getSourceRangeIncludingAttrs(ContainedNode),
+                           NodeRange);
+                     })) {
+      ContainedASTNodes.push_back(Node);
+    }
 
     if (DCInfo.isMultiStatement()) {
       postAnalysis(DCInfo.EndMatches.back());

test/IDE/range_info_basics.swift

@@ -149,6 +149,23 @@ func testDefer() {
   }
 }
 
+func testPropertyWrapper() {
+  @propertyWrapper
+  public struct SR12958 {
+    public var wrappedValue: String
+    init(_ wrappedValue: String) {
+      self.wrappedValue = wrappedValue
+    }
+  }
+
+  public struct MyStruct {
+    @SR12958("something")
+    public static var somevar: String
+    public static var someothervar: Int
+  }
+}
+
+
 // RUN: %target-swift-ide-test -range -pos=8:1 -end-pos 8:32 -source-filename %s | %FileCheck %s -check-prefix=CHECK1
 // RUN: %target-swift-ide-test -range -pos=9:1 -end-pos 9:26 -source-filename %s | %FileCheck %s -check-prefix=CHECK2
 // 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() {
 // RUN: %target-swift-ide-test -range -pos=133:1 -end-pos=135:65 -source-filename %s | %FileCheck %s -check-prefix=CHECK-NO-PATTERN
 // RUN: %target-swift-ide-test -range -pos=142:12 -end-pos=142:17 -source-filename %s | %FileCheck %s -check-prefix=CHECK-X-Y
 // RUN: %target-swift-ide-test -range -pos=147:1 -end-pos=150:1 -source-filename %s | %FileCheck %s -check-prefix=CHECK-INVALID
+// RUN: %target-swift-ide-test -range -pos=162:5 -end-pos=163:38 -source-filename %s | %FileCheck %s -check-prefix=CHECK-PROPERTY-WRAPPER
+
 
 // CHECK-NO-PATTERN: <Kind>MultiStatement</Kind>
 // CHECK-NO-PATTERN-NEXT: <Content>for key in parameters.keys.sorted(by: <) {
@@ -483,3 +502,11 @@ func testDefer() {
 
 // CHECK-X-Y: <Referenced>x</Referenced><Type>Int</Type>
 // CHECK-X-Y: <Referenced>y</Referenced><Type>Int</Type>
+
+// CHECK-PROPERTY-WRAPPER: <Kind>SingleDecl</Kind>
+// CHECK-PROPERTY-WRAPPER-NEXT: <Content>@SR12958("something")
+// CHECK-PROPERTY-WRAPPER-NEXT:     public static var somevar: String</Content>
+// CHECK-PROPERTY-WRAPPER-NEXT: <Context>swift_ide_test.(file).testPropertyWrapper().MyStruct</Context>
+// CHECK-PROPERTY-WRAPPER-NEXT: <Declared>somevar</Declared><OutscopeReference>false</OutscopeReference>
+// CHECK-PROPERTY-WRAPPER-NEXT: <ASTNodes>1</ASTNodes>
+// CHECK-PROPERTY-WRAPPER-NEXT: <end>

test/SourceKit/Refactoring/syntactic-rename.swift

@@ -104,6 +104,19 @@ _ = Memberwise1.init(x: 2)
 _ = Memberwise2.init(m: 2, n: Memberwise1(x: 34))
 #endif
 
+@propertyWrapper
+struct Bar<T> {
+  let wrappedValue: T
+  init(wrappedValue: T, other: Int) {
+    self.wrappedValue = wrappedValue
+  }
+}
+
+struct Foo {
+  @Bar(other: Foo.test)
+  static var test: Int = 10
+}
+
 // RUN: %empty-directory(%t.result)
 // RUN: %sourcekitd-test -req=syntactic-rename -rename-spec %S/syntactic-rename/x.in.json %s >> %t.result/x.expected
 // RUN: %diff -u %S/syntactic-rename/x.expected %t.result/x.expected

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

@@ -0,0 +1,4 @@
+source.edit.kind.active:
+  116:19-116:34 "renamedProperty"
+source.edit.kind.active:
+  117:14-117:29 "renamedProperty"

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

@@ -0,0 +1,20 @@
+[
+  {
+    "key.name": "wrappedProperty",
+    "key.newname": "renamedProperty",
+    "key.is_function_like": 0,
+    "key.is_non_protocol_type": 0,
+    "key.locations": [
+      {
+        "key.line": 117,
+        "key.column": 14,
+        "key.nametype": source.syntacticrename.definition
+      },
+      {
+        "key.line": 116,
+        "key.column": 19,
+        "key.nametype": source.syntacticrename.reference
+      }
+    ]
+  }
+]