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 @@ -31,9 +31,11 @@ #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" #include "llvm/ADT/StringSwitch.h" +#include "llvm/Support/Path.h" #include "llvm/Support/Regex.h" #include "llvm/Support/SMLoc.h" #include "llvm/Support/SourceMgr.h" +#include namespace clang { namespace clangd { @@ -70,6 +72,9 @@ CompiledFragmentImpl &Out; DiagnosticCallback Diagnostic; llvm::SourceMgr *SourceMgr; + // Directory containing the fragment. Absolute path with forward slashes, with + // a trailing slash or empty for user-config fragments. + std::string FragmentDirectory; llvm::Optional compileRegex(const Located &Text) { std::string Anchored = "^(" + *Text + ")$"; @@ -129,6 +134,11 @@ } void compile(Fragment &&F) { + if (!F.Source.Directory.empty()) { + FragmentDirectory = llvm::sys::path::convert_to_slash(F.Source.Directory); + if (FragmentDirectory.back() != '/') + FragmentDirectory += '/'; + } compile(std::move(F.If)); compile(std::move(F.CompileFlags)); compile(std::move(F.Index)); @@ -145,11 +155,19 @@ } if (!PathMatch->empty()) { Out.Conditions.push_back( - [PathMatch(std::move(PathMatch))](const Params &P) { + [PathMatch(std::move(PathMatch)), + FragmentDir(FragmentDirectory)](const Params &P) { if (P.Path.empty()) return false; + llvm::StringRef Path = P.Path; + // Ignore the file if it is not nested under Fragment and strip the + // prefix. + if (!Path.consume_front(FragmentDir)) + return false; + if (!FragmentDir.empty()) + Path.consume_front("/"); return llvm::any_of(*PathMatch, [&](const llvm::Regex &RE) { - return RE.match(P.Path); + return RE.match(Path); }); }); } @@ -161,11 +179,19 @@ } if (!PathExclude->empty()) { Out.Conditions.push_back( - [PathExclude(std::move(PathExclude))](const Params &P) { + [PathExclude(std::move(PathExclude)), + FragmentDir(FragmentDirectory)](const Params &P) { if (P.Path.empty()) return false; + llvm::StringRef Path = P.Path; + // Ignore the file if it is not nested under Fragment and strip the + // prefix. + if (!Path.consume_front(FragmentDir)) + return true; + if (!FragmentDir.empty()) + Path.consume_front("/"); return llvm::none_of(*PathExclude, [&](const llvm::Regex &RE) { - return RE.match(P.Path); + return RE.match(Path); }); }); } 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 @@ -91,6 +91,9 @@ /// The start of the original source for this fragment. /// Only valid if SourceManager is set. llvm::SMLoc Location; + /// Absolute path to directory the fragment is associated with. Relative + /// paths mentioned in the fragment are resolved against this. + std::string Directory; }; SourceInfo Source; diff --git a/clang-tools-extra/clangd/ConfigProvider.cpp b/clang-tools-extra/clangd/ConfigProvider.cpp --- a/clang-tools-extra/clangd/ConfigProvider.cpp +++ b/clang-tools-extra/clangd/ConfigProvider.cpp @@ -13,9 +13,11 @@ #include "support/Trace.h" #include "llvm/ADT/ScopeExit.h" #include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/Path.h" #include #include +#include namespace clang { namespace clangd { @@ -51,14 +53,18 @@ // Finally parse and compile the actual fragments. for (auto &Fragment : - Fragment::parseYAML(Buf->get()->getBuffer(), Path, DC)) + Fragment::parseYAML(Buf->get()->getBuffer(), Path, DC)) { + Fragment.Source.Directory = Dir; CachedValue.push_back(std::move(Fragment).compile(DC)); + } } public: // Must be set before the cache is used. Not a constructor param to allow // computing ancestor-relative paths to be deferred. std::string Path; + // Directory associated with this fragment. + std::string Dir; // Retrieves up-to-date config fragments from disk. // A cached result may be reused if the mtime and size are unchanged. @@ -170,6 +176,7 @@ llvm::SmallString<256> ConfigPath = Ancestor; path::append(ConfigPath, RelPath); R.first->second.Path = ConfigPath.str().str(); + R.first->second.Dir = Ancestor.str(); } Caches.push_back(&R.first->second); } @@ -177,8 +184,9 @@ // Finally query each individual file. // This will take a (per-file) lock for each file that actually exists. std::vector Result; - for (FileConfigCache *Cache : Caches) + for (FileConfigCache *Cache : Caches) { Cache->read(FS, DC, P.FreshTime, Result); + } return Result; }; 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 @@ -9,8 +9,12 @@ #include "Config.h" #include "ConfigFragment.h" #include "ConfigTesting.h" +#include "TestFS.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Path.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include namespace clang { namespace clangd { @@ -116,6 +120,62 @@ "Invalid Background value 'Foo'. Valid values are Build, Skip."))); } +TEST_F(ConfigCompileTests, PathSpecMatch) { + auto BarPath = llvm::sys::path::convert_to_slash(testPath("foo/bar.h")); + Parm.Path = BarPath; + + struct { + std::string Directory; + std::string PathSpec; + bool ShouldMatch; + } Cases[] = { + { + // Absolute path matches. + "", + llvm::sys::path::convert_to_slash(testPath("foo/bar.h")), + true, + }, + { + // Absolute path fails. + "", + llvm::sys::path::convert_to_slash(testPath("bar/bar.h")), + false, + }, + { + // Relative should fail to match as /foo/bar.h doesn't reside under + // /baz/. + testPath("baz"), + "bar\\.h", + false, + }, + { + // Relative should pass with /foo as directory. + testPath("foo"), + "bar\\.h", + true, + }, + }; + + // PathMatch + for (const auto &Case : Cases) { + Frag = {}; + Frag.If.PathMatch.emplace_back(Case.PathSpec); + Frag.Source.Directory = Case.Directory; + EXPECT_EQ(compileAndApply(), Case.ShouldMatch); + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + } + + // PathEclude + for (const auto &Case : Cases) { + SCOPED_TRACE(Case.Directory); + SCOPED_TRACE(Case.PathSpec); + Frag = {}; + Frag.If.PathExclude.emplace_back(Case.PathSpec); + Frag.Source.Directory = Case.Directory; + EXPECT_NE(compileAndApply(), Case.ShouldMatch); + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + } +} } // namespace } // namespace config } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp --- a/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigProviderTests.cpp @@ -10,6 +10,7 @@ #include "ConfigProvider.h" #include "ConfigTesting.h" #include "TestFS.h" +#include "llvm/Support/Path.h" #include "llvm/Support/SourceMgr.h" #include "gmock/gmock.h" #include "gtest/gtest.h" @@ -187,6 +188,36 @@ EXPECT_THAT(getAddedArgs(Cfg), IsEmpty()); } +TEST(ProviderTest, SourceInfo) { + MockFS FS; + + FS.Files["baz/foo.yaml"] = R"yaml( +If: + PathMatch: .* + PathExclude: bar.h +CompileFlags: + Add: bar +)yaml"; + const auto BarPath = testPath("baz/bar.h", llvm::sys::path::Style::posix); + CapturedDiags Diags; + Params Bar; + Bar.Path = BarPath; + + // This should be an absolute match/exclude hence baz/bar.h should not be + // excluded. + auto P = Provider::fromYAMLFile(testPath("baz/foo.yaml"), FS); + auto Cfg = P->getConfig(Bar, Diags.callback()); + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_THAT(getAddedArgs(Cfg), ElementsAre("bar")); + Diags.Diagnostics.clear(); + + // This should be a relative match/exclude hence baz/bar.h should be excluded. + P = Provider::fromAncestorRelativeYAMLFiles("foo.yaml", FS); + Cfg = P->getConfig(Bar, Diags.callback()); + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_THAT(getAddedArgs(Cfg), IsEmpty()); + Diags.Diagnostics.clear(); +} } // namespace } // namespace config } // namespace clangd