Skip to content

Commit

Permalink
[clangd] Use shorter, more recognizable codes for diagnostics.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@sam-mccall
sam-mccall committed Apr 17, 2019
1 parent 09ef420 commit d98170c
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 16 deletions.
12 changes: 11 additions & 1 deletion clang-tools-extra/clangd/Diagnostics.cpp
View file
Expand Up @@ -331,7 +331,17 @@ 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;
}
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/test/clangd/compile-commands-path-in-initialize.test
View file
Expand Up @@ -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"}
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/test/clangd/diagnostic-category.test
View file
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/test/clangd/diagnostics.test
View file
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/test/clangd/did-change-configuration-params.test
View file
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/test/clangd/execute-command.test
View file
Expand Up @@ -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": {
Expand Down
8 changes: 4 additions & 4 deletions clang-tools-extra/test/clangd/fixits-codeaction.test
View file
Expand Up @@ -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": {
Expand All @@ -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": {
Expand Down Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/test/clangd/fixits-command.test
View file
Expand Up @@ -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": {
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/test/clangd/fixits-embed-in-diagnostic.test
View file
Expand Up @@ -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": {
Expand Down
8 changes: 4 additions & 4 deletions clang-tools-extra/unittests/clangd/DiagnosticsTests.cpp
View file
Expand Up @@ -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.
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)};
Expand Down Expand Up @@ -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, "");
Expand Down

0 comments on commit d98170c

Please sign in to comment.