[clangd] Make use of diagnostic tags for some clang diags

[kadircet]

Summary

It is not great to list diag ids by hand, but I don't see any other
solution unless diagnostics are annotated with these explicitly, which is a
bigger change in clang and I am not sure if would be worth it.

Diagnostics handled by this patch is by no means exhaustive, there might be
other checks that don't mention "unused"/"deprecated" in their names. But it
feels like this should be enough to catch common diagnostics and can be extended
over time.

[kadircet]

• Fix base

[sammccall]

clang-tools-extra/clangd/Diagnostics.cpp

330

I think it might be nicer if this took a clangd::Diag& and added the tags to it, then we could naturally group the clang-tidy list along with the main clang diag list (I don't think having them separately serves anyone well)

331

nit: static const auto& DeprecatedDiags = *new llvm::DenseSet<unsigned>{...

This suppresses destruction

clang-tools-extra/clangd/Diagnostics.h

109 ↗

(On Diff #363038)

Maybe SmallVector or even Optional since this is 0-1 integers in practice?

clang-tools-extra/clangd/Protocol.cpp

1455

keep ordering consistent with the header? so before Diagnostic

[kadircet]

• Address comments

clang-tools-extra/clangd/Diagnostics.cpp

 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/Capacity.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cstddef>
+#include <vector>
 
 namespace clang {
 namespace clangd {
 namespace {
 
 const char *getDiagnosticCode(unsigned ID) {
   switch (ID) {
 #define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROPU, SFINAE, NOWERROR,      \
std::string noteMessage(const Diag &Main, const DiagBase &Note,
   if (!Opts.EmitRelatedLocations) {
     OS << "\n\n";
     printDiag(OS, Main);
   }
   OS.flush();
   return capitalize(std::move(Result));
 }
 
-llvm::SmallVector<DiagnosticTag, 1>
-getDiagnosticTags(const clang::Diagnostic &D) {
-  llvm::SmallVector<DiagnosticTag, 1> Result;
-  if ((diag::warn_deprecated <= D.getID() &&
-       D.getID() <= diag::warn_deprecated_volatile_structured_binding) ||
-      D.getID() == diag::warn_access_decl_deprecated ||
-      D.getID() == diag::warn_atl_uuid_deprecated ||
-      D.getID() == diag::warn_opencl_attr_deprecated_ignored ||
-      D.getID() == diag::warn_property_method_deprecated ||
-      D.getID() == diag::warn_vector_mode_deprecated) {
+std::vector<DiagnosticTag> getDiagnosticTags(unsigned DiagID) {
+  static const llvm::DenseSet<unsigned> DeprecatedDiags = {
+      diag::warn_access_decl_deprecated,
+      diag::warn_atl_uuid_deprecated,
+      diag::warn_deprecated,
+      diag::warn_deprecated_altivec_src_compat,
+      diag::warn_deprecated_comma_subscript,
+      diag::warn_deprecated_compound_assign_volatile,
+      diag::warn_deprecated_copy,
+      diag::warn_deprecated_copy_with_dtor,
+      diag::warn_deprecated_copy_with_user_provided_copy,
+      diag::warn_deprecated_copy_with_user_provided_dtor,
+      diag::warn_deprecated_def,
+      diag::warn_deprecated_increment_decrement_volatile,
+      diag::warn_deprecated_message,
+      diag::warn_deprecated_redundant_constexpr_static_def,
+      diag::warn_deprecated_register,
+      diag::warn_deprecated_simple_assign_volatile,
+      diag::warn_deprecated_string_literal_conversion,
+      diag::warn_deprecated_this_capture,
+      diag::warn_deprecated_volatile_param,
+      diag::warn_deprecated_volatile_return,
+      diag::warn_deprecated_volatile_structured_binding,
+      diag::warn_opencl_attr_deprecated_ignored,
+      diag::warn_property_method_deprecated,
+      diag::warn_vector_mode_deprecated,
+  };
+  static const llvm::DenseSet<unsigned> UnusedDiags = {
+      diag::warn_opencl_attr_deprecated_ignored,
+      diag::warn_pragma_attribute_unused,
+      diag::warn_unused_but_set_parameter,
+      diag::warn_unused_but_set_variable,
+      diag::warn_unused_comparison,
+      diag::warn_unused_const_variable,
+      diag::warn_unused_exception_param,
+      diag::warn_unused_function,
+      diag::warn_unused_label,
+      diag::warn_unused_lambda_capture,
+      diag::warn_unused_local_typedef,
+      diag::warn_unused_member_function,
+      diag::warn_unused_parameter,
+      diag::warn_unused_private_field,
+      diag::warn_unused_property_backing_ivar,
+      diag::warn_unused_template,
+      diag::warn_unused_variable,
+  };
+  std::vector<DiagnosticTag> Result;
+  if (DeprecatedDiags.contains(DiagID)) {
     Result.push_back(DiagnosticTag::Deprecated);
-  } else if (
-      // Warnings about unused function results can be semantically important,
-      // e.g. an llvm::Error that needs to be checked.
-      D.getID() != diag::warn_unused_call &&
-      ((diag::warn_unused_but_set_parameter <= D.getID() &&
-        D.getID() <= diag::warn_unused_volatile) ||
-       D.getID() == diag::warn_pragma_attribute_unused)) {
+  } else if (UnusedDiags.contains(DiagID)) {
     Result.push_back(DiagnosticTag::Unnecessary);
   }
   return Result;
 }
 } // namespace
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const DiagBase &D) {
   OS << "[";
for (auto &Note : D.Notes) {
       DiagnosticRelatedInformation RelInfo;
       RelInfo.location.range = Note.Range;
       RelInfo.location.uri =
           URIForFile::canonicalize(*Note.AbsFile, File.file());
       RelInfo.message = noteMessage(D, Note, Opts);
       Main.relatedInformation->push_back(std::move(RelInfo));
     }
   }
+  Main.tags = D.Tags;
   OutFn(std::move(Main), D.Fixes);
 
   // If we didn't emit the notes as relatedLocations, emit separate diagnostics
   // so the user can find the locations easily.
   if (!Opts.EmitRelatedLocations)
     for (auto &Note : D.Notes) {
       if (!Note.InsideMainFile)
         continue;
if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) {
       } else {
         StringRef Name(ClangDiag);
         // Almost always an error, with a name like err_enum_class_reference.
         // Drop the err_ prefix for brevity.
         Name.consume_front("err_");
         Diag.Name = std::string(Name);
       }
       Diag.Source = Diag::Clang;
+      Diag.Tags = getDiagnosticTags(Diag.ID);
       continue;
     }
     if (Tidy != nullptr) {
       std::string TidyDiag = Tidy->getCheckName(Diag.ID);
       if (!TidyDiag.empty()) {
         Diag.Name = std::move(TidyDiag);
         Diag.Source = Diag::ClangTidy;
+        // FIXME: Set diagnostic tags based on check names.
         // clang-tidy bakes the name into diagnostic messages. Strip it out.
         // It would be much nicer to make clang-tidy not do this.
         auto CleanMessage = [&](std::string &Msg) {
           StringRef Rest(Msg);
           if (Rest.consume_back("]") && Rest.consume_back(Diag.Name) &&
               Rest.consume_back(" ["))
             Msg.resize(Rest.size());
         };
if (!isNote(DiagLevel)) {
     if (isExcluded(LastDiag->ID))
       LastDiag->Severity = DiagnosticsEngine::Ignored;
     if (DiagCB)
       DiagCB(Info, *LastDiag);
     // Don't bother filling in the rest if diag is going to be dropped.
     if (LastDiag->Severity == DiagnosticsEngine::Ignored)
       return;
 
-    LastDiag->Tags = getDiagnosticTags(Info);
     LastDiagLoc.emplace(Info.getLocation(), Info.getSourceManager());
     LastDiagOriginallyError = OriginallyError;
     if (!Info.getFixItHints().empty())
       AddFix(true /* try to invent a message instead of repeating the diag */);
     if (Fixer) {
       auto ExtraFixes = Fixer(LastDiag->Severity, Info);
       LastDiag->Fixes.insert(LastDiag->Fixes.end(), ExtraFixes.begin(),
                              ExtraFixes.end());

clang-tools-extra/clangd/Protocol.h

enum DiagnosticTag {
   /// Clients are allowed to render diagnostics with this tag faded out instead
   /// of having an error squiggle.
   Unnecessary = 1,
   /// Deprecated or obsolete code.
   ///
   /// Clients are allowed to rendered diagnostics with this tag strike through.
   Deprecated = 2,
 };
+llvm::json::Value toJSON(DiagnosticTag Tag);
+
 struct CodeAction;
 struct Diagnostic {
   /// The range at which the message applies.
   Range range;
 
   /// The diagnostic's severity. Can be omitted. If omitted it is up to the
   /// client to interpret diagnostics as error, warning, info or hint.
   int severity = 0;

clang-tools-extra/clangd/Protocol.cpp

std::function<void(const ASTNode &, unsigned)> Print = [&](const ASTNode &N,
     OS << "\n";
     for (const ASTNode &C : N.children)
       Print(C, Level + 1);
   };
   Print(Root, 0);
   return OS;
 }
 
+llvm::json::Value toJSON(DiagnosticTag Tag) { return static_cast<int>(Tag); }
 } // namespace clangd
 } // namespace clang

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

 }
 
 ::testing::Matcher<const Diag &>
 WithNote(::testing::Matcher<Note> NoteMatcher1,
          ::testing::Matcher<Note> NoteMatcher2) {
   return Field(&Diag::Notes, UnorderedElementsAre(NoteMatcher1, NoteMatcher2));
 }
 
+::testing::Matcher<const Diag &>
+WithTag(::testing::Matcher<DiagnosticTag> TagMatcher) {
+  return Field(&Diag::Tags, Contains(TagMatcher));
+}
+
 MATCHER_P2(Diag, Range, Message,
            "Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
   return arg.Range == Range && arg.Message == Message;
 }
 
 MATCHER_P3(Fix, Range, Replacement, Message,
            "Fix " + llvm::to_string(Range) + " => " +
                ::testing::PrintToString(Replacement) + " = [" + Message + "]") {

 TEST(DiagnosticsTest, ToLSP) {
   URIForFile MainFile =
       URIForFile::canonicalize(testPath("foo/bar/main.cpp"), "");
   URIForFile HeaderFile =
       URIForFile::canonicalize(testPath("foo/bar/header.h"), "");
 
   clangd::Diag D;
   D.ID = clang::diag::err_undeclared_var_use;
+  D.Tags = {DiagnosticTag::Unnecessary};
   D.Name = "undeclared_var_use";
   D.Source = clangd::Diag::Clang;
   D.Message = "something terrible happened";
   D.Range = {pos(1, 2), pos(3, 4)};
   D.InsideMainFile = true;
   D.Severity = DiagnosticsEngine::Error;
   D.File = "foo/bar/main.cpp";
   D.AbsFile = std::string(MainFile.file());
TEST(DiagnosticsTest, ToLSP) {
   MainLSP.source = "clang";
   MainLSP.message =
       R"(Something terrible happened (fix available)
 
 main.cpp:6:7: remark: declared somewhere in the main file
 
 ../foo/baz/header.h:10:11:
 note: declared somewhere in the header file)";
+  MainLSP.tags = {DiagnosticTag::Unnecessary};
 
   clangd::Diagnostic NoteInMainLSP;
   NoteInMainLSP.range = NoteInMain.Range;
   NoteInMainLSP.severity = getSeverity(DiagnosticsEngine::Remark);
   NoteInMainLSP.message = R"(Declared somewhere in the main file
 
 main.cpp:2:3: error: something terrible happened)";
 
TU.ExtraArgs = {"-Wnewline-eof"};
     Annotations Code("#define FOO[[]]");
     TU.Code = Code.code().str();
     auto AST = TU.build();
     EXPECT_THAT(
         *AST.getDiagnostics(),
         testing::Contains(Diag(Code.range(), "no newline at end of file")));
   }
 }
+
+TEST(Diagnostics, Tags) {
+  TestTU TU;
+  TU.ExtraArgs = {"-Wunused", "-Wdeprecated"};
+  Annotations Test(R"cpp(
+  void bar() __attribute__((deprecated));
+  void foo() {
+    int $unused[[x]];
+    $deprecated[[bar]]();
+  })cpp");
+  TU.Code = Test.code().str();
+  EXPECT_THAT(*TU.build().getDiagnostics(),
+              UnorderedElementsAre(
+                  AllOf(Diag(Test.range("unused"), "unused variable 'x'"),
+                        WithTag(DiagnosticTag::Unnecessary)),
+                  AllOf(Diag(Test.range("deprecated"), "'bar' is deprecated"),
+                        WithTag(DiagnosticTag::Deprecated))));
+}
 } // namespace
 } // namespace clangd
 } // namespace clang