[lldb] Rewrite Swift REPL completion to use CompletionRequest

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) {