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>
This commit is contained in:
Nikolai Kosjar
2018-05-15 14:31:48 +02:00
parent 1773b4d8b5
commit 26b09af277
10 changed files with 126 additions and 25 deletions

View File

@@ -50,10 +50,13 @@
#include <projectexplorer/target.h> #include <projectexplorer/target.h>
#include <projectexplorer/session.h> #include <projectexplorer/session.h>
#include <texteditor/refactoringchanges.h>
#include <utils/fancylineedit.h> #include <utils/fancylineedit.h>
#include <utils/utilsicons.h> #include <utils/utilsicons.h>
#include <QAction> #include <QAction>
#include <QToolButton>
using namespace Core; using namespace Core;
using namespace CppTools; using namespace CppTools;
@@ -66,6 +69,43 @@ namespace Internal {
static ClangTidyClazyTool *s_instance; 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() ClangTidyClazyTool::ClangTidyClazyTool()
: ClangTool("Clang-Tidy and Clazy") : ClangTool("Clang-Tidy and Clazy")
{ {
@@ -118,6 +158,22 @@ ClangTidyClazyTool::ClangTidyClazyTool()
QRegExp(filter, Qt::CaseSensitive, QRegExp::WildcardUnix)); 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); ActionContainer *menu = ActionManager::actionContainer(Debugger::Constants::M_DEBUG_ANALYZER);
const QString toolTip = tr("Clang-Tidy and Clazy use a customized Clang executable from the " const QString toolTip = tr("Clang-Tidy and Clazy use a customized Clang executable from the "
"Clang project to search for errors and warnings."); "Clang project to search for errors and warnings.");
@@ -143,6 +199,7 @@ ClangTidyClazyTool::ClangTidyClazyTool()
tidyClazyToolbar.addAction(m_goBack); tidyClazyToolbar.addAction(m_goBack);
tidyClazyToolbar.addAction(m_goNext); tidyClazyToolbar.addAction(m_goNext);
tidyClazyToolbar.addWidget(m_filterLineEdit); tidyClazyToolbar.addWidget(m_filterLineEdit);
tidyClazyToolbar.addWidget(m_applyFixitsButton);
Debugger::registerToolbar(ClangTidyClazyPerspectiveId, tidyClazyToolbar); Debugger::registerToolbar(ClangTidyClazyPerspectiveId, tidyClazyToolbar);
updateRunActions(); updateRunActions();

View File

@@ -27,6 +27,10 @@
#include "clangtool.h" #include "clangtool.h"
QT_BEGIN_NAMESPACE
class QToolButton;
QT_END_NAMESPACE
namespace Utils { class FancyLineEdit; } namespace Utils { class FancyLineEdit; }
namespace ClangTools { namespace ClangTools {
@@ -61,6 +65,7 @@ private:
DiagnosticFilterModel *m_diagnosticFilterModel = nullptr; DiagnosticFilterModel *m_diagnosticFilterModel = nullptr;
Utils::FancyLineEdit *m_filterLineEdit = nullptr; Utils::FancyLineEdit *m_filterLineEdit = nullptr;
QToolButton *m_applyFixitsButton = nullptr;
QAction *m_goBack = nullptr; QAction *m_goBack = nullptr;
QAction *m_goNext = nullptr; QAction *m_goNext = nullptr;

View File

@@ -28,11 +28,6 @@
namespace ClangTools { namespace ClangTools {
namespace Internal { namespace Internal {
ExplainingStep::ExplainingStep()
: depth(0)
{
}
bool ExplainingStep::isValid() const bool ExplainingStep::isValid() const
{ {
return location.isValid() && !ranges.isEmpty() && !message.isEmpty(); return location.isValid() && !ranges.isEmpty() && !message.isEmpty();

View File

@@ -37,15 +37,14 @@ namespace Internal {
class ExplainingStep class ExplainingStep
{ {
public: public:
ExplainingStep();
bool isValid() const; bool isValid() const;
QString message; QString message;
QString extendedMessage; QString extendedMessage;
Debugger::DiagnosticLocation location; Debugger::DiagnosticLocation location;
QList<Debugger::DiagnosticLocation> ranges; QList<Debugger::DiagnosticLocation> ranges;
int depth; int depth = 0;
bool isFixIt = false;
}; };
class Diagnostic class Diagnostic
@@ -60,6 +59,7 @@ public:
QString issueContext; QString issueContext;
Debugger::DiagnosticLocation location; Debugger::DiagnosticLocation location;
QList<ExplainingStep> explainingSteps; QList<ExplainingStep> explainingSteps;
bool hasFixits = false;
}; };
} // namespace Internal } // namespace Internal

View File

@@ -42,19 +42,6 @@
namespace ClangTools { namespace ClangTools {
namespace Internal { 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 class ExplainingStepItem : public Utils::TreeItem
{ {
public: public:
@@ -69,7 +56,7 @@ private:
ClangToolsDiagnosticModel::ClangToolsDiagnosticModel(QObject *parent) ClangToolsDiagnosticModel::ClangToolsDiagnosticModel(QObject *parent)
: Utils::TreeModel<>(parent) : Utils::TreeModel<>(parent)
{ {
setHeader({tr("Issue"), tr("Location")}); setHeader({tr("Issue"), tr("Location"), tr("Fixits")});
} }
void ClangToolsDiagnosticModel::addDiagnostics(const QList<Diagnostic> &diagnostics) void ClangToolsDiagnosticModel::addDiagnostics(const QList<Diagnostic> &diagnostics)
@@ -225,6 +212,13 @@ DiagnosticItem::DiagnosticItem(const Diagnostic &diag) : m_diagnostic(diag)
appendChild(new ExplainingStepItem(s)); 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) static QVariant locationData(int role, const Debugger::DiagnosticLocation &location)
{ {
switch (role) { switch (role) {
@@ -255,6 +249,12 @@ QVariant DiagnosticItem::data(int column, int role) const
if (column == Debugger::DetailedErrorView::LocationColumn) if (column == Debugger::DetailedErrorView::LocationColumn)
return locationData(role, m_diagnostic.location); return locationData(role, m_diagnostic.location);
if (column == DiagnosticView::FixItColumn) {
if (role == Qt::CheckStateRole)
return m_applyFixits ? Qt::Checked : Qt::Unchecked;
return QVariant();
}
// DiagnosticColumn // DiagnosticColumn
switch (role) { switch (role) {
case Debugger::DetailedErrorView::FullTextRole: 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) 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) if (column == Debugger::DetailedErrorView::LocationColumn)
return locationData(role, m_step.location); return locationData(role, m_step.location);
if (column == DiagnosticView::FixItColumn)
return QVariant();
// DiagnosticColumn // DiagnosticColumn
switch (role) { switch (role) {
case Debugger::DetailedErrorView::FullTextRole: case Debugger::DetailedErrorView::FullTextRole:

View File

@@ -40,6 +40,24 @@ namespace ProjectExplorer { class Project; }
namespace ClangTools { namespace ClangTools {
namespace Internal { 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<> class ClangToolsDiagnosticModel : public Utils::TreeModel<>
{ {
Q_OBJECT Q_OBJECT

View File

@@ -37,6 +37,10 @@ class DiagnosticView : public Debugger::DetailedErrorView
public: public:
DiagnosticView(QWidget *parent = 0); DiagnosticView(QWidget *parent = 0);
enum ExtraColumn {
FixItColumn = LocationColumn + 1,
};
private: private:
void suppressCurrentDiagnostic(); void suppressCurrentDiagnostic();

View File

@@ -141,8 +141,8 @@ static ExplainingStep buildFixIt(const CXDiagnostic cxDiagnostic, unsigned index
{ {
ExplainingStep fixItStep; ExplainingStep fixItStep;
CXSourceRange cxFixItRange; CXSourceRange cxFixItRange;
fixItStep.message = "fix-it: " + fromCXString(clang_getDiagnosticFixIt(cxDiagnostic, index, fixItStep.isFixIt = true;
&cxFixItRange)); fixItStep.message = fromCXString(clang_getDiagnosticFixIt(cxDiagnostic, index, &cxFixItRange));
fixItStep.location = diagLocationFromSourceLocation(clang_getRangeStart(cxFixItRange)); fixItStep.location = diagLocationFromSourceLocation(clang_getRangeStart(cxFixItRange));
fixItStep.ranges.push_back(fixItStep.location); fixItStep.ranges.push_back(fixItStep.location);
fixItStep.ranges.push_back(diagLocationFromSourceLocation(clang_getRangeEnd(cxFixItRange))); 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); 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.explainingSteps.push_back(buildFixIt(cxDiagnostic, i));
diagnostic.description = fromCXString(clang_getDiagnosticSpelling(cxDiagnostic)); diagnostic.description = fromCXString(clang_getDiagnosticSpelling(cxDiagnostic));

View File

@@ -285,6 +285,11 @@ QString RefactoringFile::textOf(const Range &range) const
return textOf(range.start, range.end); return textOf(range.start, range.end);
} }
ChangeSet RefactoringFile::changeSet() const
{
return m_changes;
}
void RefactoringFile::setChangeSet(const ChangeSet &changeSet) void RefactoringFile::setChangeSet(const ChangeSet &changeSet)
{ {
if (m_fileName.isEmpty()) if (m_fileName.isEmpty())

View File

@@ -74,6 +74,7 @@ public:
QString textOf(int start, int end) const; QString textOf(int start, int end) const;
QString textOf(const Range &range) const; QString textOf(const Range &range) const;
Utils::ChangeSet changeSet() const;
void setChangeSet(const Utils::ChangeSet &changeSet); void setChangeSet(const Utils::ChangeSet &changeSet);
void appendIndentRange(const Range &range); void appendIndentRange(const Range &range);
void appendReindentRange(const Range &range); void appendReindentRange(const Range &range);