[clangd] Allow specifying what headers are always included via "" or …

…<> (llvm#67749) Projects can now add config fragments like this to their .clangd: ```yaml Style: QuotedHeaders: "src/.*" AngledHeaders: ["path/sdk/.*", "third-party/.*"] ``` to force headers inserted via the --header-insertion=iwyu mode matching at least one of the regexes to have <> (AngledHeaders) or "" (QuotedHeaders) around them, respectively. For other headers (and in conflicting cases where both styles have a matching regex), the current system header detection remains. Fixes clangd/clangd#1247
ROCm · Dec 27, 2024 · 1f90797 · 1f90797
1 parent 8caeb2e
commit 1f90797
Show file tree

Hide file tree

Showing 13 changed files with 242 additions and 28 deletions.
diff --git a/clang-tools-extra/clangd/CodeComplete.cpp b/clang-tools-extra/clangd/CodeComplete.cpp
@@ -807,8 +807,8 @@ SpecifiedScope getQueryScopes(CodeCompletionContext &CCContext,
   llvm::StringRef SpelledSpecifier = Lexer::getSourceText(
       CharSourceRange::getCharRange(SemaSpecifier->getRange()),
       CCSema.SourceMgr, clang::LangOptions());
-  if (SpelledSpecifier.consume_front("::")) 
-      Scopes.QueryScopes = {""};
+  if (SpelledSpecifier.consume_front("::"))
+    Scopes.QueryScopes = {""};
   Scopes.UnresolvedQualifier = std::string(SpelledSpecifier);
   // Sema excludes the trailing "::".
   if (!Scopes.UnresolvedQualifier->empty())
@@ -1604,7 +1604,7 @@ class CodeCompleteFlow {
   CompletionPrefix HeuristicPrefix;
   std::optional<FuzzyMatcher> Filter; // Initialized once Sema runs.
   Range ReplacedRange;
-  std::vector<std::string> QueryScopes; // Initialized once Sema runs.
+  std::vector<std::string> QueryScopes;      // Initialized once Sema runs.
   std::vector<std::string> AccessibleScopes; // Initialized once Sema runs.
   // Initialized once QueryScopes is initialized, if there are scopes.
   std::optional<ScopeDistance> ScopeProximity;
@@ -1663,7 +1663,9 @@ class CodeCompleteFlow {
       Inserter.emplace(
           SemaCCInput.FileName, SemaCCInput.ParseInput.Contents, Style,
           SemaCCInput.ParseInput.CompileCommand.Directory,
-          &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo());
+          &Recorder->CCSema->getPreprocessor().getHeaderSearchInfo(),
+          Config::current().Style.QuotedHeaders,
+          Config::current().Style.AngledHeaders);
       for (const auto &Inc : Includes.MainFileIncludes)
         Inserter->addExisting(Inc);
 
@@ -1746,7 +1748,9 @@ class CodeCompleteFlow {
     auto Style = getFormatStyleForFile(FileName, Content, TFS, false);
     // This will only insert verbatim headers.
     Inserter.emplace(FileName, Content, Style,
-                     /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr);
+                     /*BuildDir=*/"", /*HeaderSearchInfo=*/nullptr,
+                     Config::current().Style.QuotedHeaders,
+                     Config::current().Style.AngledHeaders);
 
     auto Identifiers = collectIdentifiers(Content, Style);
     std::vector<RawIdentifier> IdentifierResults;

diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
@@ -124,6 +124,10 @@ struct Config {
     // declarations, always spell out the whole name (with or without leading
     // ::). All nested namespaces are affected as well.
     std::vector<std::string> FullyQualifiedNamespaces;
+
+    // List of matcher functions for inserting certain headers with <> or "".
+    std::vector<std::function<bool(llvm::StringRef)>> QuotedHeaders;
+    std::vector<std::function<bool(llvm::StringRef)>> AngledHeaders;
   } Style;
 
   /// controls the completion options for argument lists.

diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -482,6 +482,55 @@ struct FragmentCompiler {
             FullyQualifiedNamespaces.begin(), FullyQualifiedNamespaces.end());
       });
     }
