Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.

[kadircet]

Summary

Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.

[ilya-biryukov]

Thanks for the change! Overall LG, mostly NITs about the tests.

clangd/CodeComplete.cpp

957

NIT: this change is unrelated, maybe remove from the diff?

We can submit it separately without review, since it's a noop formatting cleanup

1099–1100

Same here: maybe remove from this change and submit separately?
Again, no review needed, this is clearly a no-op change

1289

IIUC, it does not play nicely with the insertText in VSCode, right? (I.e. completing this.^ will produce this-push_back)
Let's add a FIXME comment explaining this case and indicate how we're going to fix this (using textEdit instead of the insertText, right?)

unittests/clangd/CodeCompleteTests.cpp

497

NIT: also submit as a separate change?

1374

Maybe remove the body of opeartor->() and the private field?
Will make the test a little smaller.

1378

Maybe remove namespace and put everything into global ns?

1381

Maybe tests without the prefix, so that both AuxFunction and MemberFunction are available and assert that AuxFunction does not have a fix-it?

1389

This can be made more readable with helpers from unittests/clangd/Annotations.h.

Annotations Source(R"cpp(
/// ....
void f() {
  ClassWithPtr x;
  x[[->]]MemberFunction^ 
})cpp");

Source.point(); // <-- completion point
Source.range(); // <-- range for the fix-it

Our completions helper already has parsing of annotations internally, though. But we can write a new overload for the helper that accepts completion point and the parsed text directly.

[kadircet]

Fixed discussions

[ilya-biryukov]

clangd/CodeComplete.cpp

286

IIRC LangOptions are actually important when running lexer (that is used internally to measure the length of the tokens).
Use ASTCtx.getLangOptions()?

clangd/SourceCode.cpp

11

NIT: no need for this include in .cpp file, since the header already has that.

clangd/SourceCode.h

16

NIT: #include "clang/Basic/Diagnostic.h" should be enough here

unittests/clangd/CodeCompleteTests.cpp

99

Maybe accept a stringref to source code and completion point directly?
A potential use-case: Annotaions instance might have multiple named rangers (e.g. if you have multiple completion points in same code and want to test one after another). In that case, point() will fail with an assert if we pass Annotations directly

[kadircet]

Added functionality to suggest FixIts for conversion of '->' to '.' and vice versa.

• Fixes for comments.
• Take unnecessary patches back
• Second pass for comments.
• Add downranking for scores of fixit elements.

[ilya-biryukov]

Thanks! The only major comment I have is about the FixItsScore, others are mostly NITs.

clangd/Quality.cpp

298

NIT: remove braces around single-statement loop body

clangd/Quality.h

80

The comment is a bit misleading, since it's actually the length of the inserted code.
I'd suggest keeping the comment and a name of this variable a bit more vague on the definition of this field, e.g. something like

int FixItScore; // Measure of the total amount of edit needed by the fix-its. Higher is worse, i.e. more code needs to change

And overall, it looks like the penalty might be too harsh for some potential fix-its. E.g. imagine a fix-it that adds a using ns::some_class directive somewhere to the file. The size of the inserted text will be proprotional to the size of the fully-qualfiied-name of the class, but it does not seem like the fix-its for items with larger names should be downranked more than others, after all it's the same type of fix-it.

// Hypothetical example, we don't have fix-its like that yet.
namespace ns {
  class SomeVeryLongName {};
  class ShortName {};
}


^ // <-- complete here, imagine the completion items for both classes are available and 
  // add `using ns::<ClassName>;`  to the global namespace.

In that case, we will penalize SomeVeryLongName more than ShortName, because it has a longer fix-it, but it does not look like a useful distinction.
I suggest we start with something simple, like having the same predefined penalty for all fix-its or penalize based on the number of fix-its in the list. We can generalize later when we see more fix-its and have a better understanding on penalties we want from them

unittests/clangd/CodeCompleteTests.cpp

