diff options
author | Sam McCall <sam.mccall@gmail.com> | 2017-12-13 08:48:42 +0000 |
---|---|---|
committer | Sam McCall <sam.mccall@gmail.com> | 2017-12-13 08:48:42 +0000 |
commit | fc51ebe8a0583b109cfd0b2469f509603db6c333 (patch) | |
tree | ab4645b751137423af25fd7b7f43855f7c7c0dc2 /clang-tools-extra/clangd/ClangdUnit.cpp | |
parent | 0f6debe74f2cbbe69dfed05983b57dc33db8dcf7 (diff) |
[clangd] Emit ranges for clangd diagnostics, and fix off-by-one positions
Summary:
- when the diagnostic has an explicit range, we prefer that
- if the diagnostic has a fixit, its RemoveRange is our next choice
- otherwise we try to expand the diagnostic location into a whole token.
(inspired by VSCode, which does this client-side when given an empty range)
- if all else fails, we return the zero-width range as now.
(clients react in different ways to this, highlighting a token or a char)
- this includes the off-by-one fix from D40860, and borrows heavily from it
Reviewers: rwols, hokein
Subscribers: klimek, ilya-biryukov, cfe-commits
Differential Revision: https://reviews.llvm.org/D41118
Diffstat (limited to 'clang-tools-extra/clangd/ClangdUnit.cpp')
-rw-r--r-- | clang-tools-extra/clangd/ClangdUnit.cpp | 93 |
1 files changed, 77 insertions, 16 deletions
diff --git a/clang-tools-extra/clangd/ClangdUnit.cpp b/clang-tools-extra/clangd/ClangdUnit.cpp index 313c72edfce..c699c2f0a40 100644 --- a/clang-tools-extra/clangd/ClangdUnit.cpp +++ b/clang-tools-extra/clangd/ClangdUnit.cpp @@ -120,39 +120,100 @@ static int getSeverity(DiagnosticsEngine::Level L) { llvm_unreachable("Unknown diagnostic level!"); } -llvm::Optional<DiagWithFixIts> toClangdDiag(const StoredDiagnostic &D) { - auto Location = D.getLocation(); - if (!Location.isValid() || !Location.getManager().isInMainFile(Location)) - return llvm::None; +// 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()); +} - Position P; - P.line = Location.getSpellingLineNumber() - 1; - P.character = Location.getSpellingColumnNumber(); - Range R = {P, P}; - clangd::Diagnostic Diag = {R, getSeverity(D.getLevel()), D.getMessage()}; +// 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<int>(M.getSpellingLineNumber(R.getBegin())) - 1, + static_cast<int>(M.getSpellingColumnNumber(R.getBegin())) - 1}, + {static_cast<int>(M.getSpellingLineNumber(R.getEnd())) - 1, + static_cast<int>(M.getSpellingColumnNumber(R.getEnd())) - 1}}; +} - llvm::SmallVector<tooling::Replacement, 1> FixItsForDiagnostic; - for (const FixItHint &Fix : D.getFixIts()) { - FixItsForDiagnostic.push_back(clang::tooling::Replacement( - Location.getManager(), Fix.RemoveRange, Fix.CodeToInsert)); +// 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); } - return DiagWithFixIts{Diag, std::move(FixItsForDiagnostic)}; + // 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<DiagWithFixIts> 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; + + 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<DiagWithFixIts> &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<DiagWithFixIts> &Output; + llvm::Optional<LangOptions> LangOpts; }; template <class T> bool futureIsReady(std::shared_future<T> const &Future) { |