[PATCH] D60819: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The check name is reported in Diagnostic.code.

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE358612: [clangd] Strip the  [some-check-name] 
suffix from clang-tidy diagnostics. The… (authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60819?vs=195548=195625#toc

Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60819/new/

https://reviews.llvm.org/D60819

Files:
  clangd/Diagnostics.cpp
  test/clangd/diagnostics.test
  unittests/clangd/DiagnosticsTests.cpp


Index: clangd/Diagnostics.cpp
===
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -350,6 +350,17 @@
   if (!TidyDiag.empty()) {
 Diag.Name = std::move(TidyDiag);
 Diag.Source = Diag::ClangTidy;
+// 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 ) {
+  StringRef Rest(Msg);
+  if (Rest.consume_back("]") && Rest.consume_back(Diag.Name) &&
+  Rest.consume_back(" ["))
+Msg.resize(Rest.size());
+};
+CleanMessage(Diag.Message);
+for (auto& Note : Diag.Notes)
+  CleanMessage(Note.Message);
 continue;
   }
 }
Index: test/clangd/diagnostics.test
===
--- test/clangd/diagnostics.test
+++ test/clangd/diagnostics.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  {
 # CHECK-NEXT:"code": "bugprone-sizeof-expression",
-# CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 
'K'? [bugprone-sizeof-expression]",
+# CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 
'K'?",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
 # CHECK-NEXT:"character": 12,
Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -176,23 +176,21 @@
   UnorderedElementsAre(
   AllOf(Diag(Test.range("deprecated"),
  "inclusion of deprecated C++ header 'assert.h'; consider "
- "using 'cassert' instead [modernize-deprecated-headers]"),
+ "using 'cassert' instead"),
 DiagSource(Diag::ClangTidy),
 DiagName("modernize-deprecated-headers"),
 WithFix(Fix(Test.range("deprecated"), "",
 "change '\"assert.h\"' to ''"))),
   Diag(Test.range("doubled"),
-   "suspicious usage of 'sizeof(sizeof(...))' "
-   "[bugprone-sizeof-expression]"),
+   "suspicious usage of 'sizeof(sizeof(...))'"),
   AllOf(
   Diag(Test.range("macroarg"),
"side effects in the 1st macro argument 'X' are repeated in 
"
-   "macro expansion [bugprone-macro-repeated-side-effects]"),
+   "macro expansion"),
   DiagSource(Diag::ClangTidy),
   DiagName("bugprone-macro-repeated-side-effects"),
-  WithNote(Diag(Test.range("macrodef"),
-"macro 'SQUARE' defined here "
-"[bugprone-macro-repeated-side-effects]"))),
+  WithNote(
+  Diag(Test.range("macrodef"), "macro 'SQUARE' defined 
here"))),
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'")));
 }


Index: clangd/Diagnostics.cpp
===
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -350,6 +350,17 @@
   if (!TidyDiag.empty()) {
 Diag.Name = std::move(TidyDiag);
 Diag.Source = Diag::ClangTidy;
+// 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 ) {
+  StringRef Rest(Msg);
+  if (Rest.consume_back("]") && Rest.consume_back(Diag.Name) &&
+  Rest.consume_back(" ["))
+Msg.resize(Rest.size());
+};
+CleanMessage(Diag.Message);
+for (auto& Note : Diag.Notes)
+  CleanMessage(Note.Message);
 continue;
   }
 }
