[clangd] Downgrade deprecated warnings to hints
authorKadir Cetinkaya <kadircet@google.com>
Tue, 4 Jul 2023 13:50:47 +0000 (15:50 +0200)
committerKadir Cetinkaya <kadircet@google.com>
Tue, 4 Jul 2023 14:07:08 +0000 (16:07 +0200)
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.

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

clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

index bae528a..704e61b 100644 (file)
 #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 @@ void toLSPDiags(
     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
index 5f36f28..51ffa45 100644 (file)
@@ -38,6 +38,7 @@
 #include "gtest/gtest.h"
 #include <cstddef>
 #include <memory>
+#include <optional>
 #include <string>
 #include <utility>
 #include <vector>
@@ -1879,6 +1880,42 @@ TEST(Diagnostics, Tags) {
                 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"]]