Merge pull request #273 from hamishknight/exclude-completions

Exclude expected completions in a couple of places

Author: shahmishal

SourceKitStressTester/Sources/StressTester/ActionGenerators.swift

@@ -503,6 +503,15 @@ private struct ActionToken {
         return nil
       }
     }
+
+    // Avoid matching for underscored attributes since we hide completions for
+    // those.
+    if let attr = parent.ancestorOrSelf(mapping: { $0.as(AttributeSyntax.self) }),
+       let ident = attr.attributeName.as(IdentifierTypeSyntax.self),
+       ident.name.text.hasPrefix("_") {
+      return nil
+    }
+
     if let parent = parent.as(DeclReferenceExprSyntax.self), parent.baseName == token {
       if let refArgs = parent.argumentNames {
         let name = SwiftName(base: token.text, labels: refArgs.arguments.map{ $0.name.text })

SourceKitStressTester/Sources/StressTester/SourceKitDocument.swift

@@ -718,8 +718,22 @@ public struct CompletionMatcher {
   private func matchesCall(paramLabels: [String]) -> Bool {
     var remainingArgLabels = expected.name.argLabels[...]
 
+    func skippingIgnorableArgLabels(skipEmpty: Bool = false) -> ArraySlice<String> {
+      var remainingArgLabels = remainingArgLabels
+      if skipEmpty || remainingArgLabels.count < expected.name.argLabels.count {
+        // If previous param was matched, allow consuming unlabeled args to
+        // handle variadic cases.
+        remainingArgLabels = remainingArgLabels.drop { $0.isEmpty }
+      }
+      // Ignore `file` and `line` since we don't include `#file` and `#line`
+      // default args for completion.
+      // FIXME: We ought to have this be configurable and enable it for the stress
+      // tester.
+      return remainingArgLabels.drop { $0 == "file" || $0 == "line" }
+    }
+
     guard !paramLabels.isEmpty else {
-      return remainingArgLabels.allSatisfy { $0.isEmpty }
+      return skippingIgnorableArgLabels(skipEmpty: true).isEmpty
     }
     for nextParamLabel in paramLabels {
       if nextParamLabel.isEmpty {
@@ -732,12 +746,9 @@ public struct CompletionMatcher {
           continue
         }
       } else {
-        // Has param label
-        if remainingArgLabels.count < expected.name.argLabels.count {
-          // A previous param was matched, so assume it was variadic and consume
-          // any leading unlabelled args so the next arg is labelled
-          remainingArgLabels = remainingArgLabels.drop{ $0.isEmpty }
-        }
+        // Has param label, skip any ignorable labels before matching.
+        remainingArgLabels = skippingIgnorableArgLabels()
+
         guard let nextArgLabel = remainingArgLabels.first else {
           // Assume any unprocessed parameters are defaulted
           return true
@@ -750,9 +761,6 @@ public struct CompletionMatcher {
         // Else assume this param was defaulted and skip it.
       }
     }
-    // If at least one arglabel was matched, allow for it being variadic
-    let hadMatch = remainingArgLabels.count < expected.name.argLabels.count
-    return  remainingArgLabels.isEmpty || hadMatch &&
-      remainingArgLabels.allSatisfy { $0.isEmpty }
+    return skippingIgnorableArgLabels().isEmpty
   }
 }

SourceKitStressTester/Tests/StressTesterToolTests/ActionGeneratorTests.swift

@@ -97,7 +97,11 @@ class ActionGeneratorTests: XCTestCase {
 
       // FIXME: completion doesn't suggest anonymous closure params (e.g. $0)
       (#line, "$0.first", [/*"$0",*/ "first"]),
-      (#line, "[1,2,4].filter{ $0 == 4 }", ["call:filter"/*, "$0"*/])
+      (#line, "[1,2,4].filter{ $0 == 4 }", ["call:filter"/*, "$0"*/]),
+
+      // We don't include completions for underscored attributes.
+      (#line, "@_underscored(a: Foo)", []),
+      (#line, "@_spi(Foo)", []),
     ]
 
     for test in cases {
@@ -153,7 +157,17 @@ class ActionGeneratorTests: XCTestCase {
       (match: .pattern, of: "first(_:z:)", against: "first(x:y:)", ignoreArgs: false, result: false),
       (match: .pattern, of: "first(_:)", against: "first", ignoreArgs: false, result: false),
       (match: .pattern, of: "first(_:)", against: "first()", ignoreArgs: false, result: true),
-      (match: .pattern, of: "first(_:_:)", against: "first()", ignoreArgs: false, result: true)
+      (match: .pattern, of: "first(_:_:)", against: "first()", ignoreArgs: false, result: true),
+
+      // We don't do matching for `file` and `line`.
+      (match: .call, of: "foo(x:file:line:)", against: "foo(x:)", ignoreArgs: false, result: true),
+      (match: .call, of: "foo(x:file:line:)", against: "foo(x:file:line:)", ignoreArgs: false, result: true),
+      (match: .call, of: "foo(x:file:line:y:)", against: "foo(x:y:)", ignoreArgs: false, result: true),
+      (match: .call, of: "foo(file:line:y:)", against: "foo(file:line:x:y:z:)", ignoreArgs: false, result: true),
+      (match: .call, of: "foo(x:file:line:y:)", against: "foo(x:y:z:)", ignoreArgs: false, result: true),
+      (match: .call, of: "foo(file:line:y:)", against: "foo(x:y:)", ignoreArgs: false, result: true),
+      (match: .call, of: "foo(file:line:)", against: "foo(_:)", ignoreArgs: false, result: true),
+      (match: .call, of: "foo(file:line:)", against: "foo()", ignoreArgs: false, result: true),
     ]
 
     for test in cases {