diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp --- a/clang-tools-extra/clangd/CodeComplete.cpp +++ b/clang-tools-extra/clangd/CodeComplete.cpp @@ -21,6 +21,7 @@ #include "AST.h" #include "CodeCompletionStrings.h" #include "Compiler.h" +#include "Config.h" #include "ExpectedTypes.h" #include "Feature.h" #include "FileDistance.h" @@ -1554,7 +1555,8 @@ Inserter.emplace( SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style, SemaCCInput.ParseInput.CompileCommand.Directory, - &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo()); + &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo(), + Config::current().Style.IncludeDelimiter); for (const auto &Inc : Includes.MainFileIncludes) Inserter->addExisting(Inc); @@ -1637,7 +1639,8 @@ auto Style = getFormatStyleForFile(FileName, Content, TFS); // This will only insert verbatim headers. Inserter.emplace(FileName, Content, Style, - /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr); + /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr, + Config::current().Style.IncludeDelimiter); auto Identifiers = collectIdentifiers(Content, Style); std::vector IdentifierResults; diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h --- a/clang-tools-extra/clangd/Config.h +++ b/clang-tools-extra/clangd/Config.h @@ -118,12 +118,16 @@ } Includes; } Diagnostics; + enum class IncludeDelimiterPolicy { Auto, AlwaysBrackets }; /// Style of the codebase. struct { // Namespaces that should always be fully qualified, meaning no "using" // declarations, always spell out the whole name (with or without leading // ::). All nested namespaces are affected as well. std::vector FullyQualifiedNamespaces; + + // Whether inserted includes should use <> or "". + IncludeDelimiterPolicy IncludeDelimiter = IncludeDelimiterPolicy::Auto; } Style; /// Configures code completion feature. diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp --- a/clang-tools-extra/clangd/ConfigCompile.cpp +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -488,6 +488,17 @@ FullyQualifiedNamespaces.begin(), FullyQualifiedNamespaces.end()); }); } + if (F.IncludeDelimiter) { + if (auto Val = compileEnum( + "IncludeDelimiter", **F.IncludeDelimiter) + .map("Auto", Config::IncludeDelimiterPolicy::Auto) + .map("AlwaysBrackets", + Config::IncludeDelimiterPolicy::AlwaysBrackets) + .value()) + Out.Apply.push_back([Val](const Params &, Config &C) { + C.Style.IncludeDelimiter = *Val; + }); + } } void appendTidyCheckSpec(std::string &CurSpec, diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -293,6 +293,12 @@ // ::). All nested namespaces are affected as well. // Affects availability of the AddUsing tweak. std::vector> FullyQualifiedNamespaces; + + // Whether inserted includes should use <> or "". By default, system + // headers use <> and non-system headers use "". This only affects headers + // inserted by IncludeInserter, and will not reformat existing includes. + // Legal values are "Auto" and "AlwaysBracket". + std::optional> IncludeDelimiter; }; StyleBlock Style; diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -116,6 +116,9 @@ if (auto Values = scalarValues(N)) F.FullyQualifiedNamespaces = std::move(*Values); }); + Dict.handle("IncludeDelimiter", [&](Node &N) { + F.IncludeDelimiter = scalarValue(N, "IncludeDelimiter"); + }); Dict.parse(N); } diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h --- a/clang-tools-extra/clangd/Headers.h +++ b/clang-tools-extra/clangd/Headers.h @@ -9,6 +9,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_HEADERS_H +#include "Config.h" #include "Protocol.h" #include "SourceCode.h" #include "index/Symbol.h" @@ -219,10 +220,12 @@ // include path of non-verbatim header will not be shortened. IncludeInserter(StringRef FileName, StringRef Code, const format::FormatStyle &Style, StringRef BuildDir, - HeaderSearch *HeaderSearchInfo) + HeaderSearch *HeaderSearchInfo, + Config::IncludeDelimiterPolicy IncludeDelimiter) : FileName(FileName), Code(Code), BuildDir(BuildDir), HeaderSearchInfo(HeaderSearchInfo), - Inserter(FileName, Code, Style.IncludeStyle) {} + Inserter(FileName, Code, Style.IncludeStyle), + IncludeDelimiter(IncludeDelimiter) {} void addExisting(const Inclusion &Inc); @@ -266,6 +269,7 @@ HeaderSearch *HeaderSearchInfo = nullptr; llvm::StringSet<> IncludedHeaders; // Both written and resolved. tooling::HeaderIncludes Inserter; // Computers insertion replacement. + Config::IncludeDelimiterPolicy IncludeDelimiter; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp --- a/clang-tools-extra/clangd/Headers.cpp +++ b/clang-tools-extra/clangd/Headers.cpp @@ -349,7 +349,8 @@ // FIXME: should we allow (some limited number of) "../header.h"? if (llvm::sys::path::is_absolute(Suggested)) return std::nullopt; - if (IsSystem) + if (IsSystem || + IncludeDelimiter == Config::IncludeDelimiterPolicy::AlwaysBrackets) Suggested = "<" + Suggested + ">"; else Suggested = "\"" + Suggested + "\""; diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp --- a/clang-tools-extra/clangd/ParsedAST.cpp +++ b/clang-tools-extra/clangd/ParsedAST.cpp @@ -565,7 +565,8 @@ getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS); auto Inserter = std::make_shared( Filename, Inputs.Contents, Style, BuildDir.get(), - &Clang->getPreprocessor().getHeaderSearchInfo()); + &Clang->getPreprocessor().getHeaderSearchInfo(), + Cfg.Style.IncludeDelimiter); ArrayRef MainFileIncludes; if (Preamble) { MainFileIncludes = Preamble->Includes.MainFileIncludes; diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -248,20 +248,17 @@ TEST_F(ConfigCompileTests, DiagnosticsIncludeCleaner) { // Defaults to None. EXPECT_TRUE(compileAndApply()); - EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::IncludesPolicy::None); + EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::None); Frag = {}; Frag.Diagnostics.UnusedIncludes.emplace("None"); EXPECT_TRUE(compileAndApply()); - EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::IncludesPolicy::None); + EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::None); Frag = {}; Frag.Diagnostics.UnusedIncludes.emplace("Strict"); EXPECT_TRUE(compileAndApply()); - EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, - Config::IncludesPolicy::Strict); + EXPECT_EQ(Conf.Diagnostics.UnusedIncludes, Config::IncludesPolicy::Strict); Frag = {}; EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty()) @@ -542,6 +539,21 @@ Frag.Style.FullyQualifiedNamespaces.push_back(std::string("bar")); EXPECT_TRUE(compileAndApply()); EXPECT_THAT(Conf.Style.FullyQualifiedNamespaces, ElementsAre("foo", "bar")); + + Frag.Style.IncludeDelimiter.emplace("AlwaysBrackets"); + EXPECT_TRUE(compileAndApply()); + EXPECT_EQ(Conf.Style.IncludeDelimiter, + Config::IncludeDelimiterPolicy::AlwaysBrackets); + + Frag = {}; + Frag.Style.IncludeDelimiter.emplace("Foo"); + EXPECT_TRUE(compileAndApply()); + EXPECT_EQ(Conf.Style.IncludeDelimiter, Config::IncludeDelimiterPolicy::Auto) + << "by default"; + EXPECT_THAT( + Diags.Diagnostics, + ElementsAre(diagMessage("Invalid IncludeDelimiter value 'Foo'. Valid " + "values are Auto, AlwaysBrackets."))); } TEST_F(ConfigCompileTests, AllowDiagsFromStalePreamble) { diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -265,13 +265,15 @@ CapturedDiags Diags; Annotations YAML(R"yaml( Style: - FullyQualifiedNamespaces: [foo, bar])yaml"); + FullyQualifiedNamespaces: [foo, bar] + IncludeDelimiter: AlwaysBrackets)yaml"); auto Results = Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback()); ASSERT_THAT(Diags.Diagnostics, IsEmpty()); ASSERT_EQ(Results.size(), 1u); EXPECT_THAT(Results[0].Style.FullyQualifiedNamespaces, ElementsAre(val("foo"), val("bar"))); + EXPECT_EQ("AlwaysBrackets", **Results[0].Style.IncludeDelimiter); } TEST(ParseYAML, DiagnosticsMode) { diff --git a/clang-tools-extra/clangd/unittests/HeadersTests.cpp b/clang-tools-extra/clangd/unittests/HeadersTests.cpp --- a/clang-tools-extra/clangd/unittests/HeadersTests.cpp +++ b/clang-tools-extra/clangd/unittests/HeadersTests.cpp @@ -108,7 +108,8 @@ IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(), CDB.getCompileCommand(MainFile)->Directory, - &Clang->getPreprocessor().getHeaderSearchInfo()); + &Clang->getPreprocessor().getHeaderSearchInfo(), + IncludeDelimiter); for (const auto &Inc : Inclusions) Inserter.addExisting(Inc); auto Inserted = ToHeaderFile(Preferred); @@ -128,7 +129,8 @@ IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(), CDB.getCompileCommand(MainFile)->Directory, - &Clang->getPreprocessor().getHeaderSearchInfo()); + &Clang->getPreprocessor().getHeaderSearchInfo(), + IncludeDelimiter); auto Edit = Inserter.insert(VerbatimHeader, Directive); Action.EndSourceFile(); return Edit; @@ -139,6 +141,8 @@ std::string MainFile = testPath("main.cpp"); std::string Subdir = testPath("sub"); std::string SearchDirArg = (llvm::Twine("-I") + Subdir).str(); + Config::IncludeDelimiterPolicy IncludeDelimiter = + Config::IncludeDelimiterPolicy::Auto; IgnoringDiagConsumer IgnoreDiags; std::unique_ptr Clang; }; @@ -309,6 +313,9 @@ std::string Path = testPath("sub/bar.h"); FS.Files[Path] = ""; EXPECT_EQ(calculate(Path), "\"bar.h\""); + + IncludeDelimiter = Config::IncludeDelimiterPolicy::AlwaysBrackets; + EXPECT_EQ(calculate(Path), ""); } TEST_F(HeadersTest, DoNotInsertIfInSameFile) { @@ -331,6 +338,17 @@ EXPECT_EQ(calculate(BarHeader), "\"sub/bar.h\""); } +TEST_F(HeadersTest, ShortenIncludesInSearchPathBracketed) { + IncludeDelimiter = Config::IncludeDelimiterPolicy::AlwaysBrackets; + std::string BarHeader = testPath("sub/bar.h"); + EXPECT_EQ(calculate(BarHeader), ""); + + SearchDirArg = (llvm::Twine("-I") + Subdir + "/..").str(); + CDB.ExtraClangFlags = {SearchDirArg.c_str()}; + BarHeader = testPath("sub/bar.h"); + EXPECT_EQ(calculate(BarHeader), ""); +} + TEST_F(HeadersTest, ShortenedIncludeNotInSearchPath) { std::string BarHeader = llvm::sys::path::convert_to_slash(testPath("sub-2/bar.h")); @@ -343,6 +361,10 @@ std::string BazHeader = testPath("sub/baz.h"); EXPECT_EQ(calculate(BarHeader, BazHeader), "\"baz.h\""); + + IncludeDelimiter = Config::IncludeDelimiterPolicy::AlwaysBrackets; + std::string BiffHeader = testPath("sub/biff.h"); + EXPECT_EQ(calculate(BarHeader, BiffHeader), ""); } TEST_F(HeadersTest, DontInsertDuplicatePreferred) { @@ -374,8 +396,10 @@ TEST(Headers, NoHeaderSearchInfo) { std::string MainFile = testPath("main.cpp"); - IncludeInserter Inserter(MainFile, /*Code=*/"", format::getLLVMStyle(), - /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr); + IncludeInserter Inserter( + MainFile, /*Code=*/"", format::getLLVMStyle(), + /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr, + /*IncludeDelimiter*/ Config::IncludeDelimiterPolicy::Auto); auto HeaderPath = testPath("sub/bar.h"); auto Inserting = HeaderFile{HeaderPath, /*Verbatim=*/false};