+    auto QuotedFilter = compileHeaderRegexes(F.QuotedHeaders);
+    if (QuotedFilter.has_value()) {
+      Out.Apply.push_back(
+          [QuotedFilter = *QuotedFilter](const Params &, Config &C) {
+            C.Style.QuotedHeaders.emplace_back(QuotedFilter);
+          });
+    }
+    auto AngledFilter = compileHeaderRegexes(F.AngledHeaders);
+    if (AngledFilter.has_value()) {
+      Out.Apply.push_back(
+          [AngledFilter = *AngledFilter](const Params &, Config &C) {
+            C.Style.AngledHeaders.emplace_back(AngledFilter);
+          });
+    }
+  }
+
+  auto compileHeaderRegexes(llvm::ArrayRef<Located<std::string>> HeaderPatterns)
+      -> std::optional<std::function<bool(llvm::StringRef)>> {
+    // TODO: Share this code with Diagnostics.Includes.IgnoreHeader
+#ifdef CLANGD_PATH_CASE_INSENSITIVE
+    static llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
+#else
+    static llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
+#endif
+    auto Filters = std::make_shared<std::vector<llvm::Regex>>();
+    for (auto &HeaderPattern : HeaderPatterns) {
+      // Anchor on the right.
+      std::string AnchoredPattern = "(" + *HeaderPattern + ")$";
+      llvm::Regex CompiledRegex(AnchoredPattern, Flags);
+      std::string RegexError;
+      if (!CompiledRegex.isValid(RegexError)) {
+        diag(Warning,
+             llvm::formatv("Invalid regular expression '{0}': {1}",
+                           *HeaderPattern, RegexError)
+                 .str(),
+             HeaderPattern.Range);
+        continue;
+      }
+      Filters->push_back(std::move(CompiledRegex));
+    }
+    if (Filters->empty())
+      return std::nullopt;
+    auto Filter = [Filters](llvm::StringRef Path) {
+      for (auto &Regex : *Filters)
+        if (Regex.match(Path))
+          return true;
+      return false;
+    };
+    return Filter;
   }
 
   void appendTidyCheckSpec(std::string &CurSpec,

diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
@@ -301,6 +301,23 @@ struct Fragment {
     // ::). All nested namespaces are affected as well.
     // Affects availability of the AddUsing tweak.
     std::vector<Located<std::string>> FullyQualifiedNamespaces;
+
+    /// List of regexes for headers that should always be included with a
+    /// ""-style include. By default, and in case of a conflict with
+    /// AngledHeaders (i.e. a header matches a regex in both QuotedHeaders and
+    /// AngledHeaders), system headers use <> and non-system headers use "".
+    /// These can match any suffix of the header file in question.
+    /// Matching is performed against the header text, not its absolute path
+    /// within the project.
+    std::vector<Located<std::string>> QuotedHeaders;
+    /// List of regexes for headers that should always be included with a
+    /// <>-style include. By default, and in case of a conflict with
+    /// AngledHeaders (i.e. a header matches a regex in both QuotedHeaders and
+    /// AngledHeaders), system headers use <> and non-system headers use "".
+    /// These can match any suffix of the header file in question.
+    /// Matching is performed against the header text, not its absolute path
+    /// within the project.
+    std::vector<Located<std::string>> AngledHeaders;
   };
   StyleBlock Style;
 

diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -116,6 +116,14 @@ class Parser {
       if (auto Values = scalarValues(N))
         F.FullyQualifiedNamespaces = std::move(*Values);
     });
+    Dict.handle("QuotedHeaders", [&](Node &N) {
+      if (auto Values = scalarValues(N))
+        F.QuotedHeaders = std::move(*Values);
+    });
+    Dict.handle("AngledHeaders", [&](Node &N) {
+      if (auto Values = scalarValues(N))
+        F.AngledHeaders = std::move(*Values);
+    });
     Dict.parse(N);
   }
 

diff --git a/clang-tools-extra/clangd/Headers.cpp b/clang-tools-extra/clangd/Headers.cpp
@@ -9,6 +9,7 @@
 #include "Headers.h"
 #include "Preamble.h"
 #include "SourceCode.h"
