[PATCH] D117461: [clangd] IncludeCleaner: Attach "insert prgama keep" fix-it to diagnostic

Kirill Bobyrev via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jan 17 02:29:40 PST 2022


kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: usaxena95, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Now that IWYU pragma: keep supresses the warning for unused header, it should
be possible to suggest the user both removing the header and adding a pragma to
keep it instead.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117461

Files:
  clang-tools-extra/clangd/IncludeCleaner.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
@@ -1613,7 +1613,7 @@
 
 TEST(DiagnosticsTest, IncludeCleaner) {
   Annotations Test(R"cpp(
-$fix[[  $diag[[#include "unused.h"]]
+$remove[[  $diag[[#include "unused.h"]]$insert[[]]
 ]]
   #include "used.h"
 
@@ -1645,7 +1645,9 @@
       UnorderedElementsAre(AllOf(
           Diag(Test.range("diag"), "included header unused.h is not used"),
           WithTag(DiagnosticTag::Unnecessary), DiagSource(Diag::Clangd),
-          WithFix(Fix(Test.range("fix"), "", "remove #include directive")))));
+          WithFix(Fix(Test.range("remove"), "", "remove #include directive"),
+                  Fix(Test.range("insert"), " // IWYU pragma: keep",
+                      "insert IWYU pragma: keep")))));
   Cfg.Diagnostics.SuppressAll = true;
   WithContextValue SuppressAllWithCfg(Config::Key, std::move(Cfg));
   EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -205,6 +205,28 @@
   return Result;
 }
 
+// Returns the range starting right after the included header name and ending at
+// EOL.
+clangd::Range getIWYUPragmaRange(llvm::StringRef Code, unsigned HashOffset) {
+  clangd::Range Result;
+  Result.end = Result.start = offsetToPosition(Code, HashOffset);
+
+  unsigned QuotesCount = 0;
+  Result.start.character +=
+      lspLength(Code.drop_front(HashOffset).take_until([&QuotesCount](char C) {
+        if (C == '"')
+          if (++QuotesCount == 2)
+            return true;
+        return C == '>';
+      })) +
+      1;
+  Result.end.character +=
+      lspLength(Code.drop_front(HashOffset).take_until([](char C) {
+        return C == '\n' || C == '\r';
+      }));
+  return Result;
+}
+
 // Finds locations of macros referenced from within the main file. That includes
 // references that were not yet expanded, e.g `BAR` in `#define FOO BAR`.
 void findReferencedMacros(ParsedAST &AST, ReferencedLocations &Result) {
@@ -422,14 +444,23 @@
     // FIXME(kirillbobyrev): Removing inclusion might break the code if the
     // used headers are only reachable transitively through this one. Suggest
     // including them directly instead.
-    // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
-    // (keep/export) remove the warning once we support IWYU pragmas.
     D.Fixes.emplace_back();
     D.Fixes.back().Message = "remove #include directive";
     D.Fixes.back().Edits.emplace_back();
     D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
     D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
     D.InsideMainFile = true;
+    // Allow suppressing the warning through adding keep pragma.
+    // NOTE: This would also erase the comment after the header inclusion if
+    // there is one.
+    D.Fixes.emplace_back();
+    D.Fixes.back().Message = "insert IWYU pragma: keep";
+    D.Fixes.back().Edits.emplace_back();
+    D.Fixes.back().Edits.back().range =
+        getIWYUPragmaRange(Code, Inc->HashOffset);
+    D.InsideMainFile = true;
+    D.Fixes.back().Edits.back().newText = " // IWYU pragma: keep";
+    D.InsideMainFile = true;
     Result.push_back(std::move(D));
   }
   return Result;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117461.400474.patch
Type: text/x-patch
Size: 3550 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220117/d49212d9/attachment.bin>


More information about the cfe-commits mailing list