[clangd] Treat paths case-insensitively depending on the platform
Path{Match,Exclude} and MountPoint were checking paths case-sensitively
on all platforms, as with other features, this was causing problems on
windows. Since users can have capital drive letters on config files, but
editors might lower-case them.
This patch addresses that issue by:
- Creating regexes with case-insensitive matching on those platforms.
- Introducing a new pathIsAncestor helper, which performs checks in a
case-correct manner where needed.
Differential Revision: https://reviews.llvm.org/D96690
This commit is contained in:
@@ -31,6 +31,7 @@
|
||||
#include "Features.inc"
|
||||
#include "TidyProvider.h"
|
||||
#include "support/Logger.h"
|
||||
#include "support/Path.h"
|
||||
#include "support/Trace.h"
|
||||
#include "llvm/ADT/None.h"
|
||||
#include "llvm/ADT/Optional.h"
|
||||
@@ -101,9 +102,11 @@ struct FragmentCompiler {
|
||||
// Normalized Fragment::SourceInfo::Directory.
|
||||
std::string FragmentDirectory;
|
||||
|
||||
llvm::Optional<llvm::Regex> compileRegex(const Located<std::string> &Text) {
|
||||
llvm::Optional<llvm::Regex>
|
||||
compileRegex(const Located<std::string> &Text,
|
||||
llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags) {
|
||||
std::string Anchored = "^(" + *Text + ")$";
|
||||
llvm::Regex Result(Anchored);
|
||||
llvm::Regex Result(Anchored, Flags);
|
||||
std::string RegexError;
|
||||
if (!Result.isValid(RegexError)) {
|
||||
diag(Error, "Invalid regex " + Anchored + ": " + RegexError, Text.Range);
|
||||
@@ -195,9 +198,15 @@ struct FragmentCompiler {
|
||||
if (F.HasUnrecognizedCondition)
|
||||
Out.Conditions.push_back([&](const Params &) { return false; });
|
||||
|
||||
#ifdef CLANGD_PATH_CASE_INSENSITIVE
|
||||
llvm::Regex::RegexFlags Flags = llvm::Regex::IgnoreCase;
|
||||
#else
|
||||
llvm::Regex::RegexFlags Flags = llvm::Regex::NoFlags;
|
||||
#endif
|
||||
|
||||
auto PathMatch = std::make_unique<std::vector<llvm::Regex>>();
|
||||
for (auto &Entry : F.PathMatch) {
|
||||
if (auto RE = compileRegex(Entry))
|
||||
if (auto RE = compileRegex(Entry, Flags))
|
||||
PathMatch->push_back(std::move(*RE));
|
||||
}
|
||||
if (!PathMatch->empty()) {
|
||||
@@ -218,7 +227,7 @@ struct FragmentCompiler {
|
||||
|
||||
auto PathExclude = std::make_unique<std::vector<llvm::Regex>>();
|
||||
for (auto &Entry : F.PathExclude) {
|
||||
if (auto RE = compileRegex(Entry))
|
||||
if (auto RE = compileRegex(Entry, Flags))
|
||||
PathExclude->push_back(std::move(*RE));
|
||||
}
|
||||
if (!PathExclude->empty()) {
|
||||
@@ -349,7 +358,8 @@ struct FragmentCompiler {
|
||||
return;
|
||||
Spec.MountPoint = std::move(*AbsPath);
|
||||
Out.Apply.push_back([Spec(std::move(Spec))](const Params &P, Config &C) {
|
||||
if (!P.Path.startswith(Spec.MountPoint))
|
||||
if (P.Path.empty() || !pathStartsWith(Spec.MountPoint, P.Path,
|
||||
llvm::sys::path::Style::posix))
|
||||
return;
|
||||
C.Index.External = Spec;
|
||||
// Disable background indexing for the files under the mountpoint.
|
||||
|
||||
@@ -7,24 +7,33 @@
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#include "support/Path.h"
|
||||
#include "llvm/Support/Path.h"
|
||||
namespace clang {
|
||||
namespace clangd {
|
||||
|
||||
std::string maybeCaseFoldPath(PathRef Path) {
|
||||
#if defined(_WIN32) || defined(__APPLE__)
|
||||
return Path.lower();
|
||||
#else
|
||||
return std::string(Path);
|
||||
#endif
|
||||
}
|
||||
#ifdef CLANGD_PATH_CASE_INSENSITIVE
|
||||
std::string maybeCaseFoldPath(PathRef Path) { return Path.lower(); }
|
||||
bool pathEqual(PathRef A, PathRef B) { return A.equals_lower(B); }
|
||||
#else // NOT CLANGD_PATH_CASE_INSENSITIVE
|
||||
std::string maybeCaseFoldPath(PathRef Path) { return Path.str(); }
|
||||
bool pathEqual(PathRef A, PathRef B) { return A == B; }
|
||||
#endif // CLANGD_PATH_CASE_INSENSITIVE
|
||||
|
||||
bool pathEqual(PathRef A, PathRef B) {
|
||||
#if defined(_WIN32) || defined(__APPLE__)
|
||||
return A.equals_lower(B);
|
||||
#else
|
||||
return A == B;
|
||||
#endif
|
||||
bool pathStartsWith(PathRef Ancestor, PathRef Path,
|
||||
llvm::sys::path::Style Style) {
|
||||
assert(llvm::sys::path::is_absolute(Ancestor, Style) &&
|
||||
llvm::sys::path::is_absolute(Path, Style));
|
||||
// If ancestor ends with a separator drop that, so that we can match /foo/ as
|
||||
// a parent of /foo.
|
||||
if (llvm::sys::path::is_separator(Ancestor.back(), Style))
|
||||
Ancestor = Ancestor.drop_back();
|
||||
// Ensure Path starts with Ancestor.
|
||||
if (!pathEqual(Ancestor, Path.take_front(Ancestor.size())))
|
||||
return false;
|
||||
Path = Path.drop_front(Ancestor.size());
|
||||
// Then make sure either two paths are equal or Path has a separator
|
||||
// afterwards.
|
||||
return Path.empty() || llvm::sys::path::is_separator(Path.front(), Style);
|
||||
}
|
||||
|
||||
} // namespace clangd
|
||||
} // namespace clang
|
||||
|
||||
@@ -10,8 +10,14 @@
|
||||
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_SUPPORT_PATH_H
|
||||
|
||||
#include "llvm/ADT/StringRef.h"
|
||||
#include "llvm/Support/Path.h"
|
||||
#include <string>
|
||||
|
||||
/// Whether current platform treats paths case insensitively.
|
||||
#if defined(_WIN32) || defined(__APPLE__)
|
||||
#define CLANGD_PATH_CASE_INSENSITIVE
|
||||
#endif
|
||||
|
||||
namespace clang {
|
||||
namespace clangd {
|
||||
|
||||
@@ -28,6 +34,12 @@ using PathRef = llvm::StringRef;
|
||||
std::string maybeCaseFoldPath(PathRef Path);
|
||||
bool pathEqual(PathRef, PathRef);
|
||||
|
||||
/// Checks if \p Ancestor is a proper ancestor of \p Path. This is just a
|
||||
/// smarter lexical prefix match, e.g: foo/bar/baz doesn't start with foo/./bar.
|
||||
/// Both \p Ancestor and \p Path must be absolute.
|
||||
bool pathStartsWith(
|
||||
PathRef Ancestor, PathRef Path,
|
||||
llvm::sys::path::Style Style = llvm::sys::path::Style::native);
|
||||
} // namespace clangd
|
||||
} // namespace clang
|
||||
|
||||
|
||||
@@ -105,6 +105,7 @@ add_unittest(ClangdUnitTests ClangdTests
|
||||
support/FunctionTests.cpp
|
||||
support/MarkupTests.cpp
|
||||
support/MemoryTreeTests.cpp
|
||||
support/PathTests.cpp
|
||||
support/ThreadingTests.cpp
|
||||
support/TestTracer.cpp
|
||||
support/TraceTests.cpp
|
||||
|
||||
@@ -99,6 +99,25 @@ TEST_F(ConfigCompileTests, Condition) {
|
||||
Frag.If.PathMatch.emplace_back("ba*r");
|
||||
EXPECT_FALSE(compileAndApply());
|
||||
EXPECT_THAT(Diags.Diagnostics, IsEmpty());
|
||||
|
||||
// Only matches case-insensitively.
|
||||
Frag = {};
|
||||
Frag.If.PathMatch.emplace_back("B.*R");
|
||||
EXPECT_THAT(Diags.Diagnostics, IsEmpty());
|
||||
#ifdef CLANGD_PATH_CASE_INSENSITIVE
|
||||
EXPECT_TRUE(compileAndApply());
|
||||
#else
|
||||
EXPECT_FALSE(compileAndApply());
|
||||
#endif
|
||||
|
||||
Frag = {};
|
||||
Frag.If.PathExclude.emplace_back("B.*R");
|
||||
EXPECT_THAT(Diags.Diagnostics, IsEmpty());
|
||||
#ifdef CLANGD_PATH_CASE_INSENSITIVE
|
||||
EXPECT_FALSE(compileAndApply());
|
||||
#else
|
||||
EXPECT_TRUE(compileAndApply());
|
||||
#endif
|
||||
}
|
||||
|
||||
TEST_F(ConfigCompileTests, CompileCommands) {
|
||||
@@ -406,6 +425,23 @@ TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
|
||||
ASSERT_THAT(Diags.Diagnostics, IsEmpty());
|
||||
ASSERT_TRUE(Conf.Index.External);
|
||||
EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
|
||||
|
||||
// Only matches case-insensitively.
|
||||
BazPath = testPath("fOo/baz.h", llvm::sys::path::Style::posix);
|
||||
BazPath = llvm::sys::path::convert_to_slash(BazPath);
|
||||
Parm.Path = BazPath;
|
||||
|
||||
FooPath = testPath("FOO/", llvm::sys::path::Style::posix);
|
||||
FooPath = llvm::sys::path::convert_to_slash(FooPath);
|
||||
Frag = GetFrag("", FooPath.c_str());
|
||||
compileAndApply();
|
||||
ASSERT_THAT(Diags.Diagnostics, IsEmpty());
|
||||
#ifdef CLANGD_PATH_CASE_INSENSITIVE
|
||||
ASSERT_TRUE(Conf.Index.External);
|
||||
EXPECT_THAT(Conf.Index.External->MountPoint, FooPath);
|
||||
#else
|
||||
ASSERT_FALSE(Conf.Index.External);
|
||||
#endif
|
||||
}
|
||||
} // namespace
|
||||
} // namespace config
|
||||
|
||||
@@ -1125,7 +1125,7 @@ TEST(RenameTest, IndexMergeMainFile) {
|
||||
EXPECT_THAT(Results.GlobalChanges.keys(),
|
||||
UnorderedElementsAre(Main, testPath("other.cc")));
|
||||
|
||||
#if defined(_WIN32) || defined(__APPLE__)
|
||||
#ifdef CLANGD_PATH_CASE_INSENSITIVE
|
||||
// On case-insensitive systems, no duplicates if AST vs index case differs.
|
||||
// https://github.com/clangd/clangd/issues/665
|
||||
TU.Filename = "MAIN.CC";
|
||||
|
||||
36
clang-tools-extra/clangd/unittests/support/PathTests.cpp
Normal file
36
clang-tools-extra/clangd/unittests/support/PathTests.cpp
Normal file
@@ -0,0 +1,36 @@
|
||||
//===-- PathTests.cpp -------------------------------------------*- C++ -*-===//
|
||||
//
|
||||
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
||||
// See https://llvm.org/LICENSE.txt for license information.
|
||||
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
||||
//
|
||||
//===----------------------------------------------------------------------===//
|
||||
|
||||
#include "support/Path.h"
|
||||
#include "gmock/gmock.h"
|
||||
#include "gtest/gtest.h"
|
||||
|
||||
namespace clang {
|
||||
namespace clangd {
|
||||
namespace {
|
||||
TEST(PathTests, IsAncestor) {
|
||||
EXPECT_TRUE(pathStartsWith("/foo", "/foo"));
|
||||
EXPECT_TRUE(pathStartsWith("/foo/", "/foo"));
|
||||
|
||||
EXPECT_FALSE(pathStartsWith("/foo", "/fooz"));
|
||||
EXPECT_FALSE(pathStartsWith("/foo/", "/fooz"));
|
||||
|
||||
EXPECT_TRUE(pathStartsWith("/foo", "/foo/bar"));
|
||||
EXPECT_TRUE(pathStartsWith("/foo/", "/foo/bar"));
|
||||
|
||||
#ifdef CLANGD_PATH_CASE_INSENSITIVE
|
||||
EXPECT_TRUE(pathStartsWith("/fOo", "/foo/bar"));
|
||||
EXPECT_TRUE(pathStartsWith("/foo", "/fOo/bar"));
|
||||
#else
|
||||
EXPECT_FALSE(pathStartsWith("/fOo", "/foo/bar"));
|
||||
EXPECT_FALSE(pathStartsWith("/foo", "/fOo/bar"));
|
||||
#endif
|
||||
}
|
||||
} // namespace
|
||||
} // namespace clangd
|
||||
} // namespace clang
|
||||
Reference in New Issue
Block a user