Index: test/clangd/diagnostics.test
===
--- test/clangd/diagnostics.test
+++ test/clangd/diagnostics.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  {
 # CHECK-NEXT:"code": "bugprone-sizeof-expression",
-# CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 'K'? [bugprone-sizeof-expression]",
+# CHECK-NEXT:"message": "Suspicious usage of 

[PATCH] D60819: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The check name is reported in Diagnostic.code.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

LGTM, we might need to make sure clients that do not show "code" field become 
aware of this before LLVM9 releases


Repository:
  rCTE Clang Tools Extra

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60819/new/

https://reviews.llvm.org/D60819



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60819: [clangd] Strip the ' [some-check-name]' suffix from clang-tidy diagnostics. The check name is reported in Diagnostic.code.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: cfe-commits, arphaman, jkorous, MaskRay, ioeric, 
ilya-biryukov.
Herald added a project: clang.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D60819

Files:
  clangd/Diagnostics.cpp
  test/clangd/diagnostics.test
  unittests/clangd/DiagnosticsTests.cpp


Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -176,23 +176,21 @@
   UnorderedElementsAre(
   AllOf(Diag(Test.range("deprecated"),
  "inclusion of deprecated C++ header 'assert.h'; consider "
- "using 'cassert' instead [modernize-deprecated-headers]"),
+ "using 'cassert' instead"),
 DiagSource(Diag::ClangTidy),
 DiagName("modernize-deprecated-headers"),
 WithFix(Fix(Test.range("deprecated"), "",
 "change '\"assert.h\"' to ''"))),
   Diag(Test.range("doubled"),
-   "suspicious usage of 'sizeof(sizeof(...))' "
-   "[bugprone-sizeof-expression]"),
+   "suspicious usage of 'sizeof(sizeof(...))'"),
   AllOf(
   Diag(Test.range("macroarg"),
"side effects in the 1st macro argument 'X' are repeated in 
"
-   "macro expansion [bugprone-macro-repeated-side-effects]"),
+   "macro expansion"),
   DiagSource(Diag::ClangTidy),
   DiagName("bugprone-macro-repeated-side-effects"),
-  WithNote(Diag(Test.range("macrodef"),
-"macro 'SQUARE' defined here "
-"[bugprone-macro-repeated-side-effects]"))),
+  WithNote(
+  Diag(Test.range("macrodef"), "macro 'SQUARE' defined 
here"))),
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'")));
 }
Index: test/clangd/diagnostics.test
===
--- test/clangd/diagnostics.test
+++ test/clangd/diagnostics.test
@@ -23,7 +23,7 @@
 # CHECK-NEXT:  },
 # CHECK-NEXT:  {
 # CHECK-NEXT:"code": "bugprone-sizeof-expression",
-# CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 
'K'? [bugprone-sizeof-expression]",
+# CHECK-NEXT:"message": "Suspicious usage of 'sizeof(K)'; did you mean 
'K'?",
 # CHECK-NEXT:"range": {
 # CHECK-NEXT:  "end": {
 # CHECK-NEXT:"character": 12,
Index: clangd/Diagnostics.cpp
===
--- clangd/Diagnostics.cpp
+++ clangd/Diagnostics.cpp
@@ -340,6 +340,17 @@
   if (!TidyDiag.empty()) {
 Diag.Name = std::move(TidyDiag);
 Diag.Source = Diag::ClangTidy;
+// 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 ) {
+  StringRef Rest(Msg);
+  if (Rest.consume_back("]") && Rest.consume_back(Diag.Name) &&
+  Rest.consume_back(" ["))
+Msg.resize(Rest.size());
+};
+CleanMessage(Diag.Message);
+for (auto& Note : Diag.Notes)
+  CleanMessage(Note.Message);
 continue;
   }
 }


Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -176,23 +176,21 @@
   UnorderedElementsAre(
   AllOf(Diag(Test.range("deprecated"),
  "inclusion of deprecated C++ header 'assert.h'; consider "
- "using 'cassert' instead [modernize-deprecated-headers]"),
+ "using 'cassert' instead"),
 DiagSource(Diag::ClangTidy),
 DiagName("modernize-deprecated-headers"),
 WithFix(Fix(Test.range("deprecated"), "",
 "change '\"assert.h\"' to ''"))),
   Diag(Test.range("doubled"),
-   "suspicious usage of 'sizeof(sizeof(...))' "
-   "[bugprone-sizeof-expression]"),
+   "suspicious usage of 'sizeof(sizeof(...))'"),
   AllOf(
   Diag(Test.range("macroarg"),
"side effects in the 1st macro argument 'X' are repeated in "
-   "macro expansion [bugprone-macro-repeated-side-effects]"),
+   "macro expansion"),
   DiagSource(Diag::ClangTidy),
   DiagName("bugprone-macro-repeated-side-effects"),
-  WithNote(Diag(Test.range("macrodef"),
-"macro 'SQUARE' defined here "
-