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 @@ -70,6 +70,7 @@ CompiledFragmentImpl &Out; DiagnosticCallback Diagnostic; llvm::SourceMgr *SourceMgr; + Fragment::SourceInfo &FragmentSource; llvm::Optional compileRegex(const Located &Text) { std::string Anchored = "^(" + *Text + ")$"; @@ -145,11 +146,19 @@ } if (!PathMatch->empty()) { Out.Conditions.push_back( - [PathMatch(std::move(PathMatch))](const Params &P) { + [PathMatch(std::move(PathMatch)), PathSpec(FragmentSource.PathSpec), + FragmentDir(FragmentSource.FragmentDirectory)](const Params &P) { if (P.Path.empty()) return false; + llvm::StringRef Path = P.Path; + // In relative mode ignore paths rooted outside FragmentDirectory + // and strip the prefix for relative matching. + if (PathSpec == Fragment::SourceInfo::Relative && + !Path.consume_front(FragmentDir)) { + return false; + } return llvm::any_of(*PathMatch, [&](const llvm::Regex &RE) { - return RE.match(P.Path); + return RE.match(Path); }); }); } @@ -161,11 +170,21 @@ } if (!PathExclude->empty()) { Out.Conditions.push_back( - [PathExclude(std::move(PathExclude))](const Params &P) { + [PathExclude(std::move(PathExclude)), + PathSpec(FragmentSource.PathSpec), + FragmentDir(FragmentSource.FragmentDirectory)](const Params &P) { if (P.Path.empty()) return false; + llvm::StringRef Path = P.Path; + // In relative mode ignore paths rooted outside FragmentDirectory + // and strip the prefix for relative matching. + if (PathSpec == Fragment::SourceInfo::Relative) { + if (!Path.consume_front(FragmentDir)) + return true; + Path.consume_front("/"); + } return llvm::none_of(*PathExclude, [&](const llvm::Regex &RE) { - return RE.match(P.Path); + return RE.match(Path); }); }); } @@ -255,7 +274,8 @@ vlog("Config fragment: compiling {0}:{1} -> {2}", ConfigFile, LineCol.first, Result.get()); - FragmentCompiler{*Result, D, Source.Manager.get()}.compile(std::move(*this)); + FragmentCompiler{*Result, D, Source.Manager.get(), Source}.compile( + std::move(*this)); // Return as cheaply-copyable wrapper. return [Result(std::move(Result))](const Params &P, Config &C) { return (*Result)(P, C); 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,14 @@ /// The start of the original source for this fragment. /// Only valid if SourceManager is set. llvm::SMLoc Location; + /// Absolute path to directory containing the fragment. Doesn't contain the + /// trailing slash. + std::string FragmentDirectory; + /// Describes how paths specified in this fragment should be treated. + enum PathSpecification { + Absolute, + Relative, + } PathSpec = Absolute; }; 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,6 +13,7 @@ #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 @@ -31,7 +32,8 @@ // Called once we are sure we want to read the file. // REQUIRES: Cache keys are set. Mutex must be held. - void fillCacheFromDisk(llvm::vfs::FileSystem &FS, DiagnosticCallback DC) { + void fillCacheFromDisk(llvm::vfs::FileSystem &FS, DiagnosticCallback DC, + Fragment::SourceInfo::PathSpecification PathSpec) { CachedValue.clear(); auto Buf = FS.getBufferForFile(Path); @@ -49,10 +51,15 @@ MTime = {}; } + llvm::StringRef FragmentDir = llvm::sys::path::parent_path(Path); + FragmentDir.consume_back("/"); // 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.FragmentDirectory = FragmentDir.str(); + Fragment.Source.PathSpec = PathSpec; CachedValue.push_back(std::move(Fragment).compile(DC)); + } } public: @@ -68,6 +75,7 @@ // - allow latency-sensitive operations to skip revalidating the cache void read(const ThreadsafeFS &TFS, DiagnosticCallback DC, llvm::Optional FreshTime, + Fragment::SourceInfo::PathSpecification PathSpec, std::vector &Out) { std::lock_guard Lock(Mu); // We're going to update the cache and return whatever's in it. @@ -100,7 +108,7 @@ // OK, the file has actually changed. Update cache key, compute new value. Size = Stat->getSize(); MTime = Stat->getLastModificationTime(); - fillCacheFromDisk(*FS, DC); + fillCacheFromDisk(*FS, DC, PathSpec); } }; @@ -113,7 +121,7 @@ std::vector getFragments(const Params &P, DiagnosticCallback DC) const override { std::vector Result; - Cache.read(FS, DC, P.FreshTime, Result); + Cache.read(FS, DC, P.FreshTime, Fragment::SourceInfo::Absolute, Result); return Result; }; @@ -177,8 +185,10 @@ // 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) - Cache->read(FS, DC, P.FreshTime, Result); + for (FileConfigCache *Cache : Caches) { + Cache->read(FS, DC, P.FreshTime, Fragment::SourceInfo::Relative, + 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 @@ -116,6 +116,59 @@ "Invalid Background value 'Foo'. Valid values are Build, Skip."))); } +TEST_F(ConfigCompileTests, PathSpecMatch) { + const Fragment BaseFrag = [] { + Fragment Frag; + Frag.If.PathMatch.emplace_back(".*"); + Frag.Source.FragmentDirectory = "/baz"; + return Frag; + }(); + + Parm.Path = "/foo/bar.h"; + // * matches everything with absolute spec. + Frag = BaseFrag; + Frag.Source.PathSpec = Fragment::SourceInfo::Absolute; + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_TRUE(compileAndApply()); + // Relative should fail to match as /foo/bar.h doesn't reside under /baz/. + Frag = BaseFrag; + Frag.Source.PathSpec = Fragment::SourceInfo::Relative; + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_FALSE(compileAndApply()); + // Relative should pass now. + Frag = BaseFrag; + Parm.Path = "/baz/anything"; + Frag.Source.PathSpec = Fragment::SourceInfo::Relative; + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_TRUE(compileAndApply()); +} + +TEST_F(ConfigCompileTests, PathSpecExclude) { + const Fragment BaseFrag = [] { + Fragment Frag; + Frag.If.PathExclude.emplace_back(".*"); + Frag.Source.FragmentDirectory = "/baz"; + return Frag; + }(); + + Parm.Path = "/foo/bar.h"; + // * matches everything with absolute spec. + Frag = BaseFrag; + Frag.Source.PathSpec = Fragment::SourceInfo::Absolute; + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_FALSE(compileAndApply()); + // Relative should fail to match as /foo/bar.h doesn't reside under /baz/. + Frag = BaseFrag; + Frag.Source.PathSpec = Fragment::SourceInfo::Relative; + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_TRUE(compileAndApply()); + // Relative should pass now. + Frag = BaseFrag; + Parm.Path = "/baz/anything"; + Frag.Source.PathSpec = Fragment::SourceInfo::Relative; + ASSERT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_FALSE(compileAndApply()); +} } // 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