rwols created this revision.
This translates diagnostics correctly between 0-based LSP columns, and 1-based
clang columns.
https://reviews.llvm.org/D40860
Files:
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdUnit.cpp
clang-tools-extra/clangd/ClangdUnit.h
clang-tools-extra/test/clangd/diagnostics.test
clang-tools-extra/test/clangd/execute-command.test
clang-tools-extra/test/clangd/extra-flags.test
clang-tools-extra/test/clangd/fixits.test
Index: clang-tools-extra/test/clangd/fixits.test
===================================================================
--- clang-tools-extra/test/clangd/fixits.test
+++ clang-tools-extra/test/clangd/fixits.test
@@ -15,11 +15,11 @@
# CHECK-NEXT: "message": "using the result of an assignment as a condition without parentheses",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -29,11 +29,11 @@
# CHECK-NEXT: "message": "place parentheses around the assignment to silence this warning",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -43,11 +43,11 @@
# CHECK-NEXT: "message": "use '==' to turn this assignment into an equality comparison",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -58,7 +58,7 @@
# CHECK-NEXT: }
Content-Length: 746
-{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+{"jsonrpc":"2.0","id":2,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":34}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
# CHECK: "id": 2,
# CHECK-NEXT: "jsonrpc": "2.0",
# CHECK-NEXT: "result": [
@@ -128,7 +128,7 @@
# CHECK-NEXT: ]
Content-Length: 771
-{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":2,"code":"1","source":"foo","message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 35}, "end": {"line": 0, "character": 35}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
+{"jsonrpc":"2.0","id":3,"method":"textDocument/codeAction","params":{"textDocument":{"uri":"file:///foo.c"},"range":{"start":{"line":104,"character":13},"end":{"line":0,"character":34}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":2,"code":"1","source":"foo","message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 34}},"severity":3,"message":"use '==' to turn this assignment into an equality comparison"}]}}}
# Make sure unused "code" and "source" fields ignored gracefully
# CHECK: "id": 3,
# CHECK-NEXT: "jsonrpc": "2.0",
Index: clang-tools-extra/test/clangd/extra-flags.test
===================================================================
--- clang-tools-extra/test/clangd/extra-flags.test
+++ clang-tools-extra/test/clangd/extra-flags.test
@@ -15,11 +15,11 @@
# CHECK-NEXT: "message": "variable 'i' is uninitialized when used here",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 28,
+# CHECK-NEXT: "character": 27,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 28,
+# CHECK-NEXT: "character": 27,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -29,11 +29,11 @@
# CHECK-NEXT: "message": "initialize the variable 'i' to silence this warning",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 19,
+# CHECK-NEXT: "character": 18,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 19,
+# CHECK-NEXT: "character": 18,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -52,11 +52,11 @@
# CHECK-NEXT: "message": "variable 'i' is uninitialized when used here",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 28,
+# CHECK-NEXT: "character": 27,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 28,
+# CHECK-NEXT: "character": 27,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -66,11 +66,11 @@
# CHECK-NEXT: "message": "initialize the variable 'i' to silence this warning",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 19,
+# CHECK-NEXT: "character": 18,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 19,
+# CHECK-NEXT: "character": 18,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
Index: clang-tools-extra/test/clangd/execute-command.test
===================================================================
--- clang-tools-extra/test/clangd/execute-command.test
+++ clang-tools-extra/test/clangd/execute-command.test
@@ -15,11 +15,11 @@
# CHECK-NEXT: "message": "using the result of an assignment as a condition without parentheses",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -29,11 +29,11 @@
# CHECK-NEXT: "message": "place parentheses around the assignment to silence this warning",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -43,11 +43,11 @@
# CHECK-NEXT: "message": "use '==' to turn this assignment into an equality comparison",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 35,
+# CHECK-NEXT: "character": 34,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
Index: clang-tools-extra/test/clangd/diagnostics.test
===================================================================
--- clang-tools-extra/test/clangd/diagnostics.test
+++ clang-tools-extra/test/clangd/diagnostics.test
@@ -15,11 +15,11 @@
# CHECK-NEXT: "message": "return type of 'main' is not 'int'",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 1,
+# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 1,
+# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
@@ -29,11 +29,11 @@
# CHECK-NEXT: "message": "change return type to 'int'",
# CHECK-NEXT: "range": {
# CHECK-NEXT: "end": {
-# CHECK-NEXT: "character": 1,
+# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: },
# CHECK-NEXT: "start": {
-# CHECK-NEXT: "character": 1,
+# CHECK-NEXT: "character": 0,
# CHECK-NEXT: "line": 0
# CHECK-NEXT: }
# CHECK-NEXT: },
Index: clang-tools-extra/clangd/ClangdUnit.h
===================================================================
--- clang-tools-extra/clangd/ClangdUnit.h
+++ clang-tools-extra/clangd/ClangdUnit.h
@@ -45,7 +45,7 @@
/// A diagnostic with its FixIts.
struct DiagWithFixIts {
clangd::Diagnostic Diag;
- llvm::SmallVector<tooling::Replacement, 1> FixIts;
+ llvm::SmallVector<TextEdit, 1> FixIts;
};
// Stores Preamble and associated data.
Index: clang-tools-extra/clangd/ClangdUnit.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -120,38 +120,98 @@
llvm_unreachable("Unknown diagnostic level!");
}
-llvm::Optional<DiagWithFixIts> toClangdDiag(const StoredDiagnostic &D) {
- auto Location = D.getLocation();
- if (!Location.isValid() || !Location.getManager().isInMainFile(Location))
+/// Convert a clang::SourceLocation to a clangd::Position
+Position SourceLocToClangdPosition(const SourceManager &SrcMgr,
+ SourceLocation Location) {
+ Position P;
+ // LSP rows and columns are 0-based, clang rows and columns are 1-based.
+ P.line = static_cast<int>(SrcMgr.getSpellingLineNumber(Location)) - 1;
+ P.character = static_cast<int>(SrcMgr.getSpellingColumnNumber(Location)) - 1;
+ assert(P.line >= 0 && "Unexpected negative value for P.line");
+ assert(P.character >= 0 && "Unexpected negative value for P.character");
+ return P;
+}
+
+/// Convert a clang::FullSourceLoc to a clangd::Position
+Position SourceLocToClangdPosition(const FullSourceLoc &Location) {
+ return SourceLocToClangdPosition(Location.getManager(), Location);
+}
+
+/// Convert a clang::SourceRange to a clangd::Range
+Range SourceRangeToClangdRange(const SourceManager &SrcMgr,
+ const SourceRange &Input) {
+ Range Output;
+ Output.start = SourceLocToClangdPosition(SrcMgr, Input.getBegin());
+ Output.end = SourceLocToClangdPosition(SrcMgr, Input.getEnd());
+ return Output;
+}
+
+/// Convert a clang::CharSourceRange to a clangd::Range
+Range CharSourceRangeToClangdRange(const SourceManager &SrcMgr,
+ const LangOptions &Options,
+ const CharSourceRange &Input) {
+ if (Input.isTokenRange()) {
+ const auto EndLoc =
+ clang::Lexer::getLocForEndOfToken(Input.getEnd(), 0, SrcMgr, Options);
+ clang::SourceRange R(Input.getBegin(), EndLoc);
+ return SourceRangeToClangdRange(SrcMgr, R);
+ } else if (Input.isCharRange()) {
+ return SourceRangeToClangdRange(SrcMgr, Input.getAsRange());
+ } else {
+ llvm_unreachable(
+ "Expected Input to be either a token range or a char range.");
+ }
+}
+
+/// Convert a clang::FixitHint to a clangd::TextEdit
+TextEdit FixitHintToClangdTextEdit(const SourceManager &SrcMgr,
+ const LangOptions &Options,
+ const FixItHint &Input) {
+ TextEdit Output;
+ Output.range =
+ CharSourceRangeToClangdRange(SrcMgr, Options, Input.RemoveRange);
+ Output.newText = Input.CodeToInsert;
+ return Output;
+}
+
+llvm::Optional<DiagWithFixIts> toClangdDiag(const LangOptions &Options,
+ const StoredDiagnostic &D) {
+ const auto &Location = D.getLocation();
+ const auto &SrcMgr = Location.getManager();
+ if (!Location.isValid() || !SrcMgr.isInMainFile(Location))
return llvm::None;
- Position P;
- P.line = Location.getSpellingLineNumber() - 1;
- P.character = Location.getSpellingColumnNumber();
+ const auto P = SourceLocToClangdPosition(Location);
+
+
Range R = {P, P};
clangd::Diagnostic Diag = {R, getSeverity(D.getLevel()), D.getMessage()};
- llvm::SmallVector<tooling::Replacement, 1> FixItsForDiagnostic;
+ llvm::SmallVector<TextEdit, 1> FixItsForDiagnostic;
for (const FixItHint &Fix : D.getFixIts()) {
- FixItsForDiagnostic.push_back(clang::tooling::Replacement(
- Location.getManager(), Fix.RemoveRange, Fix.CodeToInsert));
+ FixItsForDiagnostic.emplace_back(
+ FixitHintToClangdTextEdit(SrcMgr, Options, Fix));
}
return DiagWithFixIts{Diag, std::move(FixItsForDiagnostic)};
}
class StoreDiagsConsumer : public DiagnosticConsumer {
public:
- StoreDiagsConsumer(std::vector<DiagWithFixIts> &Output) : Output(Output) {}
+ StoreDiagsConsumer(const LangOptions &Options,
+ std::vector<DiagWithFixIts> &Output)
+ : Options(Options), Output(Output) {}
void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) override {
DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
- if (auto convertedDiag = toClangdDiag(StoredDiagnostic(DiagLevel, Info)))
+ if (auto convertedDiag =
+ toClangdDiag(Options, StoredDiagnostic(DiagLevel, Info)))
Output.push_back(std::move(*convertedDiag));
}
private:
+ const LangOptions &Options;
std::vector<DiagWithFixIts> &Output;
};
@@ -174,7 +234,7 @@
clangd::Logger &Logger) {
std::vector<DiagWithFixIts> ASTDiags;
- StoreDiagsConsumer UnitDiagsConsumer(/*ref*/ ASTDiags);
+ StoreDiagsConsumer UnitDiagsConsumer(*CI->getLangOpts(), /*refs*/ ASTDiags);
const PrecompiledPreamble *PreamblePCH =
Preamble ? &Preamble->Preamble : nullptr;
@@ -586,7 +646,8 @@
trace::Span Tracer("Preamble");
SPAN_ATTACH(Tracer, "File", That->FileName);
std::vector<DiagWithFixIts> PreambleDiags;
- StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags);
+ StoreDiagsConsumer PreambleDiagnosticsConsumer(*CI->getLangOpts(),
+ /* refs */ PreambleDiags);
IntrusiveRefCntPtr<DiagnosticsEngine> PreambleDiagsEngine =
CompilerInstance::createDiagnostics(
&CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false);
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -73,7 +73,7 @@
void onCommand(Ctx C, ExecuteCommandParams &Params) override;
void onRename(Ctx C, RenameParams &Parames) override;
- std::vector<clang::tooling::Replacement>
+ std::vector<TextEdit>
getFixIts(StringRef File, const clangd::Diagnostic &D);
JSONOutput &Out;
@@ -87,7 +87,7 @@
bool IsDone = false;
std::mutex FixItsMutex;
- typedef std::map<clangd::Diagnostic, std::vector<clang::tooling::Replacement>>
+ typedef std::map<clangd::Diagnostic, std::vector<TextEdit>>
DiagnosticToReplacementMap;
/// Caches FixIts per file and diagnostics
llvm::StringMap<DiagnosticToReplacementMap> FixItsMap;
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===================================================================
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -177,9 +177,7 @@
std::string Code = Server.getDocument(Params.textDocument.uri.file);
json::ary Commands;
for (Diagnostic &D : Params.context.diagnostics) {
- std::vector<clang::tooling::Replacement> Fixes =
- getFixIts(Params.textDocument.uri.file, D);
- auto Edits = replacementsToEdits(Code, Fixes);
+ auto Edits = getFixIts(Params.textDocument.uri.file, D);
if (!Edits.empty()) {
WorkspaceEdit WE;
WE.changes = {{Params.textDocument.uri.uri, std::move(Edits)}};
@@ -264,7 +262,7 @@
return ShutdownRequestReceived;
}
-std::vector<clang::tooling::Replacement>
+std::vector<TextEdit>
ClangdLSPServer::getFixIts(StringRef File, const clangd::Diagnostic &D) {
std::lock_guard<std::mutex> Lock(FixItsMutex);
auto DiagToFixItsIter = FixItsMap.find(File);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
