[clangd] Include textual diagnostic ID as Diagnostic.code.

Reviewers: kadircet

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

Tags: #clang

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

llvm-svn: 358575
This commit is contained in:
Sam McCall 2019-04-17 12:35:16 +00:00
parent 9daacec816
commit 641caa57cc
14 changed files with 137 additions and 33 deletions

View File

@ -371,11 +371,7 @@ ParsedAST::build(std::unique_ptr<CompilerInvocation> CI,
// So just inform the preprocessor of EOF, while keeping everything alive.
Clang->getPreprocessor().EndSourceFile();
std::vector<Diag> Diags = ASTDiags.take();
// Populate diagnostic source.
for (auto &D : Diags)
D.S =
!CTContext->getCheckName(D.ID).empty() ? Diag::ClangTidy : Diag::Clang;
std::vector<Diag> Diags = ASTDiags.take(CTContext.getPointer());
// Add diagnostics from the preamble, if any.
if (Preamble)
Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
@ -544,8 +540,6 @@ buildPreamble(PathRef FileName, CompilerInvocation &CI,
vlog("Built preamble of size {0} for file {1}", BuiltPreamble->getSize(),
FileName);
std::vector<Diag> Diags = PreambleDiagnostics.take();
for (auto &Diag : Diags)
Diag.S = Diag::Clang;
return std::make_shared<PreambleData>(
std::move(*BuiltPreamble), std::move(Diags),
SerializedDeclsCollector.takeIncludes(), std::move(StatCache),

View File

@ -7,9 +7,12 @@
//===----------------------------------------------------------------------===//
#include "Diagnostics.h"
#include "../clang-tidy/ClangTidyDiagnosticConsumer.h"
#include "Compiler.h"
#include "Logger.h"
#include "SourceCode.h"
#include "clang/Basic/AllDiagnostics.h"
#include "clang/Basic/DiagnosticIDs.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/Lexer.h"
#include "llvm/Support/Capacity.h"
@ -18,9 +21,31 @@
namespace clang {
namespace clangd {
namespace {
const char *getDiagnosticCode(unsigned ID) {
switch (ID) {
#define DIAG(ENUM, CLASS, DEFAULT_MAPPING, DESC, GROPU, SFINAE, NOWERROR, \
SHOWINSYSHEADER, CATEGORY) \
case clang::diag::ENUM: \
return #ENUM;
#include "clang/Basic/DiagnosticASTKinds.inc"
#include "clang/Basic/DiagnosticAnalysisKinds.inc"
#include "clang/Basic/DiagnosticCommentKinds.inc"
#include "clang/Basic/DiagnosticCommonKinds.inc"
#include "clang/Basic/DiagnosticDriverKinds.inc"
#include "clang/Basic/DiagnosticFrontendKinds.inc"
#include "clang/Basic/DiagnosticLexKinds.inc"
#include "clang/Basic/DiagnosticParseKinds.inc"
#include "clang/Basic/DiagnosticRefactoringKinds.inc"
#include "clang/Basic/DiagnosticSemaKinds.inc"
#include "clang/Basic/DiagnosticSerializationKinds.inc"
#undef DIAG
default:
return nullptr;
}
}
bool mentionsMainFile(const Diag &D) {
if (D.InsideMainFile)
return true;
@ -253,6 +278,18 @@ void toLSPDiags(
{
clangd::Diagnostic Main = FillBasicFields(D);
Main.message = mainMessage(D, Opts.DisplayFixesCount);
if (!D.Name.empty())
Main.code = D.Name;
switch (D.Source) {
case Diag::Clang:
Main.source = "clang";
break;
case Diag::ClangTidy:
Main.source = "clang-tidy";
break;
case Diag::Unknown:
break;
}
if (Opts.EmbedFixesInDiagnostics) {
Main.codeActions.emplace();
for (const auto &Fix : D.Fixes)
@ -290,7 +327,25 @@ int getSeverity(DiagnosticsEngine::Level L) {
llvm_unreachable("Unknown diagnostic level!");
}
std::vector<Diag> StoreDiags::take() { return std::move(Output); }
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;
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;
continue;
}
}
}
return std::move(Output);
}
void StoreDiags::BeginSourceFile(const LangOptions &Opts,
const Preprocessor *) {

View File

@ -20,6 +20,9 @@
#include <string>
namespace clang {
namespace tidy {
class ClangTidyContext;
} // namespace tidy
namespace clangd {
struct ClangdDiagnosticOptions {
@ -68,14 +71,14 @@ struct Note : DiagBase {};
/// A top-level diagnostic that may have Notes and Fixes.
struct Diag : DiagBase {
// Diagnostic enum ID.
unsigned ID;
unsigned ID; // e.g. member of clang::diag, or clang-tidy assigned ID.
std::string Name; // if ID was recognized.
// The source of this diagnostic.
enum Source {
enum {
Unknown,
Clang,
ClangTidy,
};
Source S = Clang;
} Source = Unknown;
/// Elaborate on the problem, usually pointing to a related piece of code.
std::vector<Note> Notes;
/// *Alternative* fixes for this diagnostic, one should be chosen.
@ -104,7 +107,8 @@ int getSeverity(DiagnosticsEngine::Level L);
/// the diag itself nor its notes are in the main file).
class StoreDiags : public DiagnosticConsumer {
public:
std::vector<Diag> take();
// The ClangTidyContext populates Source and Name for clang-tidy diagnostics.
std::vector<Diag> take(const clang::tidy::ClangTidyContext *Tidy = nullptr);
void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override;
void EndSourceFile() override;

View File

@ -429,6 +429,10 @@ llvm::json::Value toJSON(const Diagnostic &D) {
Diag["category"] = *D.category;
if (D.codeActions)
Diag["codeActions"] = D.codeActions;
if (!D.code.empty())
Diag["code"] = D.code;
if (!D.source.empty())
Diag["source"] = D.source;
return std::move(Diag);
}
@ -438,6 +442,8 @@ bool fromJSON(const llvm::json::Value &Params, Diagnostic &R) {
return false;
O.map("severity", R.severity);
O.map("category", R.category);
O.map("code", R.code);
O.map("source", R.source);
return true;
}

View File

@ -592,13 +592,11 @@ struct Diagnostic {
int severity = 0;
/// The diagnostic's code. Can be omitted.
/// Note: Not currently used by clangd
// std::string code;
std::string code;
/// A human-readable string describing the source of this
/// diagnostic, e.g. 'typescript' or 'super lint'.
/// Note: Not currently used by clangd
// std::string source;
std::string source;
/// The diagnostic's message.
std::string message;

View File

@ -21,6 +21,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "code": "warn_pragma_message",
# CHECK-NEXT: "message": "MACRO is one",
---
{"jsonrpc":"2.0","id":10000,"method":"shutdown"}

View File

@ -7,6 +7,7 @@
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "category": "Semantic Issue",
# CHECK-NEXT: "code": "err_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": {
@ -18,7 +19,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 1
# CHECK-NEXT: "severity": 1,
# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: },
# CHECK-NEXT: {
# CHECK-NEXT: "message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",

View File

@ -1,11 +1,12 @@
# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
# 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() {}"}}}
{"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: "message": "Return type of 'main' is not 'int' (fix available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@ -17,7 +18,24 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 2
# CHECK-NEXT: "severity": 2,
# CHECK-NEXT: "source": "clang"
# 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: "range": {
# CHECK-NEXT: "end": {
# CHECK-NEXT: "character": 12,
# CHECK-NEXT: "line": 1
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
# CHECK-NEXT: "character": 6,
# CHECK-NEXT: "line": 1
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 2,
# CHECK-NEXT: "source": "clang-tidy"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"

View File

@ -24,6 +24,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "code": "warn_uninit_var",
# CHECK-NEXT: "message": "Variable 'i' is uninitialized when used here (fix available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@ -35,7 +36,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 1
# CHECK-NEXT: "severity": 1,
# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"

View File

@ -6,6 +6,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@ -17,7 +18,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 2
# CHECK-NEXT: "severity": 2,
# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"

View File

@ -6,6 +6,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@ -17,19 +18,21 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 2
# 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)"}]}}}
{"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"}]}}}
# 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: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@ -41,7 +44,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 2
# CHECK-NEXT: "severity": 2,
# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "edit": {
@ -82,6 +86,7 @@
# CHECK-NEXT: {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@ -93,7 +98,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 2
# CHECK-NEXT: "severity": 2,
# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "edit": {

View File

@ -6,6 +6,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "code": "warn_condition_is_assignment",
# CHECK-NEXT: "message": "Using the result of an assignment as a condition without parentheses (fixes available)",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
@ -17,7 +18,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 2
# CHECK-NEXT: "severity": 2,
# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: }
# CHECK-NEXT: ],
# CHECK-NEXT: "uri": "file://{{.*}}/foo.c"

View File

@ -6,6 +6,7 @@
# CHECK-NEXT: "params": {
# CHECK-NEXT: "diagnostics": [
# CHECK-NEXT: {
# CHECK-NEXT: "code": "err_use_with_wrong_tag",
# CHECK-NEXT: "codeActions": [
# CHECK-NEXT: {
# CHECK-NEXT: "edit": {
@ -42,7 +43,8 @@
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
# CHECK-NEXT: "severity": 1
# CHECK-NEXT: "severity": 1,
# CHECK-NEXT: "source": "clang"
# CHECK-NEXT: },
# CHECK-NEXT: {
# CHECK-NEXT: "message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",

View File

@ -12,6 +12,7 @@
#include "TestIndex.h"
#include "TestTU.h"
#include "index/MemIndex.h"
#include "clang/Basic/DiagnosticSema.h"
#include "llvm/Support/ScopedPrinter.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@ -58,7 +59,8 @@ MATCHER_P(EqualToLSPDiag, LSPDiag,
std::tie(LSPDiag.range, LSPDiag.severity, LSPDiag.message);
}
MATCHER_P(DiagSource, Source, "") { return arg.S == Source; }
MATCHER_P(DiagSource, S, "") { return arg.Source == S; }
MATCHER_P(DiagName, N, "") { return arg.Name == N; }
MATCHER_P(EqualToFix, Fix, "LSP fix " + llvm::to_string(Fix)) {
if (arg.Message != Fix.Message)
@ -105,6 +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"),
WithFix(
Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
// This is a pretty normal range.
@ -149,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))));
DiagSource(Diag::Clang), DiagName("err_pp_file_not_found"))));
}
TEST(DiagnosticsTest, ClangTidy) {
@ -175,6 +178,7 @@ TEST(DiagnosticsTest, ClangTidy) {
"inclusion of deprecated C++ header 'assert.h'; consider "
"using 'cassert' instead [modernize-deprecated-headers]"),
DiagSource(Diag::ClangTidy),
DiagName("modernize-deprecated-headers"),
WithFix(Fix(Test.range("deprecated"), "<cassert>",
"change '\"assert.h\"' to '<cassert>'"))),
Diag(Test.range("doubled"),
@ -185,6 +189,7 @@ TEST(DiagnosticsTest, ClangTidy) {
"side effects in the 1st macro argument 'X' are repeated in "
"macro expansion [bugprone-macro-repeated-side-effects]"),
DiagSource(Diag::ClangTidy),
DiagName("bugprone-macro-repeated-side-effects"),
WithNote(Diag(Test.range("macrodef"),
"macro 'SQUARE' defined here "
"[bugprone-macro-repeated-side-effects]"))),
@ -246,6 +251,9 @@ TEST(DiagnosticsTest, NoFixItInMacro) {
TEST(DiagnosticsTest, ToLSP) {
clangd::Diag D;
D.ID = clang::diag::err_enum_class_reference;
D.Name = "err_enum_class_reference";
D.Source = clangd::Diag::Clang;
D.Message = "something terrible happened";
D.Range = {pos(1, 2), pos(3, 4)};
D.InsideMainFile = true;
@ -314,6 +322,10 @@ 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.source, "clang");
EXPECT_EQ(LSPDiags[1].first.code, "");
EXPECT_EQ(LSPDiags[1].first.source, "");
}
struct SymbolWithHeader {