[clang-tools-extra] 079ef78 - Revert "[clangd] Implement "textDocument/documentLink" protocol support"

Dmitri Gribenko via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 16 06:28:13 PST 2019


Author: Dmitri Gribenko
Date: 2019-12-16T15:21:51+01:00
New Revision: 079ef783dd5530b5f87beefe624b9179547ded7e

URL: https://github.com/llvm/llvm-project/commit/079ef783dd5530b5f87beefe624b9179547ded7e
DIFF: https://github.com/llvm/llvm-project/commit/079ef783dd5530b5f87beefe624b9179547ded7e.diff

LOG: Revert "[clangd] Implement "textDocument/documentLink" protocol support"

This reverts commit d6417f5584aa7673fa0212029a96cc9cacb1aad5. The tests
depend on builtin headers, which is not intentionally supported in
clangd tests; these tests are broken in some build environments.

Added: 
    

Modified: 
    clang-tools-extra/clangd/ClangdLSPServer.cpp
    clang-tools-extra/clangd/ClangdLSPServer.h
    clang-tools-extra/clangd/ClangdServer.cpp
    clang-tools-extra/clangd/ClangdServer.h
    clang-tools-extra/clangd/Protocol.cpp
    clang-tools-extra/clangd/Protocol.h
    clang-tools-extra/clangd/XRefs.cpp
    clang-tools-extra/clangd/XRefs.h
    clang-tools-extra/clangd/test/initialize-params.test
    clang-tools-extra/clangd/unittests/XRefsTests.cpp

Removed: 
    clang-tools-extra/clangd/test/document-link.test


################################################################################
diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 69b4308a1c9e..bd9c31d3844b 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -566,10 +566,6 @@ void ClangdLSPServer::onInitialize(const InitializeParams &Params,
             {"declarationProvider", true},
             {"definitionProvider", true},
             {"documentHighlightProvider", true},
-            {"documentLinkProvider",
-             llvm::json::Object{
-                 {"resolveProvider", false},
-             }},
             {"hoverProvider", true},
             {"renameProvider", std::move(RenameProvider)},
             {"selectionRangeProvider", true},
@@ -1204,25 +1200,6 @@ void ClangdLSPServer::onSelectionRange(
       });
 }
 