+#include "support/Logger.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/Frontend/CompilerInstance.h"
@@ -30,8 +31,7 @@ namespace clangd {
 class IncludeStructure::RecordHeaders : public PPCallbacks {
 public:
   RecordHeaders(const CompilerInstance &CI, IncludeStructure *Out)
-      : SM(CI.getSourceManager()),
-        Out(Out) {}
+      : SM(CI.getSourceManager()), Out(Out) {}
 
   // Record existing #includes - both written and resolved paths. Only #includes
   // in the main file are collected.
@@ -287,11 +287,11 @@ IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader,
   assert(InsertedHeader.valid());
   if (InsertedHeader.Verbatim)
     return InsertedHeader.File;
-  bool IsAngled = false;
+  bool IsAngledByDefault = false;
   std::string Suggested;
   if (HeaderSearchInfo) {
     Suggested = HeaderSearchInfo->suggestPathToFileForDiagnostics(
-        InsertedHeader.File, BuildDir, IncludingFile, &IsAngled);
+        InsertedHeader.File, BuildDir, IncludingFile, &IsAngledByDefault);
   } else {
     // Calculate include relative to including file only.
     StringRef IncludingDir = llvm::sys::path::parent_path(IncludingFile);
@@ -304,9 +304,33 @@ IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader,
   // FIXME: should we allow (some limited number of) "../header.h"?
   if (llvm::sys::path::is_absolute(Suggested))
     return std::nullopt;
+  bool IsAngled = false;
+  for (auto Filter : AngledHeaders) {
+    if (Filter(Suggested)) {
+      IsAngled = true;
+      break;
+    }
+  }
+  bool IsQuoted = false;
+  for (auto Filter : QuotedHeaders) {
+    if (Filter(Suggested)) {
+      IsQuoted = true;
+      break;
+    }
+  }
+  // No filters apply, or both filters apply (a bug), use system default.
+  if (IsAngled == IsQuoted) {
+    // Probably a bug in the config regex.
+    if (IsAngled && IsQuoted) {
+      elog("Header '{0}' matches both quoted and angled regexes, default will "
+           "be used.",
+           Suggested);
+    }
+    IsAngled = IsAngledByDefault;
+  }
   if (IsAngled)
     Suggested = "<" + Suggested + ">";
-  else
+  else // if (IsQuoted)
     Suggested = "\"" + Suggested + "\"";
   return Suggested;
 }

diff --git a/clang-tools-extra/clangd/Headers.h b/clang-tools-extra/clangd/Headers.h
@@ -33,6 +33,8 @@
 namespace clang {
 namespace clangd {
 
+using HeaderFilter = llvm::ArrayRef<std::function<bool(llvm::StringRef)>>;
+
 /// Returns true if \p Include is literal include like "path" or <path>.
 bool isLiteralInclude(llvm::StringRef Include);
 
@@ -211,10 +213,12 @@ class IncludeInserter {
   // 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, HeaderFilter QuotedHeaders,
+                  HeaderFilter AngledHeaders)
       : FileName(FileName), Code(Code), BuildDir(BuildDir),
         HeaderSearchInfo(HeaderSearchInfo),
-        Inserter(FileName, Code, Style.IncludeStyle) {}
+        Inserter(FileName, Code, Style.IncludeStyle),
+        QuotedHeaders(QuotedHeaders), AngledHeaders(AngledHeaders) {}
 
   void addExisting(const Inclusion &Inc);
 
@@ -258,6 +262,8 @@ class IncludeInserter {
   HeaderSearch *HeaderSearchInfo = nullptr;
   llvm::StringSet<> IncludedHeaders; // Both written and resolved.
   tooling::HeaderIncludes Inserter;  // Computers insertion replacement.
+  HeaderFilter QuotedHeaders;
+  HeaderFilter AngledHeaders;
 };
 
 } // namespace clangd

diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -57,7 +57,6 @@ IncludeCleanerFindings
 computeIncludeCleanerFindings(ParsedAST &AST,
                               bool AnalyzeAngledIncludes = false);
 
-using HeaderFilter = llvm::ArrayRef<std::function<bool(llvm::StringRef)>>;
 std::vector<Diag>
 issueIncludeCleanerDiagnostics(ParsedAST &AST, llvm::StringRef Code,
                                const IncludeCleanerFindings &Findings,

diff --git a/clang-tools-extra/clangd/ParsedAST.cpp b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -639,7 +639,8 @@ ParsedAST::build(llvm::StringRef Filename, const ParseInputs &Inputs,
           getFormatStyleForFile(Filename, Inputs.Contents, *Inputs.TFS, false);
       auto Inserter = std::make_shared<IncludeInserter>(
           Filename, Inputs.Contents, Style, BuildDir.get(),
-          &Clang->getPreprocessor().getHeaderSearchInfo());
+          &Clang->getPreprocessor().getHeaderSearchInfo(),
+          Cfg.Style.QuotedHeaders, Cfg.Style.AngledHeaders);
       ArrayRef<Inclusion> MainFileIncludes;
       if (Preamble) {
         MainFileIncludes = Preamble->Includes.MainFileIncludes;

diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -920,6 +920,41 @@ TEST(CompletionTest, NoIncludeInsertionWhenDeclFoundInFile) {
                           AllOf(named("Y"), Not(insertInclude()))));
 }
 
+TEST(CompletionTest, IncludeInsertionRespectsQuotedAngledConfig) {
+  TestTU TU;
+  TU.ExtraArgs.push_back("-I" + testPath("sub"));
+  TU.AdditionalFiles["sub/bar.h"] = "";
+  auto BarURI = URI::create(testPath("sub/bar.h")).toString();
+
+  Symbol Sym = cls("ns::X");
+  Sym.CanonicalDeclaration.FileURI = BarURI.c_str();
+  Sym.IncludeHeaders.emplace_back(BarURI, 1, Symbol::Include);
+  Annotations Test("int main() { ns::^ }");
+  TU.Code = Test.code().str();
+  auto Results = completions(TU, Test.point(), {Sym});
+  // Default for a local path is quoted include
+  EXPECT_THAT(Results.Completions,
+              ElementsAre(AllOf(named("X"), insertInclude("\"bar.h\""))));
+  {
+    Config C;
+    C.Style.AngledHeaders.push_back(
+        [](auto header) { return header == "bar.h"; });
+    WithContextValue WithCfg(Config::Key, std::move(C));
+    Results = completions(TU, Test.point(), {Sym});
+    EXPECT_THAT(Results.Completions,
+                ElementsAre(AllOf(named("X"), insertInclude("<bar.h>"))));
+  }
+  {
+    Config C;
+    C.Style.QuotedHeaders.push_back(
+        [](auto header) { return header == "bar.h"; });
+    WithContextValue WithCfg(Config::Key, std::move(C));
+    Results = completions(TU, Test.point(), {Sym});
+    EXPECT_THAT(Results.Completions,
+                ElementsAre(AllOf(named("X"), insertInclude("\"bar.h\""))));
+  }
+}
+
 TEST(CompletionTest, IndexSuppressesPreambleCompletions) {
   Annotations Test(R"cpp(
       #include "bar.h"
@@ -1138,8 +1173,8 @@ TEST(CodeCompleteTest, NoColonColonAtTheEnd) {
 }
 
 TEST(CompletionTests, EmptySnippetDoesNotCrash) {
-    // See https://github.com/clangd/clangd/issues/1216
-    auto Results = completions(R"cpp(
+  // See https://github.com/clangd/clangd/issues/1216
+  auto Results = completions(R"cpp(
         int main() {
           auto w = [&](auto &&f) { return f(f); };
           auto f = w([&](auto &&f) {
@@ -1155,18 +1190,18 @@ TEST(CompletionTests, EmptySnippetDoesNotCrash) {
 }
 
 TEST(CompletionTest, Issue1427Crash) {
-    // Need to provide main file signals to ensure that the branch in
-    // SymbolRelevanceSignals::computeASTSignals() that tries to
-    // compute a symbol ID is taken.
-    ASTSignals MainFileSignals;
-    CodeCompleteOptions Opts;
-    Opts.MainFileSignals = &MainFileSignals;
-    completions(R"cpp(
+  // Need to provide main file signals to ensure that the branch in
+  // SymbolRelevanceSignals::computeASTSignals() that tries to
+  // compute a symbol ID is taken.
+  ASTSignals MainFileSignals;
+  CodeCompleteOptions Opts;
+  Opts.MainFileSignals = &MainFileSignals;
+  completions(R"cpp(
       auto f = []() {
         1.0_^
       };
     )cpp",
-                {}, Opts);
+              {}, Opts);
 }
 
 TEST(CompletionTest, BacktrackCrashes) {

diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -545,6 +545,44 @@ TEST_F(ConfigCompileTests, Style) {
   Frag.Style.FullyQualifiedNamespaces.push_back(std::string("bar"));
   EXPECT_TRUE(compileAndApply());
   EXPECT_THAT(Conf.Style.FullyQualifiedNamespaces, ElementsAre("foo", "bar"));
+
+  {
+    Frag = {};
+    EXPECT_TRUE(Conf.Style.QuotedHeaders.empty())
+        << Conf.Style.QuotedHeaders.size();
+    Frag.Style.QuotedHeaders.push_back(Located<std::string>("foo.h"));
+    Frag.Style.QuotedHeaders.push_back(Located<std::string>(".*inc"));
+    EXPECT_TRUE(compileAndApply());
+    auto HeaderFilter = [this](llvm::StringRef Path) {
+      for (auto &Filter : Conf.Style.QuotedHeaders) {
+        if (Filter(Path))
+          return true;
+      }
+      return false;
+    };
+    EXPECT_TRUE(HeaderFilter("foo.h"));
+    EXPECT_TRUE(HeaderFilter("prefix/foo.h"));
+    EXPECT_FALSE(HeaderFilter("bar.h"));
+    EXPECT_FALSE(HeaderFilter("foo.h/bar.h"));
+  }
+
+  {
+    Frag = {};
+    EXPECT_TRUE(Conf.Style.AngledHeaders.empty())
+        << Conf.Style.AngledHeaders.size();
+    Frag.Style.AngledHeaders.push_back(Located<std::string>("foo.h"));
+    Frag.Style.AngledHeaders.push_back(Located<std::string>(".*inc"));
+    EXPECT_TRUE(compileAndApply());
+    auto HeaderFilter = [this](llvm::StringRef Path) {
+      for (auto &Filter : Conf.Style.AngledHeaders) {
+        if (Filter(Path))
+          return true;
+      }
+      return false;
+    };
+    EXPECT_TRUE(HeaderFilter("foo.h"));
+    EXPECT_FALSE(HeaderFilter("bar.h"));
+  }
 }
 } // namespace
 } // namespace config