[clangd] Assert path is absolute when assigning to URIForFile.

Summary:
The assertion will point directly to misbehaving code, so that
debugging related problems (like the one fixed by r325029) is easier.

Reviewers: hokein, ioeric, sammccall

Reviewed By: sammccall

Subscribers: klimek, jkorous-apple, cfe-commits

Differential Revision: https://reviews.llvm.org/D43246

llvm-svn: 325337

diff --git a/clang-tools-extra/clangd/ClangdLSPServer.cpp b/clang-tools-extra/clangd/ClangdLSPServer.cpp
index 3e5f79231f519ab2093098a8e9442b2f0efac76f..e9317a2d164df5dfe2c26f34f99bca5854b7e2c1 100644 (file)
--- a/clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ b/clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -87,8 +87,8 @@ std::vector<TextEdit> replacementsToEdits(StringRef Code,
 } // namespace
 
 void ClangdLSPServer::onInitialize(InitializeParams &Params) {
-  if (Params.rootUri && !Params.rootUri->file.empty())
-    Server.setRootPath(Params.rootUri->file);
+  if (Params.rootUri && *Params.rootUri)
+    Server.setRootPath(Params.rootUri->file());
   else if (Params.rootPath && !Params.rootPath->empty())
     Server.setRootPath(*Params.rootPath);
 
@@ -136,9 +136,9 @@ void ClangdLSPServer::onExit(ExitParams &Params) { IsDone = true; }
 
 void ClangdLSPServer::onDocumentDidOpen(DidOpenTextDocumentParams &Params) {
   if (Params.metadata && !Params.metadata->extraFlags.empty())
-    CDB.setExtraFlagsForFile(Params.textDocument.uri.file,
+    CDB.setExtraFlagsForFile(Params.textDocument.uri.file(),
                              std::move(Params.metadata->extraFlags));
-  Server.addDocument(Params.textDocument.uri.file, Params.textDocument.text);
+  Server.addDocument(Params.textDocument.uri.file(), Params.textDocument.text);
 }
 
 void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
@@ -146,7 +146,7 @@ void ClangdLSPServer::onDocumentDidChange(DidChangeTextDocumentParams &Params) {
     return replyError(ErrorCode::InvalidParams,
                       "can only apply one change at a time");
   // We only support full syncing right now.
-  Server.addDocument(Params.textDocument.uri.file,
+  Server.addDocument(Params.textDocument.uri.file(),
                      Params.contentChanges[0].text);
 }
 
@@ -184,7 +184,7 @@ void ClangdLSPServer::onCommand(ExecuteCommandParams &Params) {
 }
 
 void ClangdLSPServer::onRename(RenameParams &Params) {
-  Path File = Params.textDocument.uri.file;
+  Path File = Params.textDocument.uri.file();
   llvm::Optional<std::string> Code = Server.getDocument(File);
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
@@ -206,12 +206,12 @@ void ClangdLSPServer::onRename(RenameParams &Params) {
 }
 
 void ClangdLSPServer::onDocumentDidClose(DidCloseTextDocumentParams &Params) {
-  Server.removeDocument(Params.textDocument.uri.file);
+  Server.removeDocument(Params.textDocument.uri.file());
 }
 
 void ClangdLSPServer::onDocumentOnTypeFormatting(
     DocumentOnTypeFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
+  auto File = Params.textDocument.uri.file();
   auto Code = Server.getDocument(File);
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
@@ -227,7 +227,7 @@ void ClangdLSPServer::onDocumentOnTypeFormatting(
 
 void ClangdLSPServer::onDocumentRangeFormatting(
     DocumentRangeFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
+  auto File = Params.textDocument.uri.file();
   auto Code = Server.getDocument(File);
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
@@ -242,7 +242,7 @@ void ClangdLSPServer::onDocumentRangeFormatting(
 }
 
 void ClangdLSPServer::onDocumentFormatting(DocumentFormattingParams &Params) {
-  auto File = Params.textDocument.uri.file;
+  auto File = Params.textDocument.uri.file();
   auto Code = Server.getDocument(File);
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
@@ -259,14 +259,14 @@ void ClangdLSPServer::onDocumentFormatting(DocumentFormattingParams &Params) {
 void ClangdLSPServer::onCodeAction(CodeActionParams &Params) {
   // We provide a code action for each diagnostic at the requested location
   // which has FixIts available.
-  auto Code = Server.getDocument(Params.textDocument.uri.file);
+  auto Code = Server.getDocument(Params.textDocument.uri.file());
   if (!Code)
     return replyError(ErrorCode::InvalidParams,
                       "onCodeAction called for non-added file");
 
   json::ary Commands;
   for (Diagnostic &D : Params.context.diagnostics) {
-    auto Edits = getFixIts(Params.textDocument.uri.file, D);
+    auto Edits = getFixIts(Params.textDocument.uri.file(), D);
     if (!Edits.empty()) {
       WorkspaceEdit WE;
       WE.changes = {{Params.textDocument.uri.uri(), std::move(Edits)}};
@@ -281,12 +281,12 @@ void ClangdLSPServer::onCodeAction(CodeActionParams &Params) {
 }
 
 void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) {
-  Server.codeComplete(Params.textDocument.uri.file, Params.position, CCOpts,
+  Server.codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts,
                       [](Tagged<CompletionList> List) { reply(List.Value); });
 }
 
 void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) {
-  Server.signatureHelp(Params.textDocument.uri.file, Params.position,
+  Server.signatureHelp(Params.textDocument.uri.file(), Params.position,
                        [](llvm::Expected<Tagged<SignatureHelp>> SignatureHelp) {
                          if (!SignatureHelp)
                            return replyError(
@@ -298,7 +298,7 @@ void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) {
 
 void ClangdLSPServer::onGoToDefinition(TextDocumentPositionParams &Params) {
   Server.findDefinitions(
-      Params.textDocument.uri.file, Params.position,
+      Params.textDocument.uri.file(), Params.position,
       [](llvm::Expected<Tagged<std::vector<Location>>> Items) {
         if (!Items)
           return replyError(ErrorCode::InvalidParams,
@@ -308,13 +308,13 @@ void ClangdLSPServer::onGoToDefinition(TextDocumentPositionParams &Params) {
 }
 
 void ClangdLSPServer::onSwitchSourceHeader(TextDocumentIdentifier &Params) {
-  llvm::Optional<Path> Result = Server.switchSourceHeader(Params.uri.file);
+  llvm::Optional<Path> Result = Server.switchSourceHeader(Params.uri.file());
   reply(Result ? URI::createFile(*Result).toString() : "");
 }
 
 void ClangdLSPServer::onDocumentHighlight(TextDocumentPositionParams &Params) {
   Server.findDocumentHighlights(
-      Params.textDocument.uri.file, Params.position,
+      Params.textDocument.uri.file(), Params.position,
       [](llvm::Expected<Tagged<std::vector<DocumentHighlight>>> Highlights) {
         if (!Highlights)
           return replyError(ErrorCode::InternalError,

diff --git a/clang-tools-extra/clangd/Protocol.cpp b/clang-tools-extra/clangd/Protocol.cpp
index 0c10540ff3bdec29b57f6b9c981dc648a0689767..a884a0c4841d8818e4325aa8c7601ce57a0c2e26 100644 (file)
--- a/clang-tools-extra/clangd/Protocol.cpp
+++ b/clang-tools-extra/clangd/Protocol.cpp
@@ -12,8 +12,8 @@
 //===----------------------------------------------------------------------===//
 
 #include "Protocol.h"
-#include "URI.h"
 #include "Logger.h"
+#include "URI.h"
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/Format.h"
@@ -24,6 +24,11 @@
 namespace clang {
 namespace clangd {
 
+URIForFile::URIForFile(std::string AbsPath) {
+  assert(llvm::sys::path::is_absolute(AbsPath) && "the path is relative");
+  File = std::move(AbsPath);
+}
+
 bool fromJSON(const json::Expr &E, URIForFile &R) {
   if (auto S = E.asString()) {
     auto U = URI::parse(*S);
@@ -40,15 +45,13 @@ bool fromJSON(const json::Expr &E, URIForFile &R) {
       log(llvm::toString(Path.takeError()));
       return false;
     }
-    R.file = *Path;
+    R = URIForFile(*Path);
     return true;
   }
   return false;
 }
 
-json::Expr toJSON(const URIForFile &U) {
-  return U.uri();
-}
+json::Expr toJSON(const URIForFile &U) { return U.uri(); }
 
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, const URIForFile &U) {
   return OS << U.uri();

diff --git a/clang-tools-extra/clangd/Protocol.h b/clang-tools-extra/clangd/Protocol.h
index 7e1595727b173854b13bddcfdd2d356c3b7b3c63..447611906cdf6365f6a6c90938f4509a2ef04e8d 100644 (file)
--- a/clang-tools-extra/clangd/Protocol.h
+++ b/clang-tools-extra/clangd/Protocol.h
@@ -49,12 +49,17 @@ enum class ErrorCode {
 };
 
 struct URIForFile {
-  std::string file;
+  URIForFile() = default;
+  explicit URIForFile(std::string AbsPath);
 
-  std::string uri() const { return URI::createFile(file).toString(); }
+  /// Retrieves absolute path to the file.
+  llvm::StringRef file() const { return File; }
+
+  explicit operator bool() const { return !File.empty(); }
+  std::string uri() const { return URI::createFile(File).toString(); }
 
   friend bool operator==(const URIForFile &LHS, const URIForFile &RHS) {
-    return LHS.file == RHS.file;
+    return LHS.File == RHS.File;
   }
 
   friend bool operator!=(const URIForFile &LHS, const URIForFile &RHS) {
@@ -62,8 +67,11 @@ struct URIForFile {
   }
 
   friend bool operator<(const URIForFile &LHS, const URIForFile &RHS) {
-    return LHS.file < RHS.file;
+    return LHS.File < RHS.File;
   }
+
+private:
+  std::string File;
 };
 
 /// Serialize/deserialize \p URIForFile to/from a string URI.

diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp
index 6f43353dce6a9ba7f5730d82f1f8c8df9ffc54fd..849fa8bfbb283537f7ba3e9f2eb51ba2c16cd9da 100644 (file)
--- a/clang-tools-extra/clangd/XRefs.cpp
+++ b/clang-tools-extra/clangd/XRefs.cpp
@@ -149,7 +149,7 @@ getDeclarationLocation(ParsedAST &AST, const SourceRange &ValSourceRange) {
     }
   }
 
-  L.uri.file = FilePath.str();
+  L.uri = URIForFile(FilePath.str());
   L.range = R;
   return L;
 }