Skip to content

Commit

Permalink
[clangd] Assert path is absolute when assigning to URIForFile.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@ilya-biryukov
ilya-biryukov committed Feb 16, 2018
1 parent 2fac8d9 commit 7d60d20
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 27 deletions.
34 changes: 17 additions & 17 deletions clang-tools-extra/clangd/ClangdLSPServer.cpp
View file
Expand Up @@ -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);

Expand Down Expand Up @@ -136,17 +136,17 @@ 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) {
if (Params.contentChanges.size() != 1)
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);
}

Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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)}};
Expand All @@ -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(
Expand All @@ -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,
Expand All @@ -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,
Expand Down
13 changes: 8 additions & 5 deletions clang-tools-extra/clangd/Protocol.cpp
View file
Expand Up @@ -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"
Expand All @@ -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);
Expand All @@ -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();
Expand Down
16 changes: 12 additions & 4 deletions clang-tools-extra/clangd/Protocol.h
View file
Expand Up @@ -49,21 +49,29 @@ 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) {
return !(LHS == RHS);
}

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.
Expand Down
2 changes: 1 addition & 1 deletion clang-tools-extra/clangd/XRefs.cpp
View file
Expand Up @@ -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;
}
Expand Down

0 comments on commit 7d60d20

Please sign in to comment.