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 @@ -26,20 +26,34 @@ #include "CompileCommands.h" #include "Config.h" #include "ConfigFragment.h" +#include "ConfigProvider.h" #include "support/Logger.h" #include "support/Trace.h" #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 { namespace config { namespace { +// Returns an empty stringref if Path is not under FragmentDir. Returns Path +// as-is when FragmentDir is empty. +llvm::StringRef configRelative(llvm::StringRef Path, + llvm::StringRef FragmentDir) { + if (FragmentDir.empty()) + return Path; + if (!Path.consume_front(FragmentDir)) + return llvm::StringRef(); + return Path.empty() ? "." : Path; +} + struct CompiledFragmentImpl { // The independent conditions to check before using settings from this config. // The following fragment has *two* conditions: @@ -67,9 +81,14 @@ // Wrapper around condition compile() functions to reduce arg-passing. struct FragmentCompiler { + FragmentCompiler(CompiledFragmentImpl &Out, DiagnosticCallback D, + llvm::SourceMgr *SM) + : Out(Out), Diagnostic(D), SourceMgr(SM) {} CompiledFragmentImpl &Out; DiagnosticCallback Diagnostic; llvm::SourceMgr *SourceMgr; + // Normalized Fragment::SourceInfo::Directory. + std::string FragmentDirectory; llvm::Optional compileRegex(const Located &Text) { std::string Anchored = "^(" + *Text + ")$"; @@ -129,6 +148,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 +169,16 @@ } 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 = configRelative(P.Path, FragmentDir); + // Ignore the file if it is not nested under Fragment. + if (Path.empty()) + return false; return llvm::any_of(*PathMatch, [&](const llvm::Regex &RE) { - return RE.match(P.Path); + return RE.match(Path); }); }); } @@ -161,11 +190,16 @@ } 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 = configRelative(P.Path, FragmentDir); + // Ignore the file if it is not nested under Fragment. + if (Path.empty()) + return true; 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.h b/clang-tools-extra/clangd/ConfigProvider.h --- a/clang-tools-extra/clangd/ConfigProvider.h +++ b/clang-tools-extra/clangd/ConfigProvider.h @@ -18,6 +18,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGPROVIDER_H #include "llvm/ADT/FunctionExtras.h" +#include "llvm/ADT/StringRef.h" #include "llvm/Support/SMLoc.h" #include "llvm/Support/SourceMgr.h" #include @@ -61,8 +62,10 @@ public: virtual ~Provider() = default; - // Reads fragments from a single YAML file with a fixed path. - static std::unique_ptr fromYAMLFile(llvm::StringRef AbsPathPath, + /// Reads fragments from a single YAML file with a fixed path. If non-empty, + /// Directory will be used to resolve relative paths in the fragments. + static std::unique_ptr fromYAMLFile(llvm::StringRef AbsPath, + llvm::StringRef Directory, const ThreadsafeFS &); // Reads fragments from YAML files found relative to ancestors of Params.Path. // 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 = Directory; 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 Directory; // Retrieves up-to-date config fragments from disk. // A cached result may be reused if the mtime and size are unchanged. @@ -105,6 +111,7 @@ }; std::unique_ptr Provider::fromYAMLFile(llvm::StringRef AbsPath, + llvm::StringRef Directory, const ThreadsafeFS &FS) { class AbsFileProvider : public Provider { mutable FileConfigCache Cache; // threadsafe @@ -118,13 +125,16 @@ }; public: - AbsFileProvider(llvm::StringRef Path, const ThreadsafeFS &FS) : FS(FS) { + AbsFileProvider(llvm::StringRef Path, llvm::StringRef Directory, + const ThreadsafeFS &FS) + : FS(FS) { assert(llvm::sys::path::is_absolute(Path)); Cache.Path = Path.str(); + Cache.Directory = Directory.str(); } }; - return std::make_unique(AbsPath, FS); + return std::make_unique(AbsPath, Directory, FS); } std::unique_ptr @@ -170,6 +180,7 @@ llvm::SmallString<256> ConfigPath = Ancestor; path::append(ConfigPath, RelPath); R.first->second.Path = ConfigPath.str().str(); + R.first->second.Directory = Ancestor.str(); } Caches.push_back(&R.first->second); } @@ -177,8 +188,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/tool/ClangdMain.cpp b/clang-tools-extra/clangd/tool/ClangdMain.cpp --- a/clang-tools-extra/clangd/tool/ClangdMain.cpp +++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp @@ -790,7 +790,8 @@ if (llvm::sys::path::user_config_directory(UserConfig)) { llvm::sys::path::append(UserConfig, "clangd", "config.yaml"); vlog("User config file is {0}", UserConfig); - ProviderStack.push_back(config::Provider::fromYAMLFile(UserConfig, TFS)); + ProviderStack.push_back( + config::Provider::fromYAMLFile(UserConfig, /*Directory=*/"", TFS)); } else { elog("Couldn't determine user config file, not loading"); } 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" @@ -91,7 +92,7 @@ FS.Files["foo.yaml"] = AddFooWithErr; CapturedDiags Diags; - auto P = Provider::fromYAMLFile(testPath("foo.yaml"), FS); + auto P = Provider::fromYAMLFile(testPath("foo.yaml"), /*Directory=*/"", FS); auto Cfg = P->getConfig(Params(), Diags.callback()); EXPECT_THAT(Diags.Diagnostics, ElementsAre(DiagMessage("Unknown CompileFlags key Unknown"))); @@ -159,7 +160,7 @@ Params MustBeFresh; MustBeFresh.FreshTime = StartTime + std::chrono::hours(1); CapturedDiags Diags; - auto P = Provider::fromYAMLFile(testPath("foo.yaml"), FS); + auto P = Provider::fromYAMLFile(testPath("foo.yaml"), /*Directory=*/"", FS); // Initial query always reads, regardless of policy. FS.Files["foo.yaml"] = AddFooWithErr; @@ -187,6 +188,37 @@ 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"), /*Directory=*/"", 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