Index: clangd/ClangdLSPServer.h =================================================================== --- clangd/ClangdLSPServer.h +++ clangd/ClangdLSPServer.h @@ -74,8 +74,7 @@ void onCommand(Ctx C, ExecuteCommandParams &Params) override; void onRename(Ctx C, RenameParams &Parames) override; - std::vector - getFixIts(StringRef File, const clangd::Diagnostic &D); + std::vector getFixIts(StringRef File, const clangd::Diagnostic &D); JSONOutput &Out; /// Used to indicate that the 'shutdown' request was received from the @@ -88,7 +87,7 @@ bool IsDone = false; std::mutex FixItsMutex; - typedef std::map> + typedef std::map> DiagnosticToReplacementMap; /// Caches FixIts per file and diagnostics llvm::StringMap FixItsMap; Index: clangd/ClangdLSPServer.cpp =================================================================== --- clangd/ClangdLSPServer.cpp +++ clangd/ClangdLSPServer.cpp @@ -202,9 +202,7 @@ std::string Code = Server.getDocument(Params.textDocument.uri.file); json::ary Commands; for (Diagnostic &D : Params.context.diagnostics) { - std::vector 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)}}; @@ -306,8 +304,8 @@ return ShutdownRequestReceived; } -std::vector -ClangdLSPServer::getFixIts(StringRef File, const clangd::Diagnostic &D) { +std::vector ClangdLSPServer::getFixIts(StringRef File, + const clangd::Diagnostic &D) { std::lock_guard Lock(FixItsMutex); auto DiagToFixItsIter = FixItsMap.find(File); if (DiagToFixItsIter == FixItsMap.end()) Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -45,7 +45,7 @@ /// A diagnostic with its FixIts. struct DiagWithFixIts { clangd::Diagnostic Diag; - llvm::SmallVector FixIts; + llvm::SmallVector FixIts; }; // Stores Preamble and associated data. Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -120,39 +120,100 @@ llvm_unreachable("Unknown diagnostic level!"); } -llvm::Optional toClangdDiag(const StoredDiagnostic &D) { - auto Location = D.getLocation(); - if (!Location.isValid() || !Location.getManager().isInMainFile(Location)) +// Checks whether a location is within a half-open range. +// Note that clang also uses closed source ranges, which this can't handle! +bool locationInRange(SourceLocation L, CharSourceRange R, + const SourceManager &M) { + assert(R.isCharRange()); + if (!R.isValid() || M.getFileID(R.getBegin()) != M.getFileID(R.getEnd()) || + M.getFileID(R.getBegin()) != M.getFileID(L)) + return false; + return L != R.getEnd() && M.isPointWithin(L, R.getBegin(), R.getEnd()); +} + +// Converts a half-open clang source range to an LSP range. +// Note that clang also uses closed source ranges, which this can't handle! +Range toRange(CharSourceRange R, const SourceManager &M) { + // Clang is 1-based, LSP uses 0-based indexes. + return {{static_cast(M.getSpellingLineNumber(R.getBegin())) - 1, + static_cast(M.getSpellingColumnNumber(R.getBegin())) - 1}, + {static_cast(M.getSpellingLineNumber(R.getEnd())) - 1, + static_cast(M.getSpellingColumnNumber(R.getEnd())) - 1}}; +} + +// Clang diags have a location (shown as ^) and 0 or more ranges (~~~~). +// LSP needs a single range. +Range diagnosticRange(const clang::Diagnostic &D, const LangOptions &L) { + auto &M = D.getSourceManager(); + auto Loc = M.getFileLoc(D.getLocation()); + // Accept the first range that contains the location. + for (const auto &CR : D.getRanges()) { + auto R = Lexer::makeFileCharRange(CR, M, L); + if (locationInRange(Loc, R, M)) + return toRange(R, M); + } + // The range may be given as a fixit hint instead. + for (const auto &F : D.getFixItHints()) { + auto R = Lexer::makeFileCharRange(F.RemoveRange, M, L); + if (locationInRange(Loc, R, M)) + return toRange(R, M); + } + // If no suitable range is found, just use the token at the location. + auto R = Lexer::makeFileCharRange(CharSourceRange::getTokenRange(Loc), M, L); + if (!R.isValid()) // Fall back to location only, let the editor deal with it. + R = CharSourceRange::getCharRange(Loc); + return toRange(R, M); +} + +TextEdit toTextEdit(const FixItHint &FixIt, const SourceManager &M, + const LangOptions &L) { + TextEdit Result; + Result.range = toRange(Lexer::makeFileCharRange(FixIt.RemoveRange, M, L), M); + Result.newText = FixIt.CodeToInsert; + return Result; +} + +llvm::Optional toClangdDiag(const clang::Diagnostic &D, + DiagnosticsEngine::Level Level, + const LangOptions &LangOpts) { + if (!D.hasSourceManager() || !D.getLocation().isValid() || + !D.getSourceManager().isInMainFile(D.getLocation())) return llvm::None; - Position P; - P.line = Location.getSpellingLineNumber() - 1; - P.character = Location.getSpellingColumnNumber(); - Range R = {P, P}; - clangd::Diagnostic Diag = {R, getSeverity(D.getLevel()), D.getMessage()}; - - llvm::SmallVector FixItsForDiagnostic; - for (const FixItHint &Fix : D.getFixIts()) { - FixItsForDiagnostic.push_back(clang::tooling::Replacement( - Location.getManager(), Fix.RemoveRange, Fix.CodeToInsert)); - } - return DiagWithFixIts{Diag, std::move(FixItsForDiagnostic)}; + DiagWithFixIts Result; + Result.Diag.range = diagnosticRange(D, LangOpts); + Result.Diag.severity = getSeverity(Level); + SmallString<64> Message; + D.FormatDiagnostic(Message); + Result.Diag.message = Message.str(); + for (const FixItHint &Fix : D.getFixItHints()) + Result.FixIts.push_back(toTextEdit(Fix, D.getSourceManager(), LangOpts)); + return std::move(Result); } class StoreDiagsConsumer : public DiagnosticConsumer { public: StoreDiagsConsumer(std::vector &Output) : Output(Output) {} + // Track language options in case we need to expand token ranges. + void BeginSourceFile(const LangOptions &Opts, const Preprocessor *) override { + LangOpts = Opts; + } + + void EndSourceFile() override { LangOpts = llvm::None; } + void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) override { DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info); - if (auto convertedDiag = toClangdDiag(StoredDiagnostic(DiagLevel, Info))) - Output.push_back(std::move(*convertedDiag)); + if (LangOpts) + if (auto D = toClangdDiag(Info, DiagLevel, *LangOpts)) + Output.push_back(std::move(*D)); } private: std::vector &Output; + llvm::Optional LangOpts; }; template bool futureIsReady(std::shared_future const &Future) { Index: test/clangd/diagnostics.test =================================================================== --- test/clangd/diagnostics.test +++ 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": 4, # 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": 4, # 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: test/clangd/execute-command.test =================================================================== --- test/clangd/execute-command.test +++ 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": 37, # CHECK-NEXT: "line": 0 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 35, +# CHECK-NEXT: "character": 32, # CHECK-NEXT: "line": 0 # CHECK-NEXT: } # CHECK-NEXT: }, @@ -33,7 +33,7 @@ # 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: }, @@ -47,7 +47,7 @@ # 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: test/clangd/extra-flags.test =================================================================== --- test/clangd/extra-flags.test +++ test/clangd/extra-flags.test @@ -19,7 +19,7 @@ # 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: }, @@ -33,7 +33,7 @@ # 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: }, @@ -56,7 +56,7 @@ # 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: }, @@ -70,7 +70,7 @@ # 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: test/clangd/fixits.test =================================================================== --- test/clangd/fixits.test +++ 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": 37, # CHECK-NEXT: "line": 0 # CHECK-NEXT: }, # CHECK-NEXT: "start": { -# CHECK-NEXT: "character": 35, +# CHECK-NEXT: "character": 32, # CHECK-NEXT: "line": 0 # CHECK-NEXT: } # CHECK-NEXT: }, @@ -33,7 +33,7 @@ # 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: }, @@ -47,7 +47,7 @@ # 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":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"severity":2,"message":"using the result of an assignment as a condition without parentheses"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"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":35}},"context":{"diagnostics":[{"range":{"start": {"line": 0, "character": 32}, "end": {"line": 0, "character": 37}},"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": 35}},"severity":3,"message":"place parentheses around the assignment to silence this warning"},{"range":{"start": {"line": 0, "character": 34}, "end": {"line": 0, "character": 35}},"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", @@ -246,4 +246,4 @@ {"jsonrpc":"2.0","id":4,"method":"shutdown"} Content-Length: 33 -{"jsonrpc":"2.0":"method":"exit"} +{"jsonrpc":"2.0","method":"exit"}