ClangTools: Allow applying fixits

Add a new column to the view that allows to check diagnostics with
fixits. The checked fixits can then be applied with the also new "Apply
Fixits" button in the toolbar.

Some corner cases are not yet handled:
 * File is open in editor
 * File changed in the mean time

Change-Id: I3d3f353a4150699a0d082f2a4348e331a4213bcf
Reviewed-by: Ivan Donchevskii <ivan.donchevskii@qt.io>

10 files changed, 126 insertions, 25 deletions

diff --git a/src/plugins/clangtools/clangtidyclazytool.cpp b/src/plugins/clangtools/clangtidyclazytool.cpp
index 4e5b3448ca..fdf6a9adc6 100644
--- a/src/plugins/clangtools/clangtidyclazytool.cpp
+++ b/src/plugins/clangtools/clangtidyclazytool.cpp
@@ -50,10 +50,13 @@
 #include <projectexplorer/target.h>
 #include <projectexplorer/session.h>
 
+#include <texteditor/refactoringchanges.h>
+
 #include <utils/fancylineedit.h>
 #include <utils/utilsicons.h>
 
 #include <QAction>
+#include <QToolButton>
 
 using namespace Core;
 using namespace CppTools;
@@ -66,6 +69,43 @@ namespace Internal {
 
 static ClangTidyClazyTool *s_instance;
 
+static void applyFixits(const QVector<Diagnostic> &diagnostics)
+{
+    TextEditor::RefactoringChanges changes;
+    QMap<QString, TextEditor::RefactoringFilePtr> refactoringFiles;
+
+    // Create refactoring files and changes
+    for (const Diagnostic &diagnostic : diagnostics) {
+        const auto filePath = diagnostic.location.filePath;
+        QTC_ASSERT(!filePath.isEmpty(), continue);
+
+        // Get or create refactoring file
+        TextEditor::RefactoringFilePtr refactoringFile = refactoringFiles.value(filePath);
+        if (!refactoringFile) {
+            refactoringFile = changes.file(filePath);
+            refactoringFiles.insert(filePath, refactoringFile);
+        }
+
+        // Append changes
+        ChangeSet cs = refactoringFile->changeSet();
+
+        for (const ExplainingStep &step : diagnostic.explainingSteps) {
+            if (step.isFixIt) {
+                const Debugger::DiagnosticLocation start = step.ranges.first();
+                const Debugger::DiagnosticLocation end = step.ranges.last();
+                cs.replace(refactoringFile->position(start.line, start.column),
+                           refactoringFile->position(end.line, end.column), step.message);
+            }
+        }
+
+        refactoringFile->setChangeSet(cs);
+    }
+
+    // Apply refactoring file changes
+    for (TextEditor::RefactoringFilePtr refactoringFile  : refactoringFiles.values())
+        refactoringFile->apply();
+}
+
 ClangTidyClazyTool::ClangTidyClazyTool()
     : ClangTool("Clang-Tidy and Clazy")
 {
@@ -118,6 +158,22 @@ ClangTidyClazyTool::ClangTidyClazyTool()
             QRegExp(filter, Qt::CaseSensitive, QRegExp::WildcardUnix));
     });
 
+    // Apply fixits button
+    m_applyFixitsButton = new QToolButton;
+    m_applyFixitsButton->setText(tr("Apply Fixits"));
+    connect(m_applyFixitsButton, &QToolButton::clicked, [this]() {
+        QVector<Diagnostic> diagnosticsWithFixits;
+
+        const int count = m_diagnosticModel->rootItem()->childCount();
+        for (int i = 0; i < count; ++i) {
+            auto *item = static_cast<DiagnosticItem *>(m_diagnosticModel->rootItem()->childAt(i));
+            if (item->applyFixits())
+                diagnosticsWithFixits += item->diagnostic();
+        }
+
+        applyFixits(diagnosticsWithFixits);
+    });
+
     ActionContainer *menu = ActionManager::actionContainer(Debugger::Constants::M_DEBUG_ANALYZER);
     const QString toolTip = tr("Clang-Tidy and Clazy use a customized Clang executable from the "
                                "Clang project to search for errors and warnings.");
