[clangd] Use shorter, more recognizable codes for diagnostics.

[sammccall]

Summary

• for warnings, use "foo" if the warning is controlled by -Wfoo
• for errors, keep using the internal name (there's nothing better) but drop the err_ prefix

This comes at the cost of uniformity, it's no longer totally obvious
exactly what the code field contains. But the -Wname flags are so much
more useful to end-users than the internal warn_foo that this seems worth it.

[kadircet]

I agree that these are more useful. What about also adding "-W" in front of warning flags to make them more explicit and stand out from "non-flag" error names?

[sammccall]

-Wfoo instead of foo

[sammccall]

In D60822#1470227, @kadircet wrote:

I agree that these are more useful. What about also adding "-W" in front of warning flags to make them more explicit and stand out from "non-flag" error names?

I went back and forth on this, I think you're probably right. It's consistent with how clang-tidy check names vs warnings are exposed to users: "Diag text [-Wfoo]" and "Diag text [check-name]".
Let's see how it goes.

clangd/Diagnostics.cpp

int getSeverity(DiagnosticsEngine::Level L) {
   }
   llvm_unreachable("Unknown diagnostic level!");
 }
 
 std::vector<Diag> StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) {
   // Fill in name/source now that we have all the context needed to map them.
   for (auto &Diag : Output) {
     if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) {
-      Diag.Name = ClangDiag;
+      // Warnings controlled by -Wfoo are better recognized by that name.
+      StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(Diag.ID);
+      if (!Warning.empty()) {
+        Diag.Name = ("-W" + Warning).str();
+      } 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 = Name;
+      }
       Diag.Source = Diag::Clang;
       continue;
     }
     if (Tidy != nullptr) {
       std::string TidyDiag = Tidy->getCheckName(Diag.ID);
       if (!TidyDiag.empty()) {
         Diag.Name = std::move(TidyDiag);
         Diag.Source = Diag::ClangTidy;

test/clangd/compile-commands-path-in-initialize.test

 
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"initializationOptions":{"compilationDatabasePath":"INPUT_DIR/build-1"}}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://INPUT_DIR/the-file.cpp","languageId":"cpp","version":1,"text":"#if !defined(MACRO)\n#pragma message (\"MACRO is not defined\")\n#elif MACRO == 1\n#pragma message (\"MACRO is one\")\n#else\n#pragma message (\"woops\")\n#endif\nint main() {}\n"}}}
 # CHECK:   "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:   "params": {
 # CHECK-NEXT:     "diagnostics": [
 # CHECK-NEXT:       {
-# CHECK-NEXT:         "code": "warn_pragma_message",
+# CHECK-NEXT:         "code": "-W#pragma-messages",
 # CHECK-NEXT:         "message": "MACRO is one",
 ---
 {"jsonrpc":"2.0","id":10000,"method":"shutdown"}
 ---
 {"jsonrpc":"2.0","method":"exit"}

test/clangd/diagnostic-category.test

 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"categorySupport":true}}},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}}
 #      CHECK:    "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:    "params": {
 # CHECK-NEXT:     "diagnostics": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "category": "Semantic Issue",
-# CHECK-NEXT:        "code": "err_use_with_wrong_tag",
+# CHECK-NEXT:        "code": "use_with_wrong_tag",
 # CHECK-NEXT:        "message": "Use of 'Point' with tag type that does not match previous declaration (fix available)\n\nfoo.c:1:8: note: previous use is here",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
 # CHECK-NEXT:            "character": 22,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # CHECK-NEXT:            "character": 17,

test/clangd/diagnostics.test

 # RUN: clangd -lit-test -clang-tidy-checks=bugprone-sizeof-expression < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"void main() {\n(void)sizeof(42);\n}"}}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
-# CHECK-NEXT:        "code": "ext_main_returns_nonint",
+# CHECK-NEXT:        "code": "-Wmain-return-type",
 # CHECK-NEXT:        "message": "Return type of 'main' is not 'int' (fix available)",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
 # CHECK-NEXT:            "character": 4,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # CHECK-NEXT:            "character": 0,

