[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:
Kadir Cetinkaya
2021-02-15 09:00:49 +01:00
parent acfab44eeb
commit ecea7218fb
7 changed files with 124 additions and 20 deletions

View File

@@ -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.

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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

View File

@@ -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";

View 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