130

NIT: use const Annotations&?

137

The number of completions overloads seems to be getting out of hand.
Any ideas on how we can reduce them?

Generally, I think we want the most general version:

CodeCompleteResult completionsImpl(ClangdServer&, Text, Point, Index, Opts)

And two conveniece wrappers that create ClangdServer for us:

// Parses Annotations(Text) and creates a ClangdServer instance.
CodeCompleteResult completions(Text, Index = ..., Opts = ...);
// Only creates a ClangdServer instance.
CodeCompleteResult completions(Text, Point, Index=..., Opts =...);

Can we make the rest of the code to use this API? (e.g. if function accepts a ClangdServer, the callers have to do quite a bit of setup anyway and I don't think adding the annotations parsing to get the completion point will hurt the readability there).

1390

NIT: remove braces around single-statement ifs, LLVM code generally tends to avoid adding braces around single-statement bodies of loops, conditionals, etc

1432

We tend to unit-test the scores separately from general completion tests.
Could we replace this test with a unit-test in QualityTests.cpp?

[kadircet]

• Resolve discussions.

[ilya-biryukov]

LGTM, with a few NITs.
Thanks for the change!

clangd/Quality.h

81

NIT: put the comment at the previous line to keep false at the same line.
NIT2: use three slashes, to make it a doxygen comment (previous one should also be three slashes).

unittests/clangd/CodeCompleteTests.cpp

99

It seems this comment got moved accidentally, move it back?

unittests/clangd/QualityTests.cpp

360

NIT: took me some time to figure out the difference between this and the next one.
Maybe add the explicit type name to make more implicit that we're creating a fix-it here, i.e. {FixItHint{}}?

[kadircet]

• Resolve some more discussions.

clangd/CodeComplete.h

struct CodeCompleteOptions {
   bool ShowOrigins = false;
 
   // Populated internally by clangd, do not set.
   /// If `Index` is set, it is used to augment the code completion
   /// results.
   /// FIXME(ioeric): we might want a better way to pass the index around inside
   /// clangd.
   const SymbolIndex *Index = nullptr;
+
+  /// Include completions that require small corrections, e.g. change '.' to
+  /// '->' on member access etc.
+  bool IncludeFixIts = false;
 };
 
 // Semi-structured representation of a code-complete suggestion for our C++ API.
 // We don't use the LSP structures here (unlike most features) as we want
 // to expose more data to allow for more precise testing and evaluation.
 struct CodeCompletion {
   // The unqualified name of the symbol or other completion item.
   std::string Name;
struct CodeCompletion {
   SymbolOrigin Origin = SymbolOrigin::Unknown;
   // The header through which this symbol could be included.
   // Quoted string as expected by an #include directive, e.g. "<memory>".
   // Empty for non-symbol completions, or when not known.
   std::string Header;
   // Present if Header is set and should be inserted to use this item.
   llvm::Optional<TextEdit> HeaderInsertion;
 
+  /// Holds information about small corrections that needs to be done. Like
+  /// converting '->' to '.' on member access.
+  std::vector<TextEdit> FixIts;
+
   // Scores are used to rank completion items.
   struct Scores {
     // The score that items are ranked by.
     float Total = 0.f;
 
     // The finalScore with the fuzzy name match score excluded.
     // When filtering client-side, editors should calculate the new fuzzy score,
     // whose scale is 0-1 (with 1 = prefix match, special case 2 = exact match),

clangd/CodeComplete.cpp

 // it's purely AST-based, and there are few candidates.
 //
 //===---------------------------------------------------------------------===//
 
 #include "CodeComplete.h"
 #include "AST.h"
 #include "CodeCompletionStrings.h"
 #include "Compiler.h"
+#include "Diagnostics.h"
 #include "FileDistance.h"
 #include "FuzzyMatch.h"
 #include "Headers.h"
 #include "Logger.h"
 #include "Quality.h"
 #include "SourceCode.h"
 #include "Trace.h"
 #include "URI.h"
if (C.SemaResult) {
             (C.SemaResult->Kind == CodeCompletionResult::RK_Pattern))
           if (const auto *D = C.SemaResult->getDeclaration())
             if (const auto *ND = llvm::dyn_cast<NamedDecl>(D))
               Completion.Scope =
                   splitQualifiedName(printQualifiedName(*ND)).first;
       }
       Completion.Kind =
           toCompletionItemKind(C.SemaResult->Kind, C.SemaResult->Declaration);
+      for (const auto &FixIt : C.SemaResult->FixIts) {
+        Completion.FixIts.push_back(
+            toTextEdit(FixIt, ASTCtx.getSourceManager(), ASTCtx.getLangOpts()));
+      }
     }
     if (C.IndexResult) {
       Completion.Origin |= C.IndexResult->Origin;
       if (Completion.Scope.empty())
         Completion.Scope = C.IndexResult->Scope;
       if (Completion.Kind == CompletionItemKind::Missing)
         Completion.Kind = toCompletionItemKind(C.IndexResult->SymInfo.Kind);
       if (Completion.Name.empty())
clang::CodeCompleteOptions CodeCompleteOptions::getClangCompleteOpts() const {
   // FIXME: ideally, we should support doxygen in some form, e.g. do markdown
   // formatting of the comments.
   Result.IncludeBriefComments = false;
 
   // When an is used, Sema is responsible for completing the main file,
   // the index can provide results from the preamble.
   // Tell Sema not to deserialize the preamble to look for results.
   Result.LoadExternal = !Index;
+  Result.IncludeFixIts = IncludeFixIts;
 
   return Result;
 }
 
 // Runs Sema-based (AST) and Index-based completion, returns merged results.
 //
 // There are a few tricky considerations:
 //   - the AST provides information needed for the index query (e.g. which
class CodeCompleteFlow {
   PathRef FileName;
   IncludeStructure Includes; // Complete once the compiler runs.
   const CodeCompleteOptions &Opts;
   // Sema takes ownership of Recorder. Recorder is valid until Sema cleanup.
   CompletionRecorder *Recorder = nullptr;
   int NSema = 0, NIndex = 0, NBoth = 0; // Counters for logging.
   bool Incomplete = false; // Would more be available with a higher limit?
   llvm::Optional<FuzzyMatcher> Filter;       // Initialized once Sema runs.
   std::vector<std::string> QueryScopes;      // Initialized once Sema runs.
   // Include-insertion and proximity scoring rely on the include structure.
   // This is available after Sema has run.
   llvm::Optional<IncludeInserter> Inserter;  // Available during runWithSema.
   llvm::Optional<URIDistance> FileProximity; // Initialized once Sema runs.
 
 public:
   // A CodeCompleteFlow object is only useful for calling run() exactly once.
   CodeCompleteFlow(PathRef FileName, const IncludeStructure &Includes,
if (Opts.Index->fuzzyFind(
       Incomplete = true;
     return std::move(ResultsBuilder).build();
   }
 
   // Merges Sema and Index results where possible, to form CompletionCandidates.
   // Groups overloads if desired, to form CompletionCandidate::Bundles.
   // The bundles are scored and top results are returned, best to worst.
   std::vector<ScoredBundle>
       mergeResults(const std::vector<CodeCompletionResult> &SemaResults,
                    const SymbolSlab &IndexResults) {
     trace::Span Tracer("Merge and score results");
     std::vector<CompletionCandidate::Bundle> Bundles;
     llvm::DenseMap<size_t, size_t> BundleLookup;
     auto AddToBundles = [&](const CodeCompletionResult *SemaResult,
                             const Symbol *IndexResult) {
       CompletionCandidate C;
       C.SemaResult = SemaResult;
       C.IndexResult = IndexResult;
CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
   LSP.kind = Kind;
   LSP.detail = BundleSize > 1 ? llvm::formatv("[{0} overloads]", BundleSize)
                               : ReturnType;
   if (!Header.empty())
     LSP.detail += "\n" + Header;
   LSP.documentation = Documentation;
   LSP.sortText = sortText(Score.Total, Name);
   LSP.filterText = Name;
+  // FIXME(kadircet): Use LSP.textEdit instead of insertText, because it causes
+  // undesired behaviours. Like completing "this.^" into "this-push_back".
   LSP.insertText = RequiredQualifier + Name;
   if (Opts.EnableSnippets)
     LSP.insertText += SnippetSuffix;
   LSP.insertTextFormat = Opts.EnableSnippets ? InsertTextFormat::Snippet
                                              : InsertTextFormat::PlainText;
+  LSP.additionalTextEdits.reserve(FixIts.size() + (HeaderInsertion ? 1 : 0));
+  for (const auto &FixIt : FixIts)
+    LSP.additionalTextEdits.push_back(FixIt);
   if (HeaderInsertion)
-    LSP.additionalTextEdits = {*HeaderInsertion};
+    LSP.additionalTextEdits.push_back(*HeaderInsertion);
   return LSP;
 }
 
 raw_ostream &operator<<(raw_ostream &OS, const CodeCompletion &C) {
   // For now just lean on CompletionItem.
   return OS << C.render(CodeCompleteOptions());
 }
 

clangd/Diagnostics.cpp

Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) {
   }
   // If no suitable range is found, just use the token at the location.
   auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L);
   if (!R.isValid()) // Fall back to location only, let the editor deal with it.
     R = CharSourceRange::getCharRange(Loc);
   return halfOpenToRange(M, R);
 }
 
-TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
-                    const LangOptions &L) {
-  TextEdit Result;
-  Result.range =
-      halfOpenToRange(M, Lexer::makeFileCharRange(FixIt.RemoveRange, M, L));
-  Result.newText = FixIt.CodeToInsert;
-  return Result;
-}
-
 bool isInsideMainFile(const SourceLocation Loc, const SourceManager &M) {
   return Loc.isValid() && M.isInMainFile(Loc);
 }
 
 bool isInsideMainFile(const clang::Diagnostic &D) {
   if (!D.hasSourceManager())
     return false;
 

clangd/Protocol.h

 struct TextEdit {
   /// The range of the text document to be manipulated. To insert
   /// text into a document create a range where start === end.
   Range range;
 
   /// The string to be inserted. For delete operations use an
   /// empty string.
   std::string newText;
+
+  bool operator==(const TextEdit &rhs) const {
+    return newText == rhs.newText && range == rhs.range;
+  }
 };
 bool fromJSON(const llvm::json::Value &, TextEdit &);
 llvm::json::Value toJSON(const TextEdit &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const TextEdit &);
 
 struct TextDocumentItem {
   /// The text document's URI.
   URIForFile uri;

clangd/Quality.h

 llvm::raw_ostream &operator<<(llvm::raw_ostream &,
                               const SymbolQualitySignals &);
 
 /// Attributes of a symbol-query pair that affect how much we like it.
 struct SymbolRelevanceSignals {
   /// 0-1+ fuzzy-match score for unqualified name. Must be explicitly assigned.
   float NameMatch = 1;
   bool Forbidden = false; // Unavailable (e.g const) or inaccessible (private).
+  /// Whether fixits needs to be applied for that completion or not.
+  bool NeedsFixIts = false;
 
   URIDistance *FileProximityMatch = nullptr;
   /// This is used to calculate proximity between the index symbol and the
   /// query.
   llvm::StringRef SymbolURI;
   /// Proximity between best declaration and the query. [0-1], 1 is closest.
   /// FIXME: unify with index proximity score - signals should be
   /// source-independent.

clangd/Quality.cpp

float DeclProximity = (hasDeclInMainFile(*SemaCCResult.Declaration) ||
                               : 0.6;
     SemaProximityScore = std::max(DeclProximity, SemaProximityScore);
     IsInstanceMember |= isInstanceMember(SemaCCResult.Declaration);
   }
 
   // Declarations are scoped, others (like macros) are assumed global.
   if (SemaCCResult.Declaration)
     Scope = std::min(Scope, computeScope(SemaCCResult.Declaration));
+
+  NeedsFixIts = !SemaCCResult.FixIts.empty();
 }
 
 static std::pair<float, unsigned> proximityScore(llvm::StringRef SymbolURI,
                                                  URIDistance *D) {
   if (!D || SymbolURI.empty())
     return {0.f, 0u};
   unsigned Distance = D->distance(SymbolURI);
   // Assume approximately default options are used for sensible scoring.
   return {std::exp(Distance * -0.4f / FileDistanceOptions().UpCost), Distance};
 }
float SymbolRelevanceSignals::evaluate() const {
 
   // Penalize non-instance members when they are accessed via a class instance.
   if (!IsInstanceMember &&
       (Context == CodeCompletionContext::CCC_DotMemberAccess ||
        Context == CodeCompletionContext::CCC_ArrowMemberAccess)) {
     Score *= 0.5;
   }
 
+  // Penalize for FixIts.
+  if (NeedsFixIts)
+    Score *= 0.5;
+
   return Score;
 }
 
 raw_ostream &operator<<(raw_ostream &OS, const SymbolRelevanceSignals &S) {
   OS << formatv("=== Symbol relevance: {0}\n", S.evaluate());
   OS << formatv("\tName match: {0}\n", S.NameMatch);
   OS << formatv("\tForbidden: {0}\n", S.Forbidden);
+  OS << formatv("\tNeedsFixIts: {0}\n", S.NeedsFixIts);
   OS << formatv("\tIsInstanceMember: {0}\n", S.IsInstanceMember);
   OS << formatv("\tContext: {0}\n", getCompletionKindString(S.Context));
   OS << formatv("\tSymbol URI: {0}\n", S.SymbolURI);
   if (S.FileProximityMatch) {
     auto Score = proximityScore(S.SymbolURI, S.FileProximityMatch);
     OS << formatv("\tIndex proximity: {0} (distance={1})\n", Score.first,
                   Score.second);
   }

clangd/SourceCode.h

 //===--- SourceCode.h - Manipulating source code as strings -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // Various code that examines C++ source code without using heavy AST machinery
 // (and often not even the lexer). To be used sparingly!
 //
 //===----------------------------------------------------------------------===//
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SOURCECODE_H
 #include "Protocol.h"
+#include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Tooling/Core/Replacement.h"
 
 namespace clang {
 class SourceManager;
 
 namespace clangd {
 

 TextEdit replacementToEdit(StringRef Code, const tooling::Replacement &R);
 
 std::vector<TextEdit> replacementsToEdits(StringRef Code,
                                           const tooling::Replacements &Repls);
 
 /// Get the absolute file path of a given file entry.
 llvm::Optional<std::string> getAbsoluteFilePath(const FileEntry *F,
                                                 const SourceManager &SourceMgr);
+
+TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
+                    const LangOptions &L);
+
 } // namespace clangd
 } // namespace clang
 #endif

clangd/SourceCode.cpp

 //===--- SourceCode.h - Manipulating source code as strings -----*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 #include "SourceCode.h"
 
 #include "Logger.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Lex/Lexer.h"
 #include "llvm/Support/Errc.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/Path.h"
 
 namespace clang {
if (!llvm::sys::path::is_absolute(FilePath)) {
     if (!SourceMgr.getFileManager().makeAbsolutePath(FilePath)) {
       log("Could not turn relative path to absolute: {0}", FilePath);
       return llvm::None;
     }
   }
   return FilePath.str().str();
 }
 
+TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M,
+                    const LangOptions &L) {
+  TextEdit Result;
+  Result.range =
+      halfOpenToRange(M, Lexer::makeFileCharRange(FixIt.RemoveRange, M, L));
+  Result.newText = FixIt.CodeToInsert;
+  return Result;
+}
+
 } // namespace clangd
 } // namespace clang

unittests/clangd/CodeCompleteTests.cpp

 
 std::unique_ptr<SymbolIndex> memIndex(std::vector<Symbol> Symbols) {
   SymbolSlab::Builder Slab;
   for (const auto &Sym : Symbols)
     Slab.insert(Sym);
   return MemIndex::build(std::move(Slab).build());
 }
 
+CodeCompleteResult completions(ClangdServer &Server, StringRef TestCode,
+                               Position point,
+                               std::vector<Symbol> IndexSymbols = {},
+                               clangd::CodeCompleteOptions Opts = {}) {
+  std::unique_ptr<SymbolIndex> OverrideIndex;
+  if (!IndexSymbols.empty()) {
+    assert(!Opts.Index && "both Index and IndexSymbols given!");
+    OverrideIndex = memIndex(std::move(IndexSymbols));
+    Opts.Index = OverrideIndex.get();
+  }
+
+  auto File = testPath("foo.cpp");
+  runAddDocument(Server, File, TestCode);
+  auto CompletionList = cantFail(runCodeComplete(Server, File, point, Opts));
+  return CompletionList;
+}
+
 CodeCompleteResult completions(ClangdServer &Server, StringRef Text,
                                std::vector<Symbol> IndexSymbols = {},
                                clangd::CodeCompleteOptions Opts = {}) {
   std::unique_ptr<SymbolIndex> OverrideIndex;
   if (!IndexSymbols.empty()) {
     assert(!Opts.Index && "both Index and IndexSymbols given!");
     OverrideIndex = memIndex(std::move(IndexSymbols));
     Opts.Index = OverrideIndex.get();
   }
CodeCompleteResult completions(StringRef Text,
   MockFSProvider FS;
   MockCompilationDatabase CDB;
   IgnoreDiagnostics DiagConsumer;
   ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
   return completions(Server, Text, std::move(IndexSymbols), std::move(Opts));
 }
 
 std::string replace(StringRef Haystack, StringRef Needle, StringRef Repl) {
   std::string Result;
   raw_string_ostream OS(Result);
   std::pair<StringRef, StringRef> Split;
   for (Split = Haystack.split(Needle); !Split.second.empty();
        Split = Split.first.split(Needle))
     OS << Split.first << Repl;
   Result += Split.first;
   OS.flush();
   return Result;
 }
 
 // Helpers to produce fake index symbols for memIndex() or completions().
 // USRFormat is a regex replacement string for the unqualified part of the USR.
 Symbol sym(StringRef QName, index::SymbolKind Kind, StringRef USRFormat) {
   Symbol Sym;
   std::string USR = "c:"; // We synthesize a few simple cases of USRs by hand!
auto Results = completions(
           void f() { ns::x^ }
       )cpp",
       {cls("ns::XYZ"), func("ns::foo")});
   EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("XYZ")));
 }
 
 TEST(CompletionTest, ReferencesAffectRanking) {
   auto Results = completions("int main() { abs^ }", {ns("absl"), func("absb")});
   EXPECT_THAT(Results.Completions, HasSubsequence(Named("absb"), Named("absl")));
   Results = completions("int main() { abs^ }",
                         {withReferences(10000, ns("absl")), func("absb")});
   EXPECT_THAT(Results.Completions,
               HasSubsequence(Named("absl"), Named("absb")));
 }
 
 TEST(CompletionTest, GlobalQualified) {
   auto Results = completions(
auto Results = completions(
             x.^;
           }
         }
       )cpp");
 
   EXPECT_THAT(Results.Context, CodeCompletionContext::CCC_DotMemberAccess);
 }
 
