[lldb] Rewrite Swift REPL completion to use CompletionRequest

This removes all the workaround and problematic code that was needed
for the old interface. With the CompletionRequest we no longer need
to calculate the common prefix ourselves and we don't need to calculate
the magic return value. On the other hand we can no longer provide
'appendix' completions (E.g. completing "Suff" with "ix", instead
we now need to provide the completion "Suffix"), so in one case
we now need to prefix the completion with the existing prefix from
the CompletionRequest (as Swift only returns the string to append
and not the whole token).

Also rewrites the TestSwiftREPLCompletion.py to test all the
problematic cases and some more.

Fixes rdar://problem/58854651

lldb/packages/Python/lldbsuite/test/lang/swift/completion/TestSwiftREPLCompletion.py

@@ -1,22 +1,42 @@
-from __future__ import print_function
-import pexpect
-import os
+
 import lldb
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
-from lldbsuite.test import lldbutil
+from lldbsuite.test.lldbpexpect import PExpectTest
 
+class SwiftCompletionTest(PExpectTest):
 
-class TestSwiftREPLCompletion(TestBase):
     mydir = TestBase.compute_mydir(__file__)
 
+    # PExpect uses many timeouts internally and doesn't play well
+    # under ASAN on a loaded machine..
+    @skipIfAsan
     @skipUnlessDarwin
-    def test_repl_completion(self):
-        prompt = "Welcome to"
-        child = pexpect.spawn('%s --repl' % (lldbtest_config.lldbExec))
-        # Assign to make sure the sessions are killed during teardown
-        self.child = child
-        # Send a <TAB> and make sure we don't crash.
-        child.sendline("import Foundatio\t")
-        child.sendline("print(NSString(\"patatino\"))")
-        child.expect("patatino")
+    def test_basic_completion(self):
+
+        self.launch(extra_args=["--repl"], executable=None, dimensions=(100,500))
+
+        # Wait on the first prompt
+        self.child.expect_exact("1>")
+        # Press tab a few times which should do nothing.
+        # Note that we don't get any indentation whitespace as
+        # pexpect is not recognized as a interactive terminal by pexpect it seems.
+        self.child.send("\t\t\t")
+
+        # Try completing something that only has one result "Hasabl" -> "Hashable".
+        self.child.send("Hashabl\t")
+        self.child.expect_exact("Hashable")
+        self.child.sendline("")
+
+        # Try completing something that has multiple completions.
+        self.child.send("Hash\t")
+        self.child.expect_exact("Available completions:")
+        self.child.expect_exact("Hashable")
+        self.child.expect_exact("Hasher")
+        self.child.sendline("")
+
+    def setUpCommands(self):
+        return [] # REPL doesn't take any setup commands.
+
+    def expect_prompt(self):
+        pass # No constant prompt on the REPL.

lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.cpp

@@ -533,8 +533,8 @@ bool SwiftREPL::PrintOneVariable(Debugger &debugger, StreamFileSP &output_sp,
   return handled;
 }
 
-int SwiftREPL::CompleteCode(const std::string &current_code,
-                            lldb_private::StringList &matches) {
+void SwiftREPL::CompleteCode(const std::string &current_code,
+                             CompletionRequest &request) {
   //----------------------------------------------------------------------g
   // If we use the target's SwiftASTContext for completion, it reaaallly
   // slows down subsequent expressions. The compiler team doesn't have time
@@ -546,7 +546,7 @@ int SwiftREPL::CompleteCode(const std::string &current_code,
     auto type_system_or_err = m_target.GetScratchTypeSystemForLanguage(eLanguageTypeSwift);
     if (!type_system_or_err) {
       llvm::consumeError(type_system_or_err.takeError());
-      return 0;
+      return;
     }
 
     auto *target_swift_ast =
@@ -579,54 +579,56 @@ int SwiftREPL::CompleteCode(const std::string &current_code,
       swift::SourceFile &repl_source_file =
           repl_module->getMainSourceFile(swift::SourceFileKind::REPL);
 
+      // Swift likes to give us strings to append to the current token but
+      // the CompletionRequest requires a replacement for the full current
+      // token. Fix this by getting the current token here and we attach
+      // the suffix we get from Swift.
+      std::string prefix = request.GetCursorArgumentPrefix();
       llvm::StringRef current_code_ref(current_code);
       completions.populate(repl_source_file, current_code_ref);
+
+      // The root is the unique completion we need to use, so let's add it
+      // to the completion list. As the completion is unique we can stop here.
       llvm::StringRef root = completions.getRoot();
       if (!root.empty()) {
-        matches.AppendString(root.data(), root.size());
-        return 1;
+        request.AddCompletion(prefix + root.str(), "", CompletionMode::Partial);
+        return;
       }
+
       // Otherwise, advance through the completion state machine.
       const swift::CompletionState completion_state = completions.getState();
       switch (completion_state) {
       case swift::CompletionState::CompletedRoot: {
-        // We completed the root. Next step is to display the completion list.
-        matches.AppendString(""); // Empty string to indicate no completion,
-        // just display other strings that come after it
+        // Display the completion list.
         llvm::ArrayRef<llvm::StringRef> llvm_matches =
             completions.getCompletionList();
         for (const auto &llvm_match : llvm_matches) {
+          // The completions here aren't really useful for actually completing
+          // the token but are more descriptive hints for the user
+          // (e.g. "isMultiple(of: Int) -> Bool"). They aren't useful for
+          // actually completing anything so let's use the current token as
+          // a placeholder that is always valid.
           if (!llvm_match.empty())
-            matches.AppendString(llvm_match.data(), llvm_match.size());
+            request.AddCompletion(prefix, llvm_match);
         }
-        // Don't include the empty string we appended above or we will display
-        // one
-        // too many we need to return the magical value of one less than our
-        // actual matches.
-        // TODO: modify all IOHandlerDelegate::IOHandlerComplete() to use a
-        // CompletionMatches
-        // class that wraps up the "StringList matches;" along with other smarts
-        // so we don't
-        // have to return magic values and incorrect sizes.
-        return matches.GetSize() - 1;
       } break;
 
       case swift::CompletionState::DisplayedCompletionList: {
         // Complete the next completion stem in the cycle.
-        llvm::StringRef stem = completions.getPreviousStem().InsertableString;
-        matches.AppendString(stem.data(), stem.size());
+        request.AddCompletion(prefix + completions.getPreviousStem().InsertableString.str());
       } break;
 
       case swift::CompletionState::Empty:
-      case swift::CompletionState::Unique:
-        // We already provided a definitive completion--nothing else to do.
-        break;
+      case swift::CompletionState::Unique: {
+        llvm::StringRef root = completions.getRoot();
+
+        if (!root.empty())
+          request.AddCompletion(prefix + root.str());
+      } break;
 
       case swift::CompletionState::Invalid:
         llvm_unreachable("got an invalid completion set?!");
       }
     }
   }
-
-  return matches.GetSize();
 }

lldb/source/Plugins/ExpressionParser/Swift/SwiftREPL.h

@@ -68,8 +68,8 @@ class SwiftREPL : public REPL {
                         lldb::ValueObjectSP &valobj_sp,
                         ExpressionVariable *var = nullptr) override;
 
-  int CompleteCode(const std::string &current_code,
-                   StringList &matches) override;
+  void CompleteCode(const std::string &current_code,
+                    CompletionRequest &request) override;
 
 public:
   static bool classof(const REPL *repl) {