test/clangd/did-change-configuration-params.test

 # CHECK-NEXT:    "uri": "file://{{.*}}/bar.c"
 # CHECK-NEXT:  }
 ---
 {"jsonrpc":"2.0","method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabaseChanges":{"/clangd-test/foo.c": {"workingDirectory":"/clangd-test2", "compilationCommand": ["clang", "-c", "foo.c", "-Wall", "-Werror"]}}}}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
-# CHECK-NEXT:        "code": "warn_uninit_var",
+# CHECK-NEXT:        "code": "-Wuninitialized",
 # CHECK-NEXT:        "message": "Variable 'i' is uninitialized when used here (fix available)",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
 # CHECK-NEXT:            "character": 28,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # CHECK-NEXT:            "character": 27,

test/clangd/execute-command.test

 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
-# CHECK-NEXT:        "code": "warn_condition_is_assignment",
+# CHECK-NEXT:        "code": "-Wparentheses",
 # CHECK-NEXT:        "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
 # CHECK-NEXT:            "character": 37,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # CHECK-NEXT:            "character": 32,

test/clangd/fixits-codeaction.test

 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"codeAction":{"codeActionLiteralSupport":{}}}},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
 #      CHECK:    "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
-# CHECK-NEXT:        "code": "warn_condition_is_assignment",
+# CHECK-NEXT:        "code": "-Wparentheses",
 # CHECK-NEXT:        "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
 # CHECK-NEXT:            "character": 37,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # CHECK-NEXT:            "character": 32,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          }
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "severity": 2,
 # CHECK-NEXT:        "source": "clang"
 # CHECK-NEXT:      }
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "uri": "file://{{.*}}/foo.c"
 # CHECK-NEXT:  }
 ---
-{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses (fixes available)", "code": "warn_condition_is_assignment", "source": "clang"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///foo.c"},"range":{"start":{"line":0,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"Using the result of an assignment as a condition without parentheses (fixes available)", "code": "-Wparentheses", "source": "clang"}]}}}
 #      CHECK:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "diagnostics": [
 # CHECK-NEXT:        {
-# CHECK-NEXT:          "code": "warn_condition_is_assignment",
+# CHECK-NEXT:          "code": "-Wparentheses",
 # CHECK-NEXT:          "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
 # CHECK-NEXT:          "range": {
 # CHECK-NEXT:            "end": {
 # CHECK-NEXT:              "character": 37,
 # CHECK-NEXT:              "line": 0
 # CHECK-NEXT:            },
 # CHECK-NEXT:            "start": {
 # CHECK-NEXT:              "character": 32,

 # CHECK-NEXT:        }
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "kind": "quickfix",
 # CHECK-NEXT:      "title": "place parentheses around the assignment to silence this warning"
 # CHECK-NEXT:    },
 # CHECK-NEXT:    {
 # CHECK-NEXT:      "diagnostics": [
 # CHECK-NEXT:        {
-# CHECK-NEXT:          "code": "warn_condition_is_assignment",
+# CHECK-NEXT:          "code": "-Wparentheses",
 # CHECK-NEXT:          "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
 # CHECK-NEXT:          "range": {
 # CHECK-NEXT:            "end": {
 # CHECK-NEXT:              "character": 37,
 # CHECK-NEXT:              "line": 0
 # CHECK-NEXT:            },
 # CHECK-NEXT:            "start": {
 # CHECK-NEXT:              "character": 32,

test/clangd/fixits-command.test

 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"int main(int i, char **a) { if (i = 2) {}}"}}}
 #      CHECK:    "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
-# CHECK-NEXT:        "code": "warn_condition_is_assignment",
+# CHECK-NEXT:        "code": "-Wparentheses",
 # CHECK-NEXT:        "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
 # CHECK-NEXT:        "range": {
 # CHECK-NEXT:          "end": {
 # CHECK-NEXT:            "character": 37,
 # CHECK-NEXT:            "line": 0
 # CHECK-NEXT:          },
 # CHECK-NEXT:          "start": {
 # CHECK-NEXT:            "character": 32,

