From d98170c324b193ebe44cbedaa53748fc3beea91d Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Wed, 17 Apr 2019 20:12:03 +0000 Subject: [PATCH] [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 llvm-svn: 358611 --- clang-tools-extra/clangd/Diagnostics.cpp | 12 +++++++++++- .../test/clangd/compile-commands-path-in-initialize.test | 2 +- clang-tools-extra/test/clangd/diagnostic-category.test | 2 +- clang-tools-extra/test/clangd/diagnostics.test | 2 +- .../test/clangd/did-change-configuration-params.test | 2 +- clang-tools-extra/test/clangd/execute-command.test | 2 +- clang-tools-extra/test/clangd/fixits-codeaction.test | 8 ++++---- clang-tools-extra/test/clangd/fixits-command.test | 2 +- .../test/clangd/fixits-embed-in-diagnostic.test | 2 +- clang-tools-extra/unittests/clangd/DiagnosticsTests.cpp | 8 ++++---- 10 files changed, 26 insertions(+), 16 deletions(-) diff --git a/clang-tools-extra/clangd/Diagnostics.cpp b/clang-tools-extra/clangd/Diagnostics.cpp index 8a6ceba..c46cac3 100644 --- a/clang-tools-extra/clangd/Diagnostics.cpp +++ b/clang-tools-extra/clangd/Diagnostics.cpp @@ -331,7 +331,17 @@ std::vector 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; } diff --git a/clang-tools-extra/test/clangd/compile-commands-path-in-initialize.test b/clang-tools-extra/test/clangd/compile-commands-path-in-initialize.test index e070562..2905bad 100644 --- a/clang-tools-extra/test/clangd/compile-commands-path-in-initialize.test +++ b/clang-tools-extra/test/clangd/compile-commands-path-in-initialize.test @@ -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"} diff --git a/clang-tools-extra/test/clangd/diagnostic-category.test b/clang-tools-extra/test/clangd/diagnostic-category.test index a6f224d..3946774 100644 --- a/clang-tools-extra/test/clangd/diagnostic-category.test +++ b/clang-tools-extra/test/clangd/diagnostic-category.test @@ -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": { diff --git a/clang-tools-extra/test/clangd/diagnostics.test b/clang-tools-extra/test/clangd/diagnostics.test index a43198b..963affc 100644 --- a/clang-tools-extra/test/clangd/diagnostics.test +++ b/clang-tools-extra/test/clangd/diagnostics.test @@ -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": { diff --git a/clang-tools-extra/test/clangd/did-change-configuration-params.test b/clang-tools-extra/test/clangd/did-change-configuration-params.test index 35dcb55..bd8ffaf 100644 --- a/clang-tools-extra/test/clangd/did-change-configuration-params.test +++ b/clang-tools-extra/test/clangd/did-change-configuration-params.test @@ -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": { diff --git a/clang-tools-extra/test/clangd/execute-command.test b/clang-tools-extra/test/clangd/execute-command.test index db7e2aa..7abd79e 100644 --- a/clang-tools-extra/test/clangd/execute-command.test +++ b/clang-tools-extra/test/clangd/execute-command.test @@ -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": { diff --git a/clang-tools-extra/test/clangd/fixits-codeaction.test b/clang-tools-extra/test/clangd/fixits-codeaction.test index 6876f4d..e919071 100644 --- a/clang-tools-extra/test/clangd/fixits-codeaction.test +++ b/clang-tools-extra/test/clangd/fixits-codeaction.test @@ -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": { diff --git a/clang-tools-extra/test/clangd/fixits-command.test b/clang-tools-extra/test/clangd/fixits-command.test index c24275f..9d43e70 100644 --- a/clang-tools-extra/test/clangd/fixits-command.test +++ b/clang-tools-extra/test/clangd/fixits-command.test @@ -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": { diff --git a/clang-tools-extra/test/clangd/fixits-embed-in-diagnostic.test b/clang-tools-extra/test/clangd/fixits-embed-in-diagnostic.test index 304f96e..7d2cccd 100644 --- a/clang-tools-extra/test/clangd/fixits-embed-in-diagnostic.test +++ b/clang-tools-extra/test/clangd/fixits-embed-in-diagnostic.test @@ -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": { diff --git a/clang-tools-extra/unittests/clangd/DiagnosticsTests.cpp b/clang-tools-extra/unittests/clangd/DiagnosticsTests.cpp index f406ec4..3228a32 100644 --- a/clang-tools-extra/unittests/clangd/DiagnosticsTests.cpp +++ b/clang-tools-extra/unittests/clangd/DiagnosticsTests.cpp @@ -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 happened)"); 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, ""); -- 2.7.4