Skip to content

Commit ff0093c

Browse files
authored
[clangd] Use resolved path when checking AngledHeaders/QuotedHeaders in IncludeInserter (llvm#148371)
This makes IncludeInserter's behavior consistent with include-cleaner, as discussed in llvm#140594.
1 parent 29cde86 commit ff0093c

File tree

5 files changed

+20
-8
lines changed

5 files changed

+20
-8
lines changed

clang-tools-extra/clangd/ConfigFragment.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -315,15 +315,15 @@ struct Fragment {
315315
/// AngledHeaders (i.e. a header matches a regex in both QuotedHeaders and
316316
/// AngledHeaders), system headers use <> and non-system headers use "".
317317
/// These can match any suffix of the header file in question.
318-
/// Matching is performed against the header text, not its absolute path
318+
/// Matching is performed against the absolute path of the header
319319
/// within the project.
320320
std::vector<Located<std::string>> QuotedHeaders;
321321
/// List of regexes for headers that should always be included with a
322322
/// <>-style include. By default, and in case of a conflict with
323323
/// AngledHeaders (i.e. a header matches a regex in both QuotedHeaders and
324324
/// AngledHeaders), system headers use <> and non-system headers use "".
325325
/// These can match any suffix of the header file in question.
326-
/// Matching is performed against the header text, not its absolute path
326+
/// Matching is performed against the absolute path of the header
327327
/// within the project.
328328
std::vector<Located<std::string>> AngledHeaders;
329329
};

clang-tools-extra/clangd/Headers.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,16 +304,17 @@ IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader,
304304
// FIXME: should we allow (some limited number of) "../header.h"?
305305
if (llvm::sys::path::is_absolute(Suggested))
306306
return std::nullopt;
307+
auto HeaderPath = llvm::sys::path::convert_to_slash(InsertedHeader.File);
307308
bool IsAngled = false;
308309
for (auto &Filter : AngledHeaders) {
309-
if (Filter(Suggested)) {
310+
if (Filter(HeaderPath)) {
310311
IsAngled = true;
311312
break;
312313
}
313314
}
314315
bool IsQuoted = false;
315316
for (auto &Filter : QuotedHeaders) {
316-
if (Filter(Suggested)) {
317+
if (Filter(HeaderPath)) {
317318
IsQuoted = true;
318319
break;
319320
}
@@ -324,7 +325,7 @@ IncludeInserter::calculateIncludePath(const HeaderFile &InsertedHeader,
324325
if (IsAngled && IsQuoted) {
325326
elog("Header '{0}' matches both quoted and angled regexes, default will "
326327
"be used.",
327-
Suggested);
328+
HeaderPath);
328329
}
329330
IsAngled = IsAngledByDefault;
330331
}

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -938,7 +938,7 @@ TEST(CompletionTest, IncludeInsertionRespectsQuotedAngledConfig) {
938938
{
939939
Config C;
940940
C.Style.AngledHeaders.push_back(
941-
[](auto header) { return header == "bar.h"; });
941+
[](auto header) { return header.contains("bar.h"); });
942942
WithContextValue WithCfg(Config::Key, std::move(C));
943943
Results = completions(TU, Test.point(), {Sym});
944944
EXPECT_THAT(Results.Completions,
@@ -947,7 +947,7 @@ TEST(CompletionTest, IncludeInsertionRespectsQuotedAngledConfig) {
947947
{
948948
Config C;
949949
C.Style.QuotedHeaders.push_back(
950-
[](auto header) { return header == "bar.h"; });
950+
[](auto header) { return header.contains("bar.h"); });
951951
WithContextValue WithCfg(Config::Key, std::move(C));
952952
Results = completions(TU, Test.point(), {Sym});
953953
EXPECT_THAT(Results.Completions,

clang-tools-extra/clangd/unittests/HeadersTests.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,17 @@ TEST_F(HeadersTest, ShortenIncludesInSearchPathBracketed) {
344344
EXPECT_EQ(calculate(BarHeader), "<sub/bar.h>");
345345
}
346346

347+
TEST_F(HeadersTest, ShortenIncludesInSearchPathBracketedFilterByFullPath) {
348+
// The filter receives the full path of the header, so it is able to filter by
349+
// the parent directory, even if it is part of the include search path
350+
AngledHeaders.push_back([](auto Path) {
351+
llvm::Regex Pattern("sub/.*");
352+
return Pattern.match(Path);
353+
});
354+
std::string BarHeader = testPath("sub/bar.h");
355+
EXPECT_EQ(calculate(BarHeader), "<bar.h>");
356+
}
357+
347358
TEST_F(HeadersTest, ShortenedIncludeNotInSearchPath) {
348359
std::string BarHeader =
349360
llvm::sys::path::convert_to_slash(testPath("sub-2/bar.h"));

clang-tools-extra/include-cleaner/include/clang-include-cleaner/Types.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ struct Header {
136136
}
137137
StringRef verbatim() const { return std::get<Verbatim>(Storage); }
138138

139-
/// For phiscal files, either absolute path or path relative to the execution
139+
/// For physical files, either absolute path or path relative to the execution
140140
/// root. Otherwise just the spelling without surrounding quotes/brackets.
141141
llvm::StringRef resolvedPath() const;
142142

0 commit comments

Comments
 (0)