-void ClangdLSPServer::onDocumentLink(
-    const DocumentLinkParams &Params,
-    Callback<std::vector<DocumentLink>> Reply) {
-
-  // TODO(forster): This currently resolves all targets eagerly. This is slow,
-  // because it blocks on the preamble/AST being built. We could respond to the
-  // request faster by using string matching or the lexer to find the includes
-  // and resolving the targets lazily.
-  Server->documentLinks(
-      Params.textDocument.uri.file(),
-      [Reply = std::move(Reply)](
-          llvm::Expected<std::vector<DocumentLink>> Links) mutable {
-        if (!Links) {
-          return Reply(Links.takeError());
-        }
-        return Reply(std::move(Links));
-      });
-}
-
 ClangdLSPServer::ClangdLSPServer(
     class Transport &Transp, const FileSystemProvider &FSProvider,
     const clangd::CodeCompleteOptions &CCOpts,
@@ -1266,7 +1243,6 @@ ClangdLSPServer::ClangdLSPServer(
   MsgHandler->bind("textDocument/typeHierarchy", &ClangdLSPServer::onTypeHierarchy);
   MsgHandler->bind("typeHierarchy/resolve", &ClangdLSPServer::onResolveTypeHierarchy);
   MsgHandler->bind("textDocument/selectionRange", &ClangdLSPServer::onSelectionRange);
-  MsgHandler->bind("textDocument/documentLink", &ClangdLSPServer::onDocumentLink);
   // clang-format on
 }
 

diff  --git a/clang-tools-extra/clangd/ClangdLSPServer.h b/clang-tools-extra/clangd/ClangdLSPServer.h
index 9650b6a7dbb2..f1ed317f6bad 100644
--- a/clang-tools-extra/clangd/ClangdLSPServer.h
+++ b/clang-tools-extra/clangd/ClangdLSPServer.h
@@ -111,8 +111,6 @@ class ClangdLSPServer : private DiagnosticsConsumer {
                     Callback<std::vector<SymbolDetails>>);
   void onSelectionRange(const SelectionRangeParams &,
                         Callback<std::vector<SelectionRange>>);
-  void onDocumentLink(const DocumentLinkParams &,
-                      Callback<std::vector<DocumentLink>>);
 
   std::vector<Fix> getFixes(StringRef File, const clangd::Diagnostic &D);
 

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp b/clang-tools-extra/clangd/ClangdServer.cpp
index c71a8a0d566b..40fa28ae140a 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -611,17 +611,6 @@ void ClangdServer::semanticRanges(PathRef File, Position Pos,
   WorkScheduler.runWithAST("SemanticRanges", File, std::move(Action));
 }
 
-void ClangdServer::documentLinks(PathRef File,
-                                 Callback<std::vector<DocumentLink>> CB) {
-  auto Action =
-      [CB = std::move(CB)](llvm::Expected<InputsAndAST> InpAST) mutable {
-        if (!InpAST)
-          return CB(InpAST.takeError());
-        CB(clangd::getDocumentLinks(InpAST->AST));
-      };
-  WorkScheduler.runWithAST("DocumentLinks", File, std::move(Action));
-}
-
 std::vector<std::pair<Path, std::size_t>>
 ClangdServer::getUsedBytesPerFile() const {
   return WorkScheduler.getUsedBytesPerFile();

diff  --git a/clang-tools-extra/clangd/ClangdServer.h b/clang-tools-extra/clangd/ClangdServer.h
index 4574d6c35034..499340808765 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -287,9 +287,6 @@ class ClangdServer {
   void semanticRanges(PathRef File, Position Pos,
                       Callback<std::vector<Range>> CB);
 
-  /// Get all document links in a file.
-  void documentLinks(PathRef File, Callback<std::vector<DocumentLink>> CB);
- 
   /// Returns estimated memory usage for each of the currently open files.
   /// The order of results is unspecified.
   /// Overall memory usage of clangd may be significantly more than reported

diff  --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index 29ceb1da5456..25826bd5a11d 100644
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -1087,18 +1087,5 @@ llvm::json::Value toJSON(const SelectionRange &Out) {
   }
   return llvm::json::Object{{"range", Out.range}};
 }
-
-bool fromJSON(const llvm::json::Value &Params, DocumentLinkParams &R) {
-  llvm::json::ObjectMapper O(Params);
-  return O && O.map("textDocument", R.textDocument);
-}
-
-llvm::json::Value toJSON(const DocumentLink &DocumentLink) {
-  return llvm::json::Object{
-      {"range", DocumentLink.range},
-      {"target", DocumentLink.target},
-  };
-}
-
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index 1ccfa587bf87..f110292b091b 100644
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -1250,39 +1250,6 @@ struct SelectionRange {
 };
 llvm::json::Value toJSON(const SelectionRange &);
 
-/// Parameters for the document link request.
-struct DocumentLinkParams {
-  /// The document to provide document links for.
-  TextDocumentIdentifier textDocument;
-};
-bool fromJSON(const llvm::json::Value &, DocumentLinkParams &);
-
-/// A range in a text document that links to an internal or external resource,
-/// like another text document or a web site.
-struct DocumentLink {
-  /// The range this link applies to.
-  Range range;
-
-  /// The uri this link points to. If missing a resolve request is sent later.
-  URIForFile target;
-
-  // TODO(forster): The following optional fields defined by the language
-  // server protocol are unsupported:
-  //
-  // data?: any - A data entry field that is preserved on a document link
-  //              between a DocumentLinkRequest and a
-  //              DocumentLinkResolveRequest.
-
-  friend bool operator==(const DocumentLink &LHS, const DocumentLink &RHS) {
-    return LHS.range == RHS.range && LHS.target == RHS.target;
-  }
-
-  friend bool operator!=(const DocumentLink &LHS, const DocumentLink &RHS) {
-    return !(LHS == RHS);
-  }
-};
-llvm::json::Value toJSON(const DocumentLink &DocumentLink);
-
 } // namespace clangd
 } // namespace clang
 

diff  --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 1ca388f49f86..b16e7080aa68 100644
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -166,26 +166,6 @@ llvm::Optional<Location> makeLocation(ASTContext &AST, SourceLocation TokLoc,
 
 } // namespace
 
-std::vector<DocumentLink> getDocumentLinks(ParsedAST &AST) {
-  const auto &SM = AST.getSourceManager();
-  auto MainFilePath =
-      getCanonicalPath(SM.getFileEntryForID(SM.getMainFileID()), SM);
-  if (!MainFilePath) {
-    elog("Failed to get a path for the main file, so no links");
-    return {};
-  }
-
-  std::vector<DocumentLink> Result;
-  for (auto &Inc : AST.getIncludeStructure().MainFileIncludes) {
-    if (!Inc.Resolved.empty()) {
-      Result.push_back(DocumentLink(
-          {Inc.R, URIForFile::canonicalize(Inc.Resolved, *MainFilePath)}));
-    }
-  }
-
-  return Result;
-}
-
 std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
                                           const SymbolIndex *Index) {
   const auto &SM = AST.getSourceManager();

diff  --git a/clang-tools-extra/clangd/XRefs.h b/clang-tools-extra/clangd/XRefs.h
index bc8deb1e3df7..dc9e4f23f1ad 100644
--- a/clang-tools-extra/clangd/XRefs.h
+++ b/clang-tools-extra/clangd/XRefs.h
@@ -49,9 +49,6 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &, const LocatedSymbol &);
 std::vector<LocatedSymbol> locateSymbolAt(ParsedAST &AST, Position Pos,
                                           const SymbolIndex *Index = nullptr);
 
