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 <christian.kandeler@theqtcompany.com>
Reviewed-by: David Schulz <david.schulz@theqtcompany.com>
This commit is contained in:
Nikolai Kosjar
2016-06-24 15:01:16 +02:00
parent 69fa7a8ffb
commit 46840046df
3 changed files with 34 additions and 23 deletions

View File

@@ -352,13 +352,17 @@ void ClangDiagnosticManager::addFixItAvailableMarker(
QSet<int> &lineNumbersWithFixItMarker) QSet<int> &lineNumbersWithFixItMarker)
{ {
for (auto &&diagnostic : diagnostics) { for (auto &&diagnostic : diagnostics) {
const int line = int(diagnostic.location().line()); for (auto &&fixit : diagnostic.fixIts()) {
if (!diagnostic.fixIts().isEmpty() && !lineNumbersWithFixItMarker.contains(line)) { const ClangBackEnd::SourceLocationContainer location = fixit.range().start();
const TextEditor::RefactorMarker marker const int line = int(location.line());
= createFixItAvailableMarker(m_textDocument->document(), line);
lineNumbersWithFixItMarker.insert(line); if (location.filePath() == filePath() && !lineNumbersWithFixItMarker.contains(line)) {
m_fixItAvailableMarkers.append(marker); const TextEditor::RefactorMarker marker
= createFixItAvailableMarker(m_textDocument->document(), line);
lineNumbersWithFixItMarker.insert(line);
m_fixItAvailableMarkers.append(marker);
}
} }
addFixItAvailableMarker(diagnostic.children(), lineNumbersWithFixItMarker); addFixItAvailableMarker(diagnostic.children(), lineNumbersWithFixItMarker);

View File

@@ -27,6 +27,8 @@
#include "clangfixitoperation.h" #include "clangfixitoperation.h"
#include <utils/algorithm.h>
#include <QDebug> #include <QDebug>
using ClangBackEnd::DiagnosticContainer; using ClangBackEnd::DiagnosticContainer;
@@ -75,13 +77,16 @@ QString tweakedDiagnosticText(const QString &diagnosticText)
return tweakedText; return tweakedText;
} }
bool isDiagnosticFromFileAtLine(const DiagnosticContainer &diagnosticContainer, bool hasFixItAt(const QVector<ClangBackEnd::FixItContainer> &fixits,
const QString &filePath, const Utf8String &filePath,
int line) int line)
{ {
const auto location = diagnosticContainer.location(); const auto isFixitForLocation = [filePath, line] (const ClangBackEnd::FixItContainer &fixit) {
return location.filePath().toString() == filePath const ClangBackEnd::SourceLocationContainer location = fixit.range().start();
&& location.line() == uint(line); return location.filePath() == filePath && location.line() == uint(line);
};
return Utils::anyOf(fixits, isFixitForLocation);
} }
} // anonymous namespace } // anonymous namespace
@@ -103,17 +108,17 @@ ClangFixItOperationsExtractor::extract(const QString &filePath, int line)
return operations; return operations;
} }
void ClangFixItOperationsExtractor::appendFixitOperationsFromDiagnostic( void ClangFixItOperationsExtractor::appendFixitOperation(
const QString &filePath, const QString &filePath,
const DiagnosticContainer &diagnosticContainer) const QString &diagnosticText,
const QVector<ClangBackEnd::FixItContainer> &fixits)
{ {
const auto fixIts = diagnosticContainer.fixIts(); if (!fixits.isEmpty()) {
if (!fixIts.isEmpty()) { const QString diagnosticTextTweaked = tweakedDiagnosticText(diagnosticText);
const QString diagnosticText = tweakedDiagnosticText(diagnosticContainer.text().toString());
TextEditor::QuickFixOperation::Ptr operation( TextEditor::QuickFixOperation::Ptr operation(
new ClangFixItOperation(filePath, new ClangFixItOperation(filePath,
diagnosticText, diagnosticTextTweaked,
fixIts)); fixits));
operations.append(operation); operations.append(operation);
} }
} }
@@ -123,8 +128,9 @@ void ClangFixItOperationsExtractor::extractFromDiagnostic(
const QString &filePath, const QString &filePath,
int line) int line)
{ {
if (isDiagnosticFromFileAtLine(diagnosticContainer, filePath, line)) { const QVector<ClangBackEnd::FixItContainer> fixIts = diagnosticContainer.fixIts();
appendFixitOperationsFromDiagnostic(filePath, diagnosticContainer); if (hasFixItAt(fixIts, filePath, line)) {
appendFixitOperation(filePath, diagnosticContainer.text().toString(), fixIts);
foreach (const auto &child, diagnosticContainer.children()) foreach (const auto &child, diagnosticContainer.children())
extractFromDiagnostic(child, filePath, line); extractFromDiagnostic(child, filePath, line);

View File

@@ -42,8 +42,9 @@ private:
void extractFromDiagnostic(const ClangBackEnd::DiagnosticContainer &diagnosticContainer, void extractFromDiagnostic(const ClangBackEnd::DiagnosticContainer &diagnosticContainer,
const QString &filePath, const QString &filePath,
int line); int line);
void appendFixitOperationsFromDiagnostic(const QString &filePath, void appendFixitOperation(const QString &filePath,
const ClangBackEnd::DiagnosticContainer &diagnosticContainer); const QString &diagnosticText,
const QVector<ClangBackEnd::FixItContainer> &fixits);
private: private:
const QVector<ClangBackEnd::DiagnosticContainer> &diagnosticContainers; const QVector<ClangBackEnd::DiagnosticContainer> &diagnosticContainers;