+TEST(CompletionTest, FixItForArrowToDot) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+  Annotations TestCode(
+      R"cpp(
+        class Auxilary {
+         public:
+          void AuxFunction();
+        };
+        class ClassWithPtr {
+         public:
+          void MemberFunction();
+          Auxilary* operator->() const;
+          Auxilary* Aux;
+        };
+        void f() {
+          ClassWithPtr x;
+          x[[->]]^;
+        }
+      )cpp");
+  auto Results =
+      completions(Server, TestCode.code(), TestCode.point(), {}, Opts);
+  EXPECT_EQ(Results.Completions.size(), 3u);
+
+  TextEdit ReplacementEdit;
+  ReplacementEdit.range = TestCode.range();
+  ReplacementEdit.newText = ".";
+  for (const auto &C : Results.Completions) {
+    EXPECT_TRUE(C.FixIts.size() == 1u || C.Name == "AuxFunction");
+    if (!C.FixIts.empty())
+      EXPECT_THAT(C.FixIts, ElementsAre(ReplacementEdit));
+  }
+}
+
+TEST(CompletionTest, FixItForDotToArrow) {
+  MockFSProvider FS;
+  MockCompilationDatabase CDB;
+  IgnoreDiagnostics DiagConsumer;
+  ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
+
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+  Annotations TestCode(
+      R"cpp(
+        class Auxilary {
+         public:
+          void AuxFunction();
+        };
+        class ClassWithPtr {
+         public:
+          void MemberFunction();
+          Auxilary* operator->() const;
+          Auxilary* Aux;
+        };
+        void f() {
+          ClassWithPtr x;
+          x[[.]]^;
+        }
+      )cpp");
+  auto Results =
+      completions(Server, TestCode.code(), TestCode.point(), {}, Opts);
+  EXPECT_EQ(Results.Completions.size(), 3u);
+
+  TextEdit ReplacementEdit;
+  ReplacementEdit.range = TestCode.range();
+  ReplacementEdit.newText = "->";
+  for (const auto &C : Results.Completions) {
+    EXPECT_TRUE(C.FixIts.empty() || C.Name == "AuxFunction");
+    if (!C.FixIts.empty()) {
+      EXPECT_THAT(C.FixIts, ElementsAre(ReplacementEdit));
+    }
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

unittests/clangd/QualityTests.cpp

TEST(QualityTests, ConstructorQuality) {
   EXPECT_EQ(Q.Category, SymbolQualitySignals::Constructor);
 
   Q.Category = SymbolQualitySignals::Unknown;
   const Symbol &CtorSym = findSymbol(Symbols, "Foo::Foo");
   Q.merge(CtorSym);
   EXPECT_EQ(Q.Category, SymbolQualitySignals::Constructor);
 }
 
+TEST(QualityTests, ItemWithFixItsRankedDown) {
+  CodeCompleteOptions Opts;
+  Opts.IncludeFixIts = true;
+
+  auto Header = TestTU::withHeaderCode(R"cpp(
+        int x;
+      )cpp");
+  auto AST = Header.build();
+
+  SymbolRelevanceSignals RelevanceWithFixIt;
+  RelevanceWithFixIt.merge(CodeCompletionResult(&findDecl(AST, "x"), 0, nullptr,
+                                                false, true, {FixItHint{}}));
+  EXPECT_TRUE(RelevanceWithFixIt.NeedsFixIts);
+
+  SymbolRelevanceSignals RelevanceWithoutFixIt;
+  RelevanceWithoutFixIt.merge(
+      CodeCompletionResult(&findDecl(AST, "x"), 0, nullptr, false, true, {}));
+  EXPECT_FALSE(RelevanceWithoutFixIt.NeedsFixIts);
+
+  EXPECT_LT(RelevanceWithFixIt.evaluate(), RelevanceWithoutFixIt.evaluate());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang