[clangd] Add "Deprecated" field to Symbol and CodeCompletion.

[ioeric]

Summary

Also set "deprecated" field in LSP CompletionItem.

[sammccall]

I think you also need to update SymbolsYAML and Serialization.

clangd/Protocol.cpp

520

do we actually want this in JSON?
(genuinely unsure - any clients aware of this extension?)

clangd/Protocol.h

771

this is a clangd extension.
(right?)

clangd/index/Index.h

249

would you mind packing this together with IsIndexedForCompletion, for memory size?
either as an actual bitfield bool Deprecated : 1 = false or as enum flags enum Flags : uint8_t { IndexedForCompletion, Deprecated, }; Flags flags

The latter will simplify life for serialization, but up to you.

[ioeric]

• Pack flags in Symbol.

clangd/Protocol.cpp

520

This is actually defined in LSP: https://github.com/Microsoft/language-server-protocol/blob/gh-pages/specification.md#completion-request-leftwards_arrow_with_hook

clangd/index/Index.h

249

Done. Pack them into flags.

[kadircet]

clangd/index/Index.h

171

nit: rename to IndexedForCodeCompletion, since it is more of an attribute having is in the name doesn't seem so cool. Instead of the attributes themselves maybe the checkers below should have "is" prefix.

172

nit: Add a comment similar to above one leading to Deprecated()?

248

nit: rename to isDeprecated ?

unittests/clangd/CodeCompleteTests.cpp

1359

Maybe do this on its own render so that we can have both code paths covered. Just before rendering with Opts.EnableSnippets = true below. We can simply set this one render again and check deprecated is set to true.

[sammccall]

Agree with Kadir's comments.
I'd just suggest reducing boilerplate a bit by taking some shortcuts.

clangd/index/Index.h

167

enum class is a pain for bitfields.
I'd just make this a plain enum nested inside symbol, then you don't need to define the operators and don't need to cast as often.

248

FWIW I don't think these accessors pull their weight: in calller code if (Sym.Flags & Symbol::IsDeprecated) is clear enough

clangd/index/Serialization.cpp

309 ↗

(On Diff #164205)

3

[ioeric]

• addressed review comments

[ioeric]

• merged with origin/master

[kadircet]

unittests/clangd/SymbolCollectorTests.cpp

1020 ↗

(On Diff #164205)

No need for the optional parameters,

void TestClangc() __attribute__((deprecated));

should work as well

clangd/CodeComplete.h

struct Scores {
     // e.g. symbols with lots of incoming references have higher quality.
     float Quality = 0.f;
     // Relevance describes how well this candidate matched the query.
     // e.g. symbols from nearby files have higher relevance.
     float Relevance = 0.f;
   };
   Scores Score;
 
+  /// Indicates if this item is deprecated.
+  bool Deprecated = false;
+
   // Serialize this to an LSP completion item. This is a lossy operation.
   CompletionItem render(const CodeCompleteOptions &) const;
 };
 raw_ostream &operator<<(raw_ostream &, const CodeCompletion &);
 struct CodeCompleteResult {
   std::vector<CodeCompletion> Completions;
   bool HasMore = false;
   CodeCompletionContext::Kind Context = CodeCompletionContext::CCC_Other;

clangd/CodeComplete.cpp

if (C.SemaResult) {
         Completion.FixIts.push_back(
             toTextEdit(FixIt, ASTCtx.getSourceManager(), ASTCtx.getLangOpts()));
       }
       std::sort(Completion.FixIts.begin(), Completion.FixIts.end(),
                 [](const TextEdit &X, const TextEdit &Y) {
                   return std::tie(X.range.start.line, X.range.start.character) <
                          std::tie(Y.range.start.line, Y.range.start.character);
                 });
+      Completion.Deprecated |=
+          (C.SemaResult->Availability == CXAvailability_Deprecated);
     }
     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())
         Completion.Name = C.IndexResult->Name;
+      Completion.Deprecated |= C.IndexResult->Deprecated;
     }
 
     // Turn absolute path into a literal string that can be #included.
     auto Inserted =
         [&](StringRef Header) -> Expected<std::pair<std::string, bool>> {
       auto ResolvedDeclaring =
           toHeaderFile(C.IndexResult->CanonicalDeclaration.FileURI, FileName);
       if (!ResolvedDeclaring)
LSP.label = ((InsertInclude && InsertInclude->Insertion)
                    ? Opts.IncludeIndicator.Insert
                    : Opts.IncludeIndicator.NoInsert) +
               (Opts.ShowOrigins ? "[" + llvm::to_string(Origin) + "]" : "") +
               RequiredQualifier + Name + Signature;
 
   LSP.kind = Kind;
   LSP.detail = BundleSize > 1 ? llvm::formatv("[{0} overloads]", BundleSize)
                               : ReturnType;
+  LSP.deprecated = Deprecated;
   if (InsertInclude)
     LSP.detail += "\n" + InsertInclude->Header;
   LSP.documentation = Documentation;
   LSP.sortText = sortText(Score.Total, Name);
   LSP.filterText = Name;
   LSP.textEdit = {CompletionTokenRange, RequiredQualifier + Name};
   // Merge continuous additionalTextEdits into main edit. The main motivation
   // behind this is to help LSP clients, it seems most of them are confused when
CompletionItem CodeCompletion::render(const CodeCompleteOptions &Opts) const {
 
   // FIXME(kadircet): Do not even fill insertText after making sure textEdit is
   // compatible with most of the editors.
   LSP.insertText = LSP.textEdit->newText;
   LSP.insertTextFormat = Opts.EnableSnippets ? InsertTextFormat::Snippet
                                              : InsertTextFormat::PlainText;
   if (InsertInclude && InsertInclude->Insertion)
     LSP.additionalTextEdits.push_back(*InsertInclude->Insertion);
+
   return LSP;
 }
 
 raw_ostream &operator<<(raw_ostream &OS, const CodeCompletion &C) {
   // For now just lean on CompletionItem.
   return OS << C.render(CodeCompleteOptions());
 }
 

clangd/Protocol.h

struct CompletionItem {
   /// contain the position at which completion has been requested.
   llvm::Optional<TextEdit> textEdit;
 
   /// An optional array of additional text edits that are applied when selecting
   /// this completion. Edits must not overlap with the main edit nor with
   /// themselves.
   std::vector<TextEdit> additionalTextEdits;
 
+  /// Indicates if this item is deprecated.
+  bool deprecated = false;
+
   // TODO(krasimir): The following optional fields defined by the language
   // server protocol are unsupported:
   //
   // data?: any - A data entry field that is preserved on a completion item
   //              between a completion and a completion resolve request.
 };
 llvm::json::Value toJSON(const CompletionItem &);
 llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CompletionItem &);

clangd/Protocol.cpp

json::Value toJSON(const CompletionItem &CI) {
   if (!CI.insertText.empty())
     Result["insertText"] = CI.insertText;
   if (CI.insertTextFormat != InsertTextFormat::Missing)
     Result["insertTextFormat"] = static_cast<int>(CI.insertTextFormat);
   if (CI.textEdit)
     Result["textEdit"] = *CI.textEdit;
   if (!CI.additionalTextEdits.empty())
     Result["additionalTextEdits"] = json::Array(CI.additionalTextEdits);
+  if (CI.deprecated)
+    Result["deprecated"] = CI.deprecated;
   return std::move(Result);
 }
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &O, const CompletionItem &I) {
   O << I.label << " - " << toJSON(I);
   return O;
 }
 

clangd/Quality.cpp

static bool isInstanceMember(const index::SymbolInfo &D) {
   case index::SymbolKind::Field:
     return true;
   default:
     return false;
   }
 }
 
 void SymbolQualitySignals::merge(const CodeCompletionResult &SemaCCResult) {
-  if (SemaCCResult.Availability == CXAvailability_Deprecated)
-    Deprecated = true;
-
+  Deprecated |= (SemaCCResult.Availability == CXAvailability_Deprecated);
   Category = categorize(SemaCCResult);
 
   if (SemaCCResult.Declaration) {
     if (auto *ID = SemaCCResult.Declaration->getIdentifier())
       ReservedName = ReservedName || isReserved(ID->getName());
   } else if (SemaCCResult.Kind == CodeCompletionResult::RK_Macro)
     ReservedName = ReservedName || isReserved(SemaCCResult.Macro->getName());
 }
 
 void SymbolQualitySignals::merge(const Symbol &IndexResult) {
+  Deprecated |= IndexResult.Deprecated;
   References = std::max(IndexResult.References, References);
   Category = categorize(IndexResult.SymInfo);
   ReservedName = ReservedName || isReserved(IndexResult.Name);
 }
 
 float SymbolQualitySignals::evaluate() const {
   float Score = 1;
 

clangd/index/Index.h

inline SymbolOrigin &operator|=(SymbolOrigin &A, SymbolOrigin B) {
   return A = A | B;
 }
 inline SymbolOrigin operator&(SymbolOrigin A, SymbolOrigin B) {
   return static_cast<SymbolOrigin>(static_cast<uint8_t>(A) &
                                    static_cast<uint8_t>(B));
 }
 raw_ostream &operator<<(raw_ostream &, SymbolOrigin);
 
 // The class presents a C++ symbol, e.g. class, function.
 //
 // WARNING: Symbols do not own much of their underlying data - typically strings
 // are owned by a SymbolSlab. They should be treated as non-owning references.
 // Copies are shallow.
 // When adding new unowned data fields to Symbol, remember to update:
 //   - SymbolSlab::Builder in Index.cpp, to copy them to the slab's storage.
 //   - mergeSymbol in Merge.cpp, to properly combine two Symbols.
 //
 // A fully documented symbol can be split as:
 // size_type std::map<k, t>::count(const K& key) const
 // | Return  |     Scope     |Name|    Signature     |
 // We split up these components to allow display flexibility later.
 struct Symbol {
struct IncludeHeaderWithReferences {
     unsigned References = 0;
   };
   /// One Symbol can potentially be incuded via different headers.
   ///   - If we haven't seen a definition, this covers all declarations.
   ///   - If we have seen a definition, this covers declarations visible from
   ///   any definition.
   llvm::SmallVector<IncludeHeaderWithReferences, 1> IncludeHeaders;
 
-  // FIXME: add extra fields for index scoring signals.
+  /// Indicates if the symbol is deprecated.
+  /// FIXME: also add deprecation message and fixit?
+  bool Deprecated = false;
 };
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Symbol &S);
 
 // Invokes Callback with each StringRef& contained in the Symbol.
 // Useful for deduplicating backing strings.
 template <typename Callback> void visitStrings(Symbol &S, const Callback &CB) {
   CB(S.Name);
   CB(S.Scope);

clangd/index/SymbolCollector.cpp

const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
   S.Signature = Signature;
   S.CompletionSnippetSuffix = SnippetSuffix;
   S.Documentation = Documentation;
   S.ReturnType = ReturnType;
   if (!Include.empty())
     S.IncludeHeaders.emplace_back(Include, 1);
 
   S.Origin = Opts.Origin;
+  S.Deprecated = ND.getAvailability() == AR_Deprecated;
   Symbols.insert(S);
   return Symbols.find(S.ID);
 }
 
 void SymbolCollector::addDefinition(const NamedDecl &ND,
                                     const Symbol &DeclSym) {
   if (DeclSym.Definition)
     return;

unittests/clangd/CodeCompleteTests.cpp

 Matcher<const std::vector<CodeCompletion> &> Has(std::string Name) {
   return Contains(Named(std::move(Name)));
 }
 Matcher<const std::vector<CodeCompletion> &> Has(std::string Name,
                                                  CompletionItemKind K) {
   return Contains(AllOf(Named(std::move(Name)), Kind(K)));
 }
 MATCHER(IsDocumented, "") { return !arg.Documentation.empty(); }
+MATCHER(Deprecated, "") { return arg.Deprecated; }
 
 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(), RefSlab());
 }
 
