[clang-tools-extra] r358611 - [clangd] Use shorter, more recognizable codes for diagnostics.

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Wed Apr 17 13:12:03 PDT 2019


Author: sammccall
Date: Wed Apr 17 13:12:03 2019
New Revision: 358611

URL: http://llvm.org/viewvc/llvm-project?rev=358611&view=rev
Log:
[clangd] Use shorter, more recognizable codes for diagnostics.

Summary:
 - for warnings, use the flag 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.

Reviewers: kadircet

Subscribers: ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D60822

Modified:
    clang-tools-extra/trunk/clangd/Diagnostics.cpp
    clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test
    clang-tools-extra/trunk/test/clangd/diagnostic-category.test
    clang-tools-extra/trunk/test/clangd/diagnostics.test
    clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test
    clang-tools-extra/trunk/test/clangd/execute-command.test
    clang-tools-extra/trunk/test/clangd/fixits-codeaction.test
    clang-tools-extra/trunk/test/clangd/fixits-command.test
    clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test
    clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp

Modified: clang-tools-extra/trunk/clangd/Diagnostics.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Diagnostics.cpp?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/Diagnostics.cpp (original)
+++ clang-tools-extra/trunk/clangd/Diagnostics.cpp Wed Apr 17 13:12:03 2019
@@ -331,7 +331,17 @@ std::vector<Diag> StoreDiags::take(const
   // 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;
     }

Modified: clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test (original)
+++ clang-tools-extra/trunk/test/clangd/compile-commands-path-in-initialize.test Wed Apr 17 13:12:03 2019
@@ -21,7 +21,7 @@
 # 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"}

Modified: clang-tools-extra/trunk/test/clangd/diagnostic-category.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/diagnostic-category.test?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/diagnostic-category.test (original)
+++ clang-tools-extra/trunk/test/clangd/diagnostic-category.test Wed Apr 17 13:12:03 2019
@@ -7,7 +7,7 @@
 # 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": {

Modified: clang-tools-extra/trunk/test/clangd/diagnostics.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/diagnostics.test?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/diagnostics.test (original)
+++ clang-tools-extra/trunk/test/clangd/diagnostics.test Wed Apr 17 13:12:03 2019
@@ -6,7 +6,7 @@
 # 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": {

Modified: clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test (original)
+++ clang-tools-extra/trunk/test/clangd/did-change-configuration-params.test Wed Apr 17 13:12:03 2019
@@ -24,7 +24,7 @@
 # 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": {

Modified: clang-tools-extra/trunk/test/clangd/execute-command.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/execute-command.test?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/execute-command.test (original)
+++ clang-tools-extra/trunk/test/clangd/execute-command.test Wed Apr 17 13:12:03 2019
@@ -6,7 +6,7 @@
 # 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": {

Modified: clang-tools-extra/trunk/test/clangd/fixits-codeaction.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/fixits-codeaction.test?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/fixits-codeaction.test (original)
+++ clang-tools-extra/trunk/test/clangd/fixits-codeaction.test Wed Apr 17 13:12:03 2019
@@ -6,7 +6,7 @@
 # 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": {
@@ -25,14 +25,14 @@
 # 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": {
@@ -86,7 +86,7 @@
 # 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": {

Modified: clang-tools-extra/trunk/test/clangd/fixits-command.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/fixits-command.test?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/fixits-command.test (original)
+++ clang-tools-extra/trunk/test/clangd/fixits-command.test Wed Apr 17 13:12:03 2019
@@ -6,7 +6,7 @@
 # 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": {

Modified: clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test (original)
+++ clang-tools-extra/trunk/test/clangd/fixits-embed-in-diagnostic.test Wed Apr 17 13:12:03 2019
@@ -6,7 +6,7 @@
 # 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": {

Modified: clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp?rev=358611&r1=358610&r2=358611&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DiagnosticsTests.cpp Wed Apr 17 13:12:03 2019
@@ -107,7 +107,7 @@ o]]();
           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.
@@ -152,7 +152,7 @@ TEST(DiagnosticsTest, DiagnosticPreamble
   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) {
@@ -252,7 +252,7 @@ TEST(DiagnosticsTest, NoFixItInMacro) {
 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)};
@@ -322,7 +322,7 @@ main.cpp:2:3: error: something terrible
       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, "");




More information about the cfe-commits mailing list