@@ -143,6 +199,7 @@ ClangTidyClazyTool::ClangTidyClazyTool()
     tidyClazyToolbar.addAction(m_goBack);
     tidyClazyToolbar.addAction(m_goNext);
     tidyClazyToolbar.addWidget(m_filterLineEdit);
+    tidyClazyToolbar.addWidget(m_applyFixitsButton);
     Debugger::registerToolbar(ClangTidyClazyPerspectiveId, tidyClazyToolbar);
 
     updateRunActions();
diff --git a/src/plugins/clangtools/clangtidyclazytool.h b/src/plugins/clangtools/clangtidyclazytool.h
index 45358560c1..25542fdb63 100644
--- a/src/plugins/clangtools/clangtidyclazytool.h
+++ b/src/plugins/clangtools/clangtidyclazytool.h
@@ -27,6 +27,10 @@
 
 #include "clangtool.h"
 
+QT_BEGIN_NAMESPACE
+class QToolButton;
+QT_END_NAMESPACE
+
 namespace Utils { class FancyLineEdit; }
 
 namespace ClangTools {
@@ -61,6 +65,7 @@ private:
     DiagnosticFilterModel *m_diagnosticFilterModel = nullptr;
 
     Utils::FancyLineEdit *m_filterLineEdit = nullptr;
+    QToolButton *m_applyFixitsButton = nullptr;
 
     QAction *m_goBack = nullptr;
     QAction *m_goNext = nullptr;
diff --git a/src/plugins/clangtools/clangtoolsdiagnostic.cpp b/src/plugins/clangtools/clangtoolsdiagnostic.cpp
index 078113fe9e..4f04c03338 100644
--- a/src/plugins/clangtools/clangtoolsdiagnostic.cpp
+++ b/src/plugins/clangtools/clangtoolsdiagnostic.cpp
@@ -28,11 +28,6 @@
 namespace ClangTools {
 namespace Internal {
 
-ExplainingStep::ExplainingStep()
-    : depth(0)
-{
-}
-
 bool ExplainingStep::isValid() const
 {
     return location.isValid() && !ranges.isEmpty() && !message.isEmpty();
diff --git a/src/plugins/clangtools/clangtoolsdiagnostic.h b/src/plugins/clangtools/clangtoolsdiagnostic.h
index 7809fa2adf..98ebb0173d 100644
--- a/src/plugins/clangtools/clangtoolsdiagnostic.h
+++ b/src/plugins/clangtools/clangtoolsdiagnostic.h
@@ -37,15 +37,14 @@ namespace Internal {
 class ExplainingStep
 {
 public:
-    ExplainingStep();
-
     bool isValid() const;
 
     QString message;
     QString extendedMessage;
     Debugger::DiagnosticLocation location;
     QList<Debugger::DiagnosticLocation> ranges;
-    int depth;
+    int depth = 0;
+    bool isFixIt = false;
 };
 
 class Diagnostic
@@ -60,6 +59,7 @@ public:
     QString issueContext;
     Debugger::DiagnosticLocation location;
     QList<ExplainingStep> explainingSteps;
+    bool hasFixits = false;
 };
 
 } // namespace Internal
diff --git a/src/plugins/clangtools/clangtoolsdiagnosticmodel.cpp b/src/plugins/clangtools/clangtoolsdiagnosticmodel.cpp
index bd1db48d32..1ffc3351e5 100644
--- a/src/plugins/clangtools/clangtoolsdiagnosticmodel.cpp
+++ b/src/plugins/clangtools/clangtoolsdiagnosticmodel.cpp
@@ -42,19 +42,6 @@
 namespace ClangTools {
 namespace Internal {
 
-class DiagnosticItem : public Utils::TreeItem
-{
-public:
-    DiagnosticItem(const Diagnostic &diag);
-
-    Diagnostic diagnostic() const { return m_diagnostic; }
-
-private:
-    QVariant data(int column, int role) const override;
-
-    const Diagnostic m_diagnostic;
-};
-
 class ExplainingStepItem : public Utils::TreeItem
 {
 public:
@@ -69,7 +56,7 @@ private:
 ClangToolsDiagnosticModel::ClangToolsDiagnosticModel(QObject *parent)
     : Utils::TreeModel<>(parent)
 {
-    setHeader({tr("Issue"), tr("Location")});
+    setHeader({tr("Issue"), tr("Location"), tr("Fixits")});
 }
 
 void ClangToolsDiagnosticModel::addDiagnostics(const QList<Diagnostic> &diagnostics)
@@ -225,6 +212,13 @@ DiagnosticItem::DiagnosticItem(const Diagnostic &diag) : m_diagnostic(diag)
         appendChild(new ExplainingStepItem(s));
 }
 
+Qt::ItemFlags DiagnosticItem::flags(int column) const
+{
+    if (column == DiagnosticView::FixItColumn && m_diagnostic.hasFixits)
+        return TreeItem::flags(column) | Qt::ItemIsUserCheckable;
+    return TreeItem::flags(column);
+}
+
 static QVariant locationData(int role, const Debugger::DiagnosticLocation &location)
 {
     switch (role) {
@@ -255,6 +249,12 @@ QVariant DiagnosticItem::data(int column, int role) const
     if (column == Debugger::DetailedErrorView::LocationColumn)
         return locationData(role, m_diagnostic.location);
 
+    if (column == DiagnosticView::FixItColumn) {
+        if (role == Qt::CheckStateRole)
+            return m_applyFixits ? Qt::Checked : Qt::Unchecked;
+        return QVariant();
+    }
+
     // DiagnosticColumn
     switch (role) {
     case Debugger::DetailedErrorView::FullTextRole:
@@ -272,6 +272,17 @@ QVariant DiagnosticItem::data(int column, int role) const
     }
 }
 
+bool DiagnosticItem::setData(int column, const QVariant &data, int role)
+{
+    if (column == DiagnosticView::FixItColumn && role == Qt::CheckStateRole) {
+        m_applyFixits = data.value<Qt::CheckState>() == Qt::Checked ? true : false;
+        update();
+        return true;
+    }
+
+    return Utils::TreeItem::setData(column, data, role);
+}
+
 ExplainingStepItem::ExplainingStepItem(const ExplainingStep &step) : m_step(step)
 {
 }
@@ -281,6 +292,9 @@ QVariant ExplainingStepItem::data(int column, int role) const
     if (column == Debugger::DetailedErrorView::LocationColumn)
         return locationData(role, m_step.location);
 
+    if (column == DiagnosticView::FixItColumn)
+        return QVariant();
+
     // DiagnosticColumn
     switch (role) {
     case Debugger::DetailedErrorView::FullTextRole:
diff --git a/src/plugins/clangtools/clangtoolsdiagnosticmodel.h b/src/plugins/clangtools/clangtoolsdiagnosticmodel.h
index 11a7caf652..0fbc99a5b7 100644
--- a/src/plugins/clangtools/clangtoolsdiagnosticmodel.h
+++ b/src/plugins/clangtools/clangtoolsdiagnosticmodel.h
@@ -40,6 +40,24 @@ namespace ProjectExplorer { class Project; }
 namespace ClangTools {
 namespace Internal {
 
+class DiagnosticItem : public Utils::TreeItem
+{
+public:
+    DiagnosticItem(const Diagnostic &diag);
+
+    Diagnostic diagnostic() const { return m_diagnostic; }
+    bool applyFixits() const { return m_applyFixits; }
+
+private:
+    Qt::ItemFlags flags(int column) const override;
+    QVariant data(int column, int role) const override;
+    bool setData(int column, const QVariant &data, int role) override;
+
+private:
+    const Diagnostic m_diagnostic;
+    bool m_applyFixits = false;
+};
+
 class ClangToolsDiagnosticModel : public Utils::TreeModel<>
 {
     Q_OBJECT
diff --git a/src/plugins/clangtools/clangtoolsdiagnosticview.h b/src/plugins/clangtools/clangtoolsdiagnosticview.h
index 6eff2ebf72..69d83416b5 100644
--- a/src/plugins/clangtools/clangtoolsdiagnosticview.h
+++ b/src/plugins/clangtools/clangtoolsdiagnosticview.h
@@ -37,6 +37,10 @@ class DiagnosticView : public Debugger::DetailedErrorView
 public:
     DiagnosticView(QWidget *parent = 0);
 
+    enum ExtraColumn {
+        FixItColumn = LocationColumn + 1,
+    };
+
 private:
     void suppressCurrentDiagnostic();
 
diff --git a/src/plugins/clangtools/clangtoolslogfilereader.cpp b/src/plugins/clangtools/clangtoolslogfilereader.cpp
index 388868e7e8..f0885c22fc 100644
--- a/src/plugins/clangtools/clangtoolslogfilereader.cpp
+++ b/src/plugins/clangtools/clangtoolslogfilereader.cpp
@@ -141,8 +141,8 @@ static ExplainingStep buildFixIt(const CXDiagnostic cxDiagnostic, unsigned index
 {
     ExplainingStep fixItStep;
     CXSourceRange cxFixItRange;
-    fixItStep.message = "fix-it: " + fromCXString(clang_getDiagnosticFixIt(cxDiagnostic, index,
-                                                                           &cxFixItRange));
+    fixItStep.isFixIt = true;
+    fixItStep.message = fromCXString(clang_getDiagnosticFixIt(cxDiagnostic, index, &cxFixItRange));
     fixItStep.location = diagLocationFromSourceLocation(clang_getRangeStart(cxFixItRange));
     fixItStep.ranges.push_back(fixItStep.location);
     fixItStep.ranges.push_back(diagLocationFromSourceLocation(clang_getRangeEnd(cxFixItRange)));
@@ -184,7 +184,9 @@ static Diagnostic buildDiagnostic(const CXDiagnostic cxDiagnostic, const QString
         diagnostic.explainingSteps.push_back(diagnosticStep);
     }
 
-    for (unsigned i = 0; i < clang_getDiagnosticNumFixIts(cxDiagnostic); ++i)
+    const unsigned fixItCount = clang_getDiagnosticNumFixIts(cxDiagnostic);
+    diagnostic.hasFixits = fixItCount != 0;
+    for (unsigned i = 0; i < fixItCount; ++i)
         diagnostic.explainingSteps.push_back(buildFixIt(cxDiagnostic, i));
 
     diagnostic.description = fromCXString(clang_getDiagnosticSpelling(cxDiagnostic));
diff --git a/src/plugins/texteditor/refactoringchanges.cpp b/src/plugins/texteditor/refactoringchanges.cpp
index 85be3c76a6..93f53012ec 100644
--- a/src/plugins/texteditor/refactoringchanges.cpp
+++ b/src/plugins/texteditor/refactoringchanges.cpp
@@ -285,6 +285,11 @@ QString RefactoringFile::textOf(const Range &range) const
     return textOf(range.start, range.end);
 }
 
+ChangeSet RefactoringFile::changeSet() const
+{
+    return m_changes;
+}
+
 void RefactoringFile::setChangeSet(const ChangeSet &changeSet)
 {
     if (m_fileName.isEmpty())
diff --git a/src/plugins/texteditor/refactoringchanges.h b/src/plugins/texteditor/refactoringchanges.h
index 737728a931..58084306ec 100644
--- a/src/plugins/texteditor/refactoringchanges.h
+++ b/src/plugins/texteditor/refactoringchanges.h
@@ -74,6 +74,7 @@ public:
     QString textOf(int start, int end) const;
     QString textOf(const Range &range) const;
 
+    Utils::ChangeSet changeSet() const;
     void setChangeSet(const Utils::ChangeSet &changeSet);
     void appendIndentRange(const Range &range);
     void appendReindentRange(const Range &range);