[CodeComplete] Don't replace the rest of line in #include completion.

[hokein]

Summary

The previous behavior was aggressive,

#include "abc/f^/abc.h"
              foo/  -> candidate

"f/abc.h" is replaced with "foo/", this patch will preserve the "abc.h".

[sammccall]

Does this regress the case where you complete a file rather than a directory?
e.g. #include "foo/^one.h"
If you select "two.h", then we definitely want to replace "one.h", right?

The root here is that we have to decide what to replace independently of the completion item.
One compromise might be to replace up to the quote, but if you're completing "foo/^one.h" and bar is a directory, check if foo/bar/one.h exists and offer it as a completion. Seems complicated though.

Or we can take the tradeoff here if it's better but it'd be good to understand why.

[hokein]

In D76770#1941603, @sammccall wrote:

Does this regress the case where you complete a file rather than a directory?
e.g. #include "foo/^one.h"
If you select "two.h", then we definitely want to replace "one.h", right?

yes, it has a regression, but we don't change the behavior for the above case. A regression case is like

#include "foo/^dir1/one.h"
// with two candidates: dir2 and foo.h

if we choose 2), after completion, "foo/foo.h/one.h" (now) vs foo/foo.h (before)

The root here is that we have to decide what to replace independently of the completion item.
One compromise might be to replace up to the quote, but if you're completing "foo/^one.h" and bar is a directory, check if foo/bar/one.h exists and offer it as a completion. Seems complicated though.

agree, it doesn't seem to be worth the effort.

Or we can take the tradeoff here if it's better but it'd be good to understand why.

it was originally reported by one internal user.

I think the new behavior (with this patch)

• is more aligned with mental model of code completion (most people think code completion just inserts the code text at the cursor position )
• keeps the consistent behavior with other code completions, e.g. foo.^member() if you select the member in code completion, another member text will be inserted

[sammccall]

In D76770#1943077, @hokein wrote:

yes, it has a regression, but we don't change the behavior for the above case. A regression case is like

Oops, right :-)

Or we can take the tradeoff here if it's better but it'd be good to understand why.

it was originally reported by one internal user.

I think the new behavior (with this patch)

• is more aligned with mental model of code completion (most people think code completion just inserts the code text at the cursor position )
• keeps the consistent behavior with other code completions, e.g. foo.^member() if you select the member in code completion, another member text will be inserted

OK, let's try it and see if people complain.
(There's also an argument for only replacing the text on the left of the cursor, that's more consistent with completion in general though I think less helpful too)

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

TEST(CompletionTest, RenderWithFixItNonMerged) {
   EXPECT_TRUE(R.textEdit);
   EXPECT_EQ(R.textEdit->newText, "Foo::x");
   EXPECT_THAT(R.additionalTextEdits, UnorderedElementsAre(FixIt));
 }
 
 TEST(CompletionTest, CompletionTokenRange) {
   MockFSProvider FS;
   MockCompilationDatabase CDB;
+  FS.Files["foo/abc/foo.h"] = "";
   ClangdServer Server(CDB, FS, ClangdServer::optsForTest());
 
   constexpr const char *TestCodes[] = {
       R"cpp(
         class Auxilary {
          public:
           void AuxFunction();
         };
         void f() {
           Auxilary x;
           x.[[Aux]]^;
         }
       )cpp",
       R"cpp(
         class Auxilary {
          public:
           void AuxFunction();
         };
         void f() {
           Auxilary x;
           x.[[]]^;
         }
-      )cpp"};
+      )cpp",
+      R"cpp(
+        #include "foo/[[a^/]]foo.h"
+      )cpp",
+      R"cpp(
+        #include "foo/abc/[[fo^o.h"]]
+      )cpp",
+      };
   for (const auto &Text : TestCodes) {
     Annotations TestCode(Text);
     auto Results = completions(Server, TestCode.code(), TestCode.point());
 
     if (Results.Completions.size() != 1) {
       ADD_FAILURE() << "Results.Completions.size() != 1";
       continue;
     }

clang/lib/Lex/Lexer.cpp

 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Lex/Token.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LLVM.h"
 #include "clang/Basic/TokenKinds.h"
 #include "llvm/ADT/None.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/ConvertUTF.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/NativeFormatting.h"
bool Lexer::LexAngledStringLiteral(Token &Result, const char *CurPtr) {
   return true;
 }
 
 void Lexer::codeCompleteIncludedFile(const char *PathStart,
                                      const char *CompletionPoint,
                                      bool IsAngled) {
   // Completion only applies to the filename, after the last slash.
   StringRef PartialPath(PathStart, CompletionPoint - PathStart);
-  auto Slash = PartialPath.find_last_of(LangOpts.MSVCCompat ? "/\\" : "/");
+  llvm::StringRef SlashChars = LangOpts.MSVCCompat ? "/\\" : "/";
+  auto Slash = PartialPath.find_last_of(SlashChars);
   StringRef Dir =
       (Slash == StringRef::npos) ? "" : PartialPath.take_front(Slash);
   const char *StartOfFilename =
       (Slash == StringRef::npos) ? PathStart : PathStart + Slash + 1;
   // Code completion filter range is the filename only, up to completion point.
   PP->setCodeCompletionIdentifierInfo(&PP->getIdentifierTable().get(
       StringRef(StartOfFilename, CompletionPoint - StartOfFilename)));
-  // We should replace the characters up to the closing quote, if any.
+  // We should replace the characters up to the closing quote or closest slash,
+  // if any.
   while (CompletionPoint < BufferEnd) {
     char Next = *(CompletionPoint + 1);
     if (Next == 0 || Next == '\r' || Next == '\n')
       break;
     ++CompletionPoint;
     if (Next == (IsAngled ? '>' : '"'))
       break;
+    if (llvm::is_contained(SlashChars, Next))
+      break;
   }
+
   PP->setCodeCompletionTokenRange(
       FileLoc.getLocWithOffset(StartOfFilename - BufferStart),
       FileLoc.getLocWithOffset(CompletionPoint - BufferStart));
   PP->CodeCompleteIncludedFile(Dir, IsAngled);
 }
 
 /// LexCharConstant - Lex the remainder of a character constant, after having
 /// lexed either ' or L' or u8' or u' or U'.