kbobyrev updated this revision to Diff 405238.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.
Address review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117461/new/
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,9 @@
TEST(DiagnosticsTest, IncludeCleaner) {
Annotations Test(R"cpp(
-$fix[[ $diag[[#include "unused.h"]]
+$remove[[ $diag[[#include "unused.h"]]$pragma_keep[[]]
+]]
+$remove_no_fixit[[ $diag_no_fixit[[#include "no_fixit.h" // Has comment: don't offer IWYU pragma keep FixIt!]]
]]
#include "used.h"
@@ -1629,6 +1631,10 @@
#pragma once
void unused() {}
)cpp";
+ TU.AdditionalFiles["no_fixit.h"] = R"cpp(
+ #pragma once
+ void no_fixit() {}
+ )cpp";
TU.AdditionalFiles["used.h"] = R"cpp(
#pragma once
void used() {}
@@ -1642,10 +1648,19 @@
WithContextValue WithCfg(Config::Key, std::move(Cfg));
EXPECT_THAT(
*TU.build().getDiagnostics(),
- 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")))));
+ UnorderedElementsAre(
+ AllOf(
+ Diag(Test.range("diag"), "included header unused.h is not used"),
+ WithTag(DiagnosticTag::Unnecessary), DiagSource(Diag::Clangd),
+ WithFix(
+ Fix(Test.range("remove"), "", "remove #include directive"),
+ Fix(Test.range("pragma_keep"), " // IWYU pragma: keep",
+ "insert IWYU pragma: keep"))),
+ AllOf(Diag(Test.range("diag_no_fixit"),
+ "included header no_fixit.h is not used"),
+ WithTag(DiagnosticTag::Unnecessary), DiagSource(Diag::Clangd),
+ WithFix(Fix(Test.range("remove_no_fixit"), "",
+ "remove #include directive")))));
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,34 @@
return Result;
}
+// Returns the range starting right after the included header name and ending at
+// EOL.
+llvm::Optional<clangd::Range> getIWYUPragmaRange(llvm::StringRef Code,
+ unsigned HashOffset) {
+ // Attempt to parse the string: it should be a simple one-line inclusion
+ // without a trailing comment. Other formats are not supported.
+ StringRef RestOfLine = Code.substr(HashOffset).take_until([](char C) {
+ return C == '\n' || C == '\r';
+ });
+ if (!RestOfLine.consume_front("#"))
+ return None;
+ RestOfLine = RestOfLine.ltrim();
+ if (!RestOfLine.consume_front("include"))
+ return None;
+ RestOfLine = RestOfLine.ltrim();
+ if (!RestOfLine.consume_front("<") && !RestOfLine.consume_front("\""))
+ return None;
+ RestOfLine =
+ RestOfLine.drop_until([](char C) { return C == '>' || C == '"'; });
+ if (RestOfLine.startswith("\"") || RestOfLine.startswith(">"))
+ RestOfLine = RestOfLine.drop_front();
+ if (!RestOfLine.ltrim().empty())
+ return None;
+ return clangd::Range{
+ offsetToPosition(Code, RestOfLine.begin() - Code.data()),
+ offsetToPosition(Code, RestOfLine.begin() - Code.data())};
+}
+
// 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 +450,24 @@
// 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. The FixIt will
+ // be offered for lines with no extra comment at the end of the line that
+ // have the full name of an included header.
+ if (auto PragmaRange = getIWYUPragmaRange(Code, Inc->HashOffset)) {
+ D.Fixes.emplace_back();
+ D.Fixes.back().Message = "insert IWYU pragma: keep";
+ D.Fixes.back().Edits.emplace_back();
+ D.Fixes.back().Edits.back().range = *PragmaRange;
+ 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