TEST(CompletionTest, Render) {
   C.Scope = "ns::Foo::";
   C.Documentation = "This is x().";
   C.Includes.emplace_back();
   auto &Include = C.Includes.back();
   Include.Header = "\"foo.h\"";
   C.Kind = CompletionItemKind::Method;
   C.Score.Total = 1.0;
   C.Origin = SymbolOrigin::AST | SymbolOrigin::Static;
+  C.Deprecated = true;
 
   CodeCompleteOptions Opts;
   Opts.IncludeIndicator.Insert = "^";
   Opts.IncludeIndicator.NoInsert = "";
   Opts.EnableSnippets = false;
 
   auto R = C.render(Opts);
   EXPECT_EQ(R.label, "Foo::x(bool) const");
   EXPECT_EQ(R.insertText, "Foo::x");
   EXPECT_EQ(R.insertTextFormat, InsertTextFormat::PlainText);
   EXPECT_EQ(R.filterText, "x");
   EXPECT_EQ(R.detail, "int\n\"foo.h\"");
   EXPECT_EQ(R.documentation, "This is x().");
   EXPECT_THAT(R.additionalTextEdits, IsEmpty());
   EXPECT_EQ(R.sortText, sortText(1.0, "x"));
+  EXPECT_TRUE(R.deprecated);
 
   Opts.EnableSnippets = true;
   R = C.render(Opts);
   EXPECT_EQ(R.insertText, "Foo::x(${0:bool})");
   EXPECT_EQ(R.insertTextFormat, InsertTextFormat::Snippet);
 
   Include.Insertion.emplace();
   R = C.render(Opts);
TEST(CompletionTest, NoInsertIncludeIfOnePresent) {
   sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000);
 
   EXPECT_THAT(
       completions(Server, "#include \"foo.h\"\nFun^", {sym}).Completions,
       UnorderedElementsAre(
           AllOf(Named("Func"), HasInclude("\"foo.h\""), Not(InsertInclude()))));
 }
 
+TEST(CompletionTest, DeprecatedResults) {
+  std::string Body = R"cpp(
+    void TestClangd();
+    void TestClangc() __attribute__((deprecated("", "")));
+  )cpp";
+
+  EXPECT_THAT(
+      completions(Body + "int main() { TestClang^ }").Completions,
+      UnorderedElementsAre(AllOf(Named("TestClangd"), Not(Deprecated())),
+                           AllOf(Named("TestClangc"), Deprecated())));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang

unittests/clangd/QualityTests.cpp

TEST(QualityTests, SymbolQualitySignalExtraction) {
   EXPECT_TRUE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
 
   Symbol F = findSymbol(Symbols, "_f");
   F.References = 24; // TestTU doesn't count references, so fake it.
   Quality = {};
   Quality.merge(F);
-  EXPECT_FALSE(Quality.Deprecated); // FIXME: Include deprecated bit in index.
+  EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);
   EXPECT_EQ(Quality.References, 24u);
   EXPECT_EQ(Quality.Category, SymbolQualitySignals::Function);
 
   Quality = {};
   Quality.merge(CodeCompletionResult(&findDecl(AST, "_f"), /*Priority=*/42));
   EXPECT_TRUE(Quality.Deprecated);
   EXPECT_FALSE(Quality.ReservedName);