[clang-tools-extra] 8f84797 - [clangd] downgrade missing-includes diagnostic to Information level

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2023-05-16T15:03:50+02:00
New Revision: 8f8479789a08a7ec68008b20d3889ccf7fbcda1c

URL: https://github.com/llvm/llvm-project/commit/8f8479789a08a7ec68008b20d3889ccf7fbcda1c
DIFF: https://github.com/llvm/llvm-project/commit/8f8479789a08a7ec68008b20d3889ccf7fbcda1c.diff

LOG: [clangd] downgrade missing-includes diagnostic to Information level

In practice, a Warning on every occurrence is very unpopular, even on a codebase
with clear rules about direct inclusion & moderately good compliance.

This change has various practical effects (in vscode for concreteness):
  - makes the diagnostic decoration less striking (blue vs yellow)
  - makes these diagnostics visually distinct from others when reading
  - causes these diagnostics to sort last in the "problems" view
  - allows these diagnostics to be easily filtered from the "problems" view

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

Added: 
    

Modified: 
    clang-tools-extra/clangd/IncludeCleaner.cpp
    clang-tools-extra/clangd/test/include-cleaner-batch-fix.test

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 22d0808235f7f..18be1329f1fad 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -218,7 +218,23 @@ std::vector<Diag> generateMissingIncludeDiagnostics(
     D.Source = Diag::DiagSource::Clangd;
     D.File = AST.tuPath();
     D.InsideMainFile = true;
-    D.Severity = DiagnosticsEngine::Warning;
+    // We avoid the "warning" severity here in favor of LSP's "information".
+    //
+    // Users treat most warnings on code being edited as high-priority.
+    // They don't think of include cleanups the same way: they want to edit
+    // lines with existing violations without fixing them.
+    // Diagnostics at the same level tend to be visually indistinguishable,
+    // and a few missing includes can cause many diagnostics.
+    // Marking these as "information" leaves them visible, but less intrusive.
+    //
+    // (These concerns don't apply to unused #include warnings: these are fewer,
+    // they appear on infrequently-edited lines with few other warnings, and
+    // the 'Unneccesary' tag often result in a 
diff erent rendering)
+    //
+    // Usually clang's "note" severity usually has special semantics, being
+    // translated into LSP RelatedInformation of a parent diagnostic.
+    // But not here: these aren't processed by clangd's DiagnosticConsumer.
+    D.Severity = DiagnosticsEngine::Note;
     D.Range = clangd::Range{
         offsetToPosition(Code,
                          SymbolWithMissingInclude.SymRefRange.beginOffset()),

diff  --git a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
index b7e9661b79b08..cfc84ad27a2bd 100644
--- a/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
+++ b/clang-tools-extra/clangd/test/include-cleaner-batch-fix.test
@@ -44,7 +44,7 @@
 # CHECK-NEXT:             "line": 2
 # CHECK-NEXT:           }
 # CHECK-NEXT:         },
-# CHECK-NEXT:         "severity": 2,
+# CHECK-NEXT:         "severity": 3,
 # CHECK-NEXT:         "source": "clangd"
 # CHECK-NEXT:       },
 # CHECK-NEXT:       {
@@ -60,7 +60,7 @@
 # CHECK-NEXT:             "line": 2
 # CHECK-NEXT:           }
 # CHECK-NEXT:         },
-# CHECK-NEXT:         "severity": 2,
+# CHECK-NEXT:         "severity": 3,
 # CHECK-NEXT:         "source": "clangd"
 # CHECK-NEXT:       },
 # CHECK-NEXT:       {
@@ -112,7 +112,7 @@
 # CHECK-NEXT:     "version": 0
 # CHECK-NEXT:   }
 ---
-{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///simple.cpp"},"range":{"start":{"line":2,"character":1},"end":{"line":2,"character":4}},"context":{"diagnostics":[{"range":{"start": {"line": 2, "character": 1}, "end": {"line": 2, "character": 4}},"severity":2,"message":"No header providing \"Foo\" is directly included (fixes available)", "code": "missing-includes", "source": "clangd"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"test:///simple.cpp"},"range":{"start":{"line":2,"character":1},"end":{"line":2,"character":4}},"context":{"diagnostics":[{"range":{"start": {"line": 2, "character": 1}, "end": {"line": 2, "character": 4}},"severity":3,"message":"No header providing \"Foo\" is directly included (fixes available)", "code": "missing-includes", "source": "clangd"}]}}}
 #      CHECK:  "id": 2,
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": [