llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clangd Author: None (Harald-R) <details> <summary>Changes</summary> The missing include diagnostic has the capability to introduce the necessary headers into the source file. However, it does not currently follow the inclusion style found in the `.clangd` file. For example, if the file explicitly mentions that headers should be include with angled brackets, they could be included with quotes instead. More details in https://github.com/llvm/llvm-project/issues/138740. This PR fixes this gap, so that the style configuration is followed. --- Full diff: https://github.com/llvm/llvm-project/pull/140594.diff 4 Files Affected: - (modified) clang-tools-extra/clangd/IncludeCleaner.cpp (+14-7) - (modified) clang-tools-extra/clangd/IncludeCleaner.h (+4-5) - (modified) clang-tools-extra/clangd/ParsedAST.cpp (+2-1) - (modified) clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp (+13-2) ``````````diff diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index dc4c8fc498b1f..ce16d060a1f06 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -117,7 +117,8 @@ bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST, std::vector<Diag> generateMissingIncludeDiagnostics( ParsedAST &AST, llvm::ArrayRef<MissingIncludeDiagInfo> MissingIncludes, - llvm::StringRef Code, HeaderFilter IgnoreHeaders, const ThreadsafeFS &TFS) { + llvm::StringRef Code, HeaderFilter IgnoreHeaders, + HeaderFilter AngledHeaders, const ThreadsafeFS &TFS) { std::vector<Diag> Result; const SourceManager &SM = AST.getSourceManager(); const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID()); @@ -142,6 +143,13 @@ std::vector<Diag> generateMissingIncludeDiagnostics( llvm::StringRef HeaderRef{Spelling}; bool Angled = HeaderRef.starts_with("<"); + for (auto Filter : AngledHeaders) { + if (Filter(HeaderRef)) { + Angled = true; + break; + } + } + // We might suggest insertion of an existing include in edge cases, e.g., // include is present in a PP-disabled region, or spelling of the header // turns out to be the same as one of the unresolved includes in the @@ -481,18 +489,17 @@ bool isPreferredProvider(const Inclusion &Inc, return false; // no header provides the symbol } -std::vector<Diag> -issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code, - const IncludeCleanerFindings &Findings, - const ThreadsafeFS &TFS, - HeaderFilter IgnoreHeaders) { +std::vector<Diag> issueIncludeCleanerDiagnostics( + ParsedAST &AST, llvm::StringRef Code, + const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS, + HeaderFilter IgnoreHeaders, HeaderFilter AngledHeaders) { trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics"); std::vector<Diag> UnusedIncludes = generateUnusedIncludeDiagnostics( AST.tuPath(), Findings.UnusedIncludes, Code, IgnoreHeaders); std::optional<Fix> RemoveAllUnused = removeAllUnusedIncludes(UnusedIncludes); std::vector<Diag> MissingIncludeDiags = generateMissingIncludeDiagnostics( - AST, Findings.MissingIncludes, Code, IgnoreHeaders, TFS); + AST, Findings.MissingIncludes, Code, IgnoreHeaders, AngledHeaders, TFS); std::optional<Fix> AddAllMissing = addAllMissingIncludes(MissingIncludeDiags); std::optional<Fix> FixAll; diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h index 3f6e3b2fd45b6..884feb69488d5 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.h +++ b/clang-tools-extra/clangd/IncludeCleaner.h @@ -57,11 +57,10 @@ IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST, bool AnalyzeAngledIncludes = false); -std::vector<Diag> -issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code, - const IncludeCleanerFindings &Findings, - const ThreadsafeFS &TFS, - HeaderFilter IgnoreHeader = {}); +std::vector<Diag> issueIncludeCleanerDiagnostics( + ParsedAST &AST, llvm::StringRef Code, + const IncludeCleanerFindings &Findings, const ThreadsafeFS &TFS, + HeaderFilter IgnoreHeader = {}, HeaderFilter AngledHeaders = {}); /// Converts the clangd include representation to include-cleaner /// include representation. diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp index 3f63daaf400db..a47bfafe8a92c 100644 --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -382,7 +382,8 @@ std::vector<Diag> getIncludeCleanerDiags(ParsedAST &AST, llvm::StringRef Code, if (SuppressUnused) Findings.UnusedIncludes.clear(); return issueIncludeCleanerDiagnostics(AST, Code, Findings, TFS, - Cfg.Diagnostics.Includes.IgnoreHeader); + Cfg.Diagnostics.Includes.IgnoreHeader, + Cfg.Style.AngledHeaders); } tidy::ClangTidyCheckFactories diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index 0ee748c1ed2d0..fda1b3b1835ae 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -220,12 +220,13 @@ TEST(IncludeCleaner, ComputeMissingHeaders) { TEST(IncludeCleaner, GenerateMissingHeaderDiags) { Annotations MainFile(R"cpp( #include "a.h" +#include "a_angled.h" #include "all.h" $insert_b[[]]#include "baz.h" #include "dir/c.h" $insert_d[[]]$insert_foo[[]]#include "fuzz.h" #include "header.h" -$insert_foobar[[]]#include <e.h> +$insert_foobar[[]]$insert_b_angled[[]]#include <e.h> $insert_f[[]]$insert_vector[[]] #define DEF(X) const Foo *X; @@ -237,6 +238,7 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) { void foo() { $b[[b]](); + $b_angled[[b_angled]](); ns::$bar[[Bar]] bar; bar.d(); @@ -262,6 +264,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) { TU.Filename = "main.cpp"; TU.AdditionalFiles["a.h"] = guard("#include \"b.h\""); TU.AdditionalFiles["b.h"] = guard("void b();"); + TU.AdditionalFiles["a_angled.h"] = guard("#include \"b_angled.h\""); + TU.AdditionalFiles["b_angled.h"] = guard("void b_angled();"); TU.AdditionalFiles["dir/c.h"] = guard("#include \"d.h\""); TU.AdditionalFiles["dir/d.h"] = @@ -297,7 +301,8 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) { Findings.UnusedIncludes.clear(); std::vector<clangd::Diag> Diags = issueIncludeCleanerDiagnostics( AST, TU.Code, Findings, MockFS(), - {[](llvm::StringRef Header) { return Header.ends_with("buzz.h"); }}); + {[](llvm::StringRef Header) { return Header.ends_with("buzz.h"); }}, + {[](llvm::StringRef Header) { return Header.contains("b_angled.h"); }}); EXPECT_THAT( Diags, UnorderedElementsAre( @@ -306,6 +311,12 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) { withFix({Fix(MainFile.range("insert_b"), "#include \"b.h\"\n", "#include \"b.h\""), FixMessage("add all missing includes")})), + AllOf(Diag(MainFile.range("b_angled"), + "No header providing \"b_angled\" is directly included"), + withFix( + {Fix(MainFile.range("insert_b_angled"), + "#include <b_angled.h>\n", "#include \"b_angled.h\""), + FixMessage("add all missing includes")})), AllOf(Diag(MainFile.range("bar"), "No header providing \"ns::Bar\" is directly included"), withFix({Fix(MainFile.range("insert_d"), `````````` </details> https://github.com/llvm/llvm-project/pull/140594 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
