Skip to content

Commit

Permalink
[clangd] Make use of diagnostic tags for some clang diags
Browse files Browse the repository at this point in the history
It is not great to list diag ids by hand, but I don't see any other
solution unless diagnostics are annotated with these explicitly, which is a
bigger change in clang and I am not sure if would be worth it.

Diagnostics handled by this patch is by no means exhaustive, there might be
other checks that don't mention "unused"/"deprecated" in their names. But it
feels like this should be enough to catch common diagnostics and can be extended
over time.

Differential Revision: https://reviews.llvm.org/D107040
  • Loading branch information
kadircet committed Jul 30, 2021
1 parent 84705ed commit 5734652
Show file tree
Hide file tree
Showing 5 changed files with 107 additions and 1 deletion.
60 changes: 59 additions & 1 deletion clang-tools-extra/clangd/Diagnostics.cpp
Expand Up @@ -26,6 +26,7 @@
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/Capacity.h"
Expand All @@ -35,6 +36,7 @@
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <cstddef>
#include <vector>

namespace clang {
namespace clangd {
Expand Down Expand Up @@ -324,6 +326,60 @@ std::string noteMessage(const Diag &Main, const DiagBase &Note,
OS.flush();
return capitalize(std::move(Result));
}

void setTags(clangd::Diag &D) {
static const auto *DeprecatedDiags = new llvm::DenseSet<unsigned>{
diag::warn_access_decl_deprecated,
diag::warn_atl_uuid_deprecated,
diag::warn_deprecated,
diag::warn_deprecated_altivec_src_compat,
diag::warn_deprecated_comma_subscript,
diag::warn_deprecated_compound_assign_volatile,
diag::warn_deprecated_copy,
diag::warn_deprecated_copy_with_dtor,
diag::warn_deprecated_copy_with_user_provided_copy,
diag::warn_deprecated_copy_with_user_provided_dtor,
diag::warn_deprecated_def,
diag::warn_deprecated_increment_decrement_volatile,
diag::warn_deprecated_message,
diag::warn_deprecated_redundant_constexpr_static_def,
diag::warn_deprecated_register,
diag::warn_deprecated_simple_assign_volatile,
diag::warn_deprecated_string_literal_conversion,
diag::warn_deprecated_this_capture,
diag::warn_deprecated_volatile_param,
diag::warn_deprecated_volatile_return,
diag::warn_deprecated_volatile_structured_binding,
diag::warn_opencl_attr_deprecated_ignored,
diag::warn_property_method_deprecated,
diag::warn_vector_mode_deprecated,
};
static const auto *UnusedDiags = new llvm::DenseSet<unsigned>{
diag::warn_opencl_attr_deprecated_ignored,
diag::warn_pragma_attribute_unused,
diag::warn_unused_but_set_parameter,
diag::warn_unused_but_set_variable,
diag::warn_unused_comparison,
diag::warn_unused_const_variable,
diag::warn_unused_exception_param,
diag::warn_unused_function,
diag::warn_unused_label,
diag::warn_unused_lambda_capture,
diag::warn_unused_local_typedef,
diag::warn_unused_member_function,
diag::warn_unused_parameter,
diag::warn_unused_private_field,
diag::warn_unused_property_backing_ivar,
diag::warn_unused_template,
diag::warn_unused_variable,
};
if (DeprecatedDiags->contains(D.ID)) {
D.Tags.push_back(DiagnosticTag::Deprecated);
} else if (UnusedDiags->contains(D.ID)) {
D.Tags.push_back(DiagnosticTag::Unnecessary);
}
// FIXME: Set tags for tidy-based diagnostics too.
}
} // namespace

llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const DiagBase &D) {
Expand Down Expand Up @@ -461,6 +517,7 @@ void toLSPDiags(
Main.relatedInformation->push_back(std::move(RelInfo));
}
}
Main.tags = D.Tags;
OutFn(std::move(Main), D.Fixes);