test/clangd/fixits-embed-in-diagnostic.test

 # RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"publishDiagnostics":{"codeActionsInline":true}}},"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///foo.c","languageId":"c","version":1,"text":"struct Point {}; union Point p;"}}}
 #      CHECK:  "method": "textDocument/publishDiagnostics",
 # CHECK-NEXT:  "params": {
 # CHECK-NEXT:    "diagnostics": [
 # CHECK-NEXT:      {
-# CHECK-NEXT:        "code": "err_use_with_wrong_tag",
+# CHECK-NEXT:        "code": "use_with_wrong_tag",
 # CHECK-NEXT:        "codeActions": [
 # CHECK-NEXT:          {
 # CHECK-NEXT:            "edit": {
 # CHECK-NEXT:              "changes": {
 # CHECK-NEXT:                "file://{{.*}}/foo.c": [
 # CHECK-NEXT:                  {
 # CHECK-NEXT:                    "newText": "struct",
 # CHECK-NEXT:                    "range": {

unittests/clangd/DiagnosticsTests.cpp

o]]();
   )cpp");
   EXPECT_THAT(
       TestTU::withCode(Test.code()).build().getDiagnostics(),
       ElementsAre(
           // This range spans lines.
           AllOf(Diag(Test.range("typo"),
                      "use of undeclared identifier 'goo'; did you mean 'foo'?"),
                 DiagSource(Diag::Clang),
-                DiagName("err_undeclared_var_use_suggest"),
+                DiagName("undeclared_var_use_suggest"),
                 WithFix(
                     Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
                 // This is a pretty normal range.
                 WithNote(Diag(Test.range("decl"), "'foo' declared here"))),
           // This range is zero-width and insertion. Therefore make sure we are
           // not expanding it into other tokens. Since we are not going to
           // replace those.
           AllOf(Diag(Test.range("semicolon"), "expected ';' after expression"),
TEST(DiagnosticsTest, DiagnosticPreamble) {
   Annotations Test(R"cpp(
     #include $[["not-found.h"]]
   )cpp");
 
   auto TU = TestTU::withCode(Test.code());
   EXPECT_THAT(TU.build().getDiagnostics(),
               ElementsAre(testing::AllOf(
                   Diag(Test.range(), "'not-found.h' file not found"),
-                  DiagSource(Diag::Clang), DiagName("err_pp_file_not_found"))));
+                  DiagSource(Diag::Clang), DiagName("pp_file_not_found"))));
 }
 
 TEST(DiagnosticsTest, ClangTidy) {
   Annotations Test(R"cpp(
     #include $deprecated[["assert.h"]]
 
     #define $macrodef[[SQUARE]](X) (X)*(X)
     int main() {
TEST(DiagnosticsTest, NoFixItInMacro) {
   EXPECT_THAT(TU.build().getDiagnostics(),
               ElementsAre(AllOf(Diag(Test.range(), "'main' must return 'int'"),
                                 Not(WithFix(_)))));
 }
 
 TEST(DiagnosticsTest, ToLSP) {
   clangd::Diag D;
   D.ID = clang::diag::err_enum_class_reference;
-  D.Name = "err_enum_class_reference";
+  D.Name = "enum_class_reference";
   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";
 
   clangd::Note NoteInMain;
#endif
                    {std::move(LSPDiag),
                     std::vector<clangd::Fix>(Fixes.begin(), Fixes.end())});
              });
 
   EXPECT_THAT(
       LSPDiags,
       ElementsAre(Pair(EqualToLSPDiag(MainLSP), ElementsAre(EqualToFix(F))),
                   Pair(EqualToLSPDiag(NoteInMainLSP), IsEmpty())));
-  EXPECT_EQ(LSPDiags[0].first.code, "err_enum_class_reference");
+  EXPECT_EQ(LSPDiags[0].first.code, "enum_class_reference");
   EXPECT_EQ(LSPDiags[0].first.source, "clang");
   EXPECT_EQ(LSPDiags[1].first.code, "");
   EXPECT_EQ(LSPDiags[1].first.source, "");
 }
 
 struct SymbolWithHeader {
   std::string QName;
   std::string DeclaringFile;