From e9a5a2f10ca1d04fbf06b50dcb052711b8ddb1e7 Mon Sep 17 00:00:00 2001 From: Haojian Wu Date: Mon, 9 Apr 2018 15:09:44 +0000 Subject: [PATCH] [clangd] Allow using customized include path in URI. Summary: Calculating the include path from absolute file path does not always work for all build system, e.g. bazel uses symlink as the build working directory. The absolute file path from editor and clang is diverged from each other. We need to address it properly in build sysmtem integration. This patch worksarounds the issue by providing a hook in URI which allows clients to provide their customized include path. Reviewers: sammccall Subscribers: klimek, ilya-biryukov, jkorous-apple, ioeric, MaskRay, cfe-commits Differential Revision: https://reviews.llvm.org/D45426 llvm-svn: 329578 --- clang-tools-extra/clangd/ClangdServer.cpp | 7 +++++ clang-tools-extra/clangd/URI.cpp | 7 +++++ clang-tools-extra/clangd/URI.h | 17 ++++++++++++ .../unittests/clangd/ClangdTests.cpp | 26 +++++++++++++++++++ .../unittests/clangd/TestScheme.h | 0 5 files changed, 57 insertions(+) create mode 100644 clang-tools-extra/unittests/clangd/TestScheme.h diff --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp index dbb3ce4930668..e73834be32daf 100644 --- a/clang-tools-extra/clangd/ClangdServer.cpp +++ b/clang-tools-extra/clangd/ClangdServer.cpp @@ -286,6 +286,13 @@ static llvm::Expected toHeaderFile(StringRef Header, auto U = URI::parse(Header); if (!U) return U.takeError(); + + auto IncludePath = URI::includeSpelling(*U); + if (!IncludePath) + return IncludePath.takeError(); + if (!IncludePath->empty()) + return HeaderFile{std::move(*IncludePath), /*Verbatim=*/true}; + auto Resolved = URI::resolve(*U, HintPath); if (!Resolved) return Resolved.takeError(); diff --git a/clang-tools-extra/clangd/URI.cpp b/clang-tools-extra/clangd/URI.cpp index c0e10cef1735a..a722640890dc8 100644 --- a/clang-tools-extra/clangd/URI.cpp +++ b/clang-tools-extra/clangd/URI.cpp @@ -196,5 +196,12 @@ llvm::Expected URI::resolve(const URI &Uri, return S->get()->getAbsolutePath(Uri.Authority, Uri.Body, HintPath); } +llvm::Expected URI::includeSpelling(const URI &Uri) { + auto S = findSchemeByName(Uri.Scheme); + if (!S) + return S.takeError(); + return S->get()->getIncludeSpelling(Uri); +} + } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/URI.h b/clang-tools-extra/clangd/URI.h index 51b23e37da025..ff4bc2d31fb01 100644 --- a/clang-tools-extra/clangd/URI.h +++ b/clang-tools-extra/clangd/URI.h @@ -60,6 +60,16 @@ class URI { static llvm::Expected resolve(const URI &U, llvm::StringRef HintPath = ""); + /// Gets the preferred spelling of this file for #include, if there is one, + /// e.g. , "path/to/x.h". + /// + /// This allows URI schemas to provide their customized include paths. + /// + /// Returns an empty string if normal include-shortening based on the absolute + /// path should be used. + /// Fails if the URI is not valid in the schema. + static llvm::Expected includeSpelling(const URI &U); + friend bool operator==(const URI &LHS, const URI &RHS) { return std::tie(LHS.Scheme, LHS.Authority, LHS.Body) == std::tie(RHS.Scheme, RHS.Authority, RHS.Body); @@ -94,6 +104,13 @@ class URIScheme { virtual llvm::Expected uriFromAbsolutePath(llvm::StringRef AbsolutePath) const = 0; + + /// Returns the include path of the file (e.g. , "path"), which can be + /// #included directly. See URI::includeSpelling for details. + virtual llvm::Expected + getIncludeSpelling(const URI& U) const { + return ""; // no customized include path for this scheme. + } }; /// By default, a "file" scheme is supported where URI paths are always absolute diff --git a/clang-tools-extra/unittests/clangd/ClangdTests.cpp b/clang-tools-extra/unittests/clangd/ClangdTests.cpp index b0e56ba78e724..4704fad3a3ddc 100644 --- a/clang-tools-extra/unittests/clangd/ClangdTests.cpp +++ b/clang-tools-extra/unittests/clangd/ClangdTests.cpp @@ -152,6 +152,28 @@ class ClangdVFSTest : public ::testing::Test { } }; +constexpr const char* ClangdTestScheme = "ClangdTests"; +class TestURIScheme : public URIScheme { +public: + llvm::Expected + getAbsolutePath(llvm::StringRef /*Authority*/, llvm::StringRef Body, + llvm::StringRef /*HintPath*/) const override { + llvm_unreachable("ClangdTests never makes absolute path."); + } + + llvm::Expected + uriFromAbsolutePath(llvm::StringRef AbsolutePath) const override { + llvm_unreachable("ClangdTest never creates a test URI."); + } + + llvm::Expected getIncludeSpelling(const URI &U) const override { + return ("\"" + U.body().trim("/") + "\"").str(); + } +}; + +static URISchemeRegistry::Add + X(ClangdTestScheme, "Test scheme for ClangdTests."); + TEST_F(ClangdVFSTest, Parse) { // FIXME: figure out a stable format for AST dumps, so that we can check the // output of the dump itself is equal to the expected one, not just that it's @@ -961,6 +983,10 @@ void f() {} /*Preferred=*/"", "")); EXPECT_TRUE(Inserted(OriginalHeader, PreferredHeader, "\"Y.h\"")); EXPECT_TRUE(Inserted("", PreferredHeader, "\"Y.h\"")); + auto TestURIHeader = + URI::parse(llvm::formatv("{0}:///x/y/z.h", ClangdTestScheme).str()); + EXPECT_TRUE(static_cast(TestURIHeader)); + EXPECT_TRUE(Inserted(TestURIHeader->toString(), "", "\"x/y/z.h\"")); // Check that includes are sorted. const auto Expected = R"cpp( diff --git a/clang-tools-extra/unittests/clangd/TestScheme.h b/clang-tools-extra/unittests/clangd/TestScheme.h new file mode 100644 index 0000000000000..e69de29bb2d1d