-/// Get all document links
-std::vector<DocumentLink> getDocumentLinks(ParsedAST &AST);
-
 /// Returns highlights for all usages of a symbol at \p Pos.
 std::vector<DocumentHighlight> findDocumentHighlights(ParsedAST &AST,
                                                       Position Pos);

diff  --git a/clang-tools-extra/clangd/test/document-link.test b/clang-tools-extra/clangd/test/document-link.test
deleted file mode 100644
index 7802fe784568..000000000000
--- a/clang-tools-extra/clangd/test/document-link.test
+++ /dev/null
@@ -1,42 +0,0 @@
-# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
----
-{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"#include <stdint.h>\n#include <stddef.h>"}}}
----
-{"jsonrpc":"2.0","id":2,"method":"textDocument/documentLink","params":{"textDocument":{"uri":"test:///main.cpp"}}}
-#      CHECK:  "id": 2,
-# CHECK-NEXT:  "jsonrpc": "2.0",
-# CHECK-NEXT:  "result": [
-# CHECK-NEXT:    {
-# CHECK-NEXT:      "range": {
-# CHECK-NEXT:        "end": {
-# CHECK-NEXT:          "character": 19,
-# CHECK-NEXT:          "line": 0
-# CHECK-NEXT:        },
-# CHECK-NEXT:        "start": {
-# CHECK-NEXT:          "character": 9,
-# CHECK-NEXT:          "line": 0
-# CHECK-NEXT:        }
-# CHECK-NEXT:      },
-# CHECK-NEXT:      "target": "file://{{.*}}/stdint.h"
-# CHECK-NEXT:    },
-# CHECK-NEXT:    {
-# CHECK-NEXT:      "range": {
-# CHECK-NEXT:        "end": {
-# CHECK-NEXT:          "character": 19,
-# CHECK-NEXT:          "line": 1
-# CHECK-NEXT:        },
-# CHECK-NEXT:        "start": {
-# CHECK-NEXT:          "character": 9,
-# CHECK-NEXT:          "line": 1
-# CHECK-NEXT:        }
-# CHECK-NEXT:      },
-# CHECK-NEXT:      "target": "file://{{.*}}/stddef.h"
-# CHECK-NEXT:    }
-# CHECK-NEXT:  ]
-# CHECK-NEXT:}
-
----
-{"jsonrpc":"2.0","id":3,"method":"shutdown"}
----
-{"jsonrpc":"2.0","method":"exit"}

diff  --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test
index e5ec9fb3d7ac..8a853b7c6223 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -18,9 +18,6 @@
 # CHECK-NEXT:      "definitionProvider": true,
 # CHECK-NEXT:      "documentFormattingProvider": true,
 # CHECK-NEXT:      "documentHighlightProvider": true,
-# CHECK-NEXT:      "documentLinkProvider": {
-# CHECK-NEXT:        "resolveProvider": false
-# CHECK-NEXT:      }
 # CHECK-NEXT:      "documentOnTypeFormattingProvider": {
 # CHECK-NEXT:        "firstTriggerCharacter": "\n",
 # CHECK-NEXT:        "moreTriggerCharacter": []

diff  --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 51a7f81dcb5a..5ec29af76a9b 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1121,27 +1121,6 @@ TEST(GetNonLocalDeclRefs, All) {
   }
 }
 
-TEST(DocumentLinks, All) {
-  Annotations MainCpp(R"cpp(
-      #include $foo[["foo.h"]]
-      int end_of_preamble = 0;
-      #include $bar[["bar.h"]]
-    )cpp");
-
-  TestTU TU;
-  TU.Code = MainCpp.code();
-  TU.AdditionalFiles = {{"foo.h", ""}, {"bar.h", ""}};
-  auto AST = TU.build();
-
-  EXPECT_THAT(
-      clangd::getDocumentLinks(AST),
-      ElementsAre(
-          DocumentLink({MainCpp.range("foo"),
-                        URIForFile::canonicalize(testPath("foo.h"), "")}),
-          DocumentLink({MainCpp.range("bar"),
-                        URIForFile::canonicalize(testPath("bar.h"), "")})));
-}
-
 } // namespace
 } // namespace clangd
 } // namespace clang


        


More information about the cfe-commits mailing list