From 46840046dff74cc1f56027ce769928241f534629 Mon Sep 17 00:00:00 2001 From: Nikolai Kosjar Date: Fri, 24 Jun 2016 15:01:16 +0200 Subject: [PATCH] Clang: Honor fixits own locations ...when showing the refactoring marker and generating the quick fix operations. Opening main.cpp main.cpp: #include "file.h" void f(char *s) { foo(s); } file.h: // significant line 1 // significant line 2 // significant line 3 bool foo(int fd); led to SOFT ASSERT: "textBlock.isValid()" in file clangdiagnosticmanager.cpp, line 205 and a misbehavior when applying the fixit from the tooltip. We take the line of a diagnostic to display the fixit marker if it has any fixits. But the (child) diagnostic might be issued for an other file (that's what happening here), so better take the line of the location where the fixit is meant to be applied. Same applies for generation of the quick fix entries. Change-Id: I48d38420b285d2d2f86e3faa2319513aa8b47848 Reviewed-by: Christian Kandeler Reviewed-by: David Schulz --- .../clangcodemodel/clangdiagnosticmanager.cpp | 16 +++++---- .../clangfixitoperationsextractor.cpp | 36 +++++++++++-------- .../clangfixitoperationsextractor.h | 5 +-- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/src/plugins/clangcodemodel/clangdiagnosticmanager.cpp b/src/plugins/clangcodemodel/clangdiagnosticmanager.cpp index 0e3d70ec9dd..71afe79bf17 100644 --- a/src/plugins/clangcodemodel/clangdiagnosticmanager.cpp +++ b/src/plugins/clangcodemodel/clangdiagnosticmanager.cpp @@ -352,13 +352,17 @@ void ClangDiagnosticManager::addFixItAvailableMarker( QSet &lineNumbersWithFixItMarker) { for (auto &&diagnostic : diagnostics) { - const int line = int(diagnostic.location().line()); - if (!diagnostic.fixIts().isEmpty() && !lineNumbersWithFixItMarker.contains(line)) { - const TextEditor::RefactorMarker marker - = createFixItAvailableMarker(m_textDocument->document(), line); + for (auto &&fixit : diagnostic.fixIts()) { + const ClangBackEnd::SourceLocationContainer location = fixit.range().start(); + const int line = int(location.line()); - lineNumbersWithFixItMarker.insert(line); - m_fixItAvailableMarkers.append(marker); + if (location.filePath() == filePath() && !lineNumbersWithFixItMarker.contains(line)) { + const TextEditor::RefactorMarker marker + = createFixItAvailableMarker(m_textDocument->document(), line); + + lineNumbersWithFixItMarker.insert(line); + m_fixItAvailableMarkers.append(marker); + } } addFixItAvailableMarker(diagnostic.children(), lineNumbersWithFixItMarker); diff --git a/src/plugins/clangcodemodel/clangfixitoperationsextractor.cpp b/src/plugins/clangcodemodel/clangfixitoperationsextractor.cpp index 08983d2dab2..7203e18dcb0 100644 --- a/src/plugins/clangcodemodel/clangfixitoperationsextractor.cpp +++ b/src/plugins/clangcodemodel/clangfixitoperationsextractor.cpp @@ -27,6 +27,8 @@ #include "clangfixitoperation.h" +#include + #include using ClangBackEnd::DiagnosticContainer; @@ -75,13 +77,16 @@ QString tweakedDiagnosticText(const QString &diagnosticText) return tweakedText; } -bool isDiagnosticFromFileAtLine(const DiagnosticContainer &diagnosticContainer, - const QString &filePath, - int line) +bool hasFixItAt(const QVector &fixits, + const Utf8String &filePath, + int line) { - const auto location = diagnosticContainer.location(); - return location.filePath().toString() == filePath - && location.line() == uint(line); + const auto isFixitForLocation = [filePath, line] (const ClangBackEnd::FixItContainer &fixit) { + const ClangBackEnd::SourceLocationContainer location = fixit.range().start(); + return location.filePath() == filePath && location.line() == uint(line); + }; + + return Utils::anyOf(fixits, isFixitForLocation); } } // anonymous namespace @@ -103,17 +108,17 @@ ClangFixItOperationsExtractor::extract(const QString &filePath, int line) return operations; } -void ClangFixItOperationsExtractor::appendFixitOperationsFromDiagnostic( +void ClangFixItOperationsExtractor::appendFixitOperation( const QString &filePath, - const DiagnosticContainer &diagnosticContainer) + const QString &diagnosticText, + const QVector &fixits) { - const auto fixIts = diagnosticContainer.fixIts(); - if (!fixIts.isEmpty()) { - const QString diagnosticText = tweakedDiagnosticText(diagnosticContainer.text().toString()); + if (!fixits.isEmpty()) { + const QString diagnosticTextTweaked = tweakedDiagnosticText(diagnosticText); TextEditor::QuickFixOperation::Ptr operation( new ClangFixItOperation(filePath, - diagnosticText, - fixIts)); + diagnosticTextTweaked, + fixits)); operations.append(operation); } } @@ -123,8 +128,9 @@ void ClangFixItOperationsExtractor::extractFromDiagnostic( const QString &filePath, int line) { - if (isDiagnosticFromFileAtLine(diagnosticContainer, filePath, line)) { - appendFixitOperationsFromDiagnostic(filePath, diagnosticContainer); + const QVector fixIts = diagnosticContainer.fixIts(); + if (hasFixItAt(fixIts, filePath, line)) { + appendFixitOperation(filePath, diagnosticContainer.text().toString(), fixIts); foreach (const auto &child, diagnosticContainer.children()) extractFromDiagnostic(child, filePath, line); diff --git a/src/plugins/clangcodemodel/clangfixitoperationsextractor.h b/src/plugins/clangcodemodel/clangfixitoperationsextractor.h index e504d27dea8..6c49a3da181 100644 --- a/src/plugins/clangcodemodel/clangfixitoperationsextractor.h +++ b/src/plugins/clangcodemodel/clangfixitoperationsextractor.h @@ -42,8 +42,9 @@ private: void extractFromDiagnostic(const ClangBackEnd::DiagnosticContainer &diagnosticContainer, const QString &filePath, int line); - void appendFixitOperationsFromDiagnostic(const QString &filePath, - const ClangBackEnd::DiagnosticContainer &diagnosticContainer); + void appendFixitOperation(const QString &filePath, + const QString &diagnosticText, + const QVector &fixits); private: const QVector &diagnosticContainers;