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;
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
