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 + std::vector getFixIts(StringRef File, const clangd::Diagnostic &D); JSONOutput &Out; @@ -87,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: 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 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 +std::vector ClangdLSPServer::getFixIts(StringRef File, const clangd::Diagnostic &D) { std::lock_guard Lock(FixItsMutex); auto DiagToFixItsIter = FixItsMap.find(File); 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 FixIts; + llvm::SmallVector 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 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(SrcMgr.getSpellingLineNumber(Location)) - 1; + P.character = static_cast(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 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 FixItsForDiagnostic; + llvm::SmallVector 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 &Output) : Output(Output) {} + StoreDiagsConsumer(const LangOptions &Options, + std::vector &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 &Output; }; @@ -174,7 +234,7 @@ clangd::Logger &Logger) { std::vector 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 PreambleDiags; - StoreDiagsConsumer PreambleDiagnosticsConsumer(/*ref*/ PreambleDiags); + StoreDiagsConsumer PreambleDiagnosticsConsumer(*CI->getLangOpts(), + /* refs */ PreambleDiags); IntrusiveRefCntPtr PreambleDiagsEngine = CompilerInstance::createDiagnostics( &CI->getDiagnosticOpts(), &PreambleDiagnosticsConsumer, false); 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/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/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/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",