Skip to content

Commit 8f84797

Browse files
committed
[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
1 parent 8178de3 commit 8f84797

File tree

2 files changed

+20
-4
lines changed

2 files changed

+20
-4
lines changed

clang-tools-extra/clangd/IncludeCleaner.cpp

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,23 @@ std::vector<Diag> generateMissingIncludeDiagnostics(
218218
D.Source = Diag::DiagSource::Clangd;
219219
D.File = AST.tuPath();
220220
D.InsideMainFile = true;
221-
D.Severity = DiagnosticsEngine::Warning;
221+
// We avoid the "warning" severity here in favor of LSP's "information".
222+
//
223+
// Users treat most warnings on code being edited as high-priority.
224+
// They don't think of include cleanups the same way: they want to edit
225+
// lines with existing violations without fixing them.
226+
// Diagnostics at the same level tend to be visually indistinguishable,
227+
// and a few missing includes can cause many diagnostics.
228+
// Marking these as "information" leaves them visible, but less intrusive.
229+
//
230+
// (These concerns don't apply to unused #include warnings: these are fewer,
231+
// they appear on infrequently-edited lines with few other warnings, and
232+
// the 'Unneccesary' tag often result in a different rendering)
233+
//
234+
// Usually clang's "note" severity usually has special semantics, being
235+
// translated into LSP RelatedInformation of a parent diagnostic.
236+
// But not here: these aren't processed by clangd's DiagnosticConsumer.
237+
D.Severity = DiagnosticsEngine::Note;
222238
D.Range = clangd::Range{
223239
offsetToPosition(Code,
224240
SymbolWithMissingInclude.SymRefRange.beginOffset()),

clang-tools-extra/clangd/test/include-cleaner-batch-fix.test

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@
4444
# CHECK-NEXT: "line": 2
4545
# CHECK-NEXT: }
4646
# CHECK-NEXT: },
47-
# CHECK-NEXT: "severity": 2,
47+
# CHECK-NEXT: "severity": 3,
4848
# CHECK-NEXT: "source": "clangd"
4949
# CHECK-NEXT: },
5050
# CHECK-NEXT: {
@@ -60,7 +60,7 @@
6060
# CHECK-NEXT: "line": 2
6161
# CHECK-NEXT: }
6262
# CHECK-NEXT: },
63-
# CHECK-NEXT: "severity": 2,
63+
# CHECK-NEXT: "severity": 3,
6464
# CHECK-NEXT: "source": "clangd"
6565
# CHECK-NEXT: },
6666
# CHECK-NEXT: {
@@ -112,7 +112,7 @@
112112
# CHECK-NEXT: "version": 0
113113
# CHECK-NEXT: }
114114
---
115-
{"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"}]}}}
115+
{"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"}]}}}
116116
# CHECK: "id": 2,
117117
# CHECK-NEXT: "jsonrpc": "2.0",
118118
# CHECK-NEXT: "result": [

0 commit comments

Comments
 (0)