// If we didn't emit the notes as relatedLocations, emit separate diagnostics
Expand Down Expand Up @@ -504,6 +561,7 @@ 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) {
setTags(Diag);
if (const char *ClangDiag = getDiagnosticCode(Diag.ID)) {
// Warnings controlled by -Wfoo are better recognized by that name.
StringRef Warning = DiagnosticIDs::getWarningOptionForDiag(Diag.ID);
Expand Down Expand Up @@ -545,7 +603,7 @@ std::vector<Diag> StoreDiags::take(const clang::tidy::ClangTidyContext *Tidy) {
// duplicated messages due to various reasons (e.g. the check doesn't handle
// template instantiations well; clang-tidy alias checks).
std::set<std::pair<Range, std::string>> SeenDiags;
llvm::erase_if(Output, [&](const Diag& D) {
llvm::erase_if(Output, [&](const Diag &D) {
return !SeenDiags.emplace(D.Range, D.Message).second;
});
return std::move(Output);
Expand Down
2 changes: 2 additions & 0 deletions clang-tools-extra/clangd/Diagnostics.h
Expand Up @@ -19,6 +19,7 @@
#include "llvm/ADT/None.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/JSON.h"
#include "llvm/Support/SourceMgr.h"
Expand Down Expand Up @@ -106,6 +107,7 @@ struct Diag : DiagBase {
std::vector<Note> Notes;
/// *Alternative* fixes for this diagnostic, one should be chosen.
std::vector<Fix> Fixes;
llvm::SmallVector<DiagnosticTag, 1> Tags;
};
llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const Diag &D);

Expand Down
4 changes: 4 additions & 0 deletions clang-tools-extra/clangd/Protocol.cpp
Expand Up @@ -584,6 +584,8 @@ llvm::json::Value toJSON(const DiagnosticRelatedInformation &DRI) {
};
}

llvm::json::Value toJSON(DiagnosticTag Tag) { return static_cast<int>(Tag); }

llvm::json::Value toJSON(const Diagnostic &D) {
llvm::json::Object Diag{
{"range", D.range},
Expand All @@ -602,6 +604,8 @@ llvm::json::Value toJSON(const Diagnostic &D) {
Diag["relatedInformation"] = *D.relatedInformation;
if (!D.data.empty())
Diag["data"] = llvm::json::Object(D.data);
if (!D.tags.empty())
Diag["tags"] = llvm::json::Array{D.tags};
// FIXME: workaround for older gcc/clang
return std::move(Diag);
}
Expand Down
17 changes: 17 additions & 0 deletions clang-tools-extra/clangd/Protocol.h
Expand Up @@ -28,6 +28,7 @@
#include "support/MemoryTree.h"
#include "clang/Index/IndexSymbol.h"
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/JSON.h"
#include "llvm/Support/raw_ostream.h"
#include <bitset>
Expand Down Expand Up @@ -811,6 +812,19 @@ struct DiagnosticRelatedInformation {
};
llvm::json::Value toJSON(const DiagnosticRelatedInformation &);

enum DiagnosticTag {
/// Unused or unnecessary code.
///
/// Clients are allowed to render diagnostics with this tag faded out instead
/// of having an error squiggle.
Unnecessary = 1,
/// Deprecated or obsolete code.
///
/// Clients are allowed to rendered diagnostics with this tag strike through.
Deprecated = 2,
};
llvm::json::Value toJSON(DiagnosticTag Tag);

struct CodeAction;
struct Diagnostic {
/// The range at which the message applies.
Expand All @@ -830,6 +844,9 @@ struct Diagnostic {
/// The diagnostic's message.
std::string message;

/// Additional metadata about the diagnostic.
llvm::SmallVector<DiagnosticTag, 1> tags;

/// An array of related diagnostic information, e.g. when symbol-names within
/// a scope collide all definitions can be marked via this property.
llvm::Optional<std::vector<DiagnosticRelatedInformation>> relatedInformation;
Expand Down
25 changes: 25 additions & 0 deletions clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
Expand Up @@ -65,6 +65,11 @@ WithNote(::testing::Matcher<Note> NoteMatcher1,
return Field(&Diag::Notes, UnorderedElementsAre(NoteMatcher1, NoteMatcher2));
}

::testing::Matcher<const Diag &>
WithTag(::testing::Matcher<DiagnosticTag> TagMatcher) {
return Field(&Diag::Tags, Contains(TagMatcher));
}

MATCHER_P2(Diag, Range, Message,
"Diag at " + llvm::to_string(Range) + " = [" + Message + "]") {
return arg.Range == Range && arg.Message == Message;
Expand Down Expand Up @@ -665,6 +670,7 @@ TEST(DiagnosticsTest, ToLSP) {

clangd::Diag D;
D.ID = clang::diag::err_undeclared_var_use;
D.Tags = {DiagnosticTag::Unnecessary};
D.Name = "undeclared_var_use";
D.Source = clangd::Diag::Clang;
D.Message = "something terrible happened";
Expand Down Expand Up @@ -710,6 +716,7 @@ main.cpp:6:7: remark: declared somewhere in the main file
../foo/baz/header.h:10:11:
note: declared somewhere in the header file)";
MainLSP.tags = {DiagnosticTag::Unnecessary};

clangd::Diagnostic NoteInMainLSP;
NoteInMainLSP.range = NoteInMain.Range;
Expand Down Expand Up @@ -1419,6 +1426,24 @@ TEST(Preamble, EndsOnNonEmptyLine) {
testing::Contains(Diag(Code.range(), "no newline at end of file")));
}
}

TEST(Diagnostics, Tags) {
TestTU TU;
TU.ExtraArgs = {"-Wunused", "-Wdeprecated"};
Annotations Test(R"cpp(
void bar() __attribute__((deprecated));
void foo() {
int $unused[[x]];
$deprecated[[bar]]();
})cpp");
TU.Code = Test.code().str();
EXPECT_THAT(*TU.build().getDiagnostics(),
UnorderedElementsAre(
AllOf(Diag(Test.range("unused"), "unused variable 'x'"),
WithTag(DiagnosticTag::Unnecessary)),
AllOf(Diag(Test.range("deprecated"), "'bar' is deprecated"),
WithTag(DiagnosticTag::Deprecated))));
}
} // namespace
} // namespace clangd
} // namespace clang

0 comments on commit 5734652

Please sign in to comment.