kadircet created this revision.
kadircet added a reviewer: sammccall.
Herald added a subscriber: arphaman.
Herald added a project: All.
kadircet requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
This tries to improve adoption of noisy warnings in existing codebases.
Hints have a lot less visual clutter in most of the editors, and DiagnosticTags
already imply a custom decorations per LSP.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D154443
Files:
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -38,6 +38,7 @@
#include "gtest/gtest.h"
#include <cstddef>
#include <memory>
+#include <optional>
#include <string>
#include <utility>
#include <vector>
@@ -1879,6 +1880,42 @@
withTag(DiagnosticTag::Deprecated)))));
}
+TEST(Diagnostics, DeprecatedDiagsAreHints) {
+ ClangdDiagnosticOptions Opts;
+ std::optional<clangd::Diagnostic> Diag;
+ clangd::Diag D;
+ D.Range = {pos(1, 2), pos(3, 4)};
+ D.InsideMainFile = true;
+
+ // Downgrade warnings with deprecated tags to remark.
+ D.Tags = {Deprecated};
+ D.Severity = DiagnosticsEngine::Warning;
+ toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef<clangd::Fix>) {
+ Diag = std::move(LSPDiag);
+ });
+ EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Remark));
+ Diag.reset();
+
+ // Preserve errors.
+ D.Severity = DiagnosticsEngine::Error;
+ toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef<clangd::Fix>) {
+ Diag = std::move(LSPDiag);
+ });
+ EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Error));
+ Diag.reset();
+
+ // No-op without tag.
+ D.Tags = {};
+ D.Severity = DiagnosticsEngine::Warning;
+ toLSPDiags(D, {}, Opts,
+ [&](clangd::Diagnostic LSPDiag, ArrayRef<clangd::Fix>) {
+ Diag = std::move(LSPDiag);
+ });
+ EXPECT_EQ(Diag->severity, getSeverity(DiagnosticsEngine::Warning));
+}
+
TEST(DiagnosticsTest, IncludeCleaner) {
Annotations Test(R"cpp(
$fix[[ $diag[[#include "unused.h"]]
Index: clang-tools-extra/clangd/Diagnostics.cpp
===================================================================
--- clang-tools-extra/clangd/Diagnostics.cpp
+++ clang-tools-extra/clangd/Diagnostics.cpp
@@ -15,24 +15,35 @@
#include "clang/Basic/AllDiagnostics.h" // IWYU pragma: keep
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/DiagnosticIDs.h"
+#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
+#include "clang/Basic/TokenKinds.h"
#include "clang/Lex/Lexer.h"
#include "clang/Lex/Token.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/ScopeExit.h"
#include "llvm/ADT/SmallString.h"
#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringExtras.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
#include "llvm/ADT/Twine.h"
+#include "llvm/Support/ErrorHandling.h"
+#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
+#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/raw_ostream.h"
#include <algorithm>
#include <cassert>
-#include <cstddef>
#include <optional>
+#include <set>
+#include <string>
+#include <tuple>
+#include <utility>
#include <vector>
namespace clang {
@@ -460,6 +471,14 @@
llvm::function_ref<void(clangd::Diagnostic, llvm::ArrayRef<Fix>)> OutFn) {
clangd::Diagnostic Main;
Main.severity = getSeverity(D.Severity);
+ // We downgrade severity for certain noisy warnings, like deprecated
+ // declartions. These already have visible decorations inside the editor and
+ // most users find the extra clutter in the UI (gutter, minimap, diagnostics
+ // views) overwhelming.
+ if (D.Severity == DiagnosticsEngine::Warning) {
+ if (llvm::is_contained(D.Tags, DiagnosticTag::Deprecated))
+ Main.severity = getSeverity(DiagnosticsEngine::Remark);
+ }
// Main diagnostic should always refer to a range inside main file. If a
// diagnostic made it so for, it means either itself or one of its notes is
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
