Analyzer: Re-design the diagnostics view.

The old one had a number of problems, mainly due to the awkward
delegate that was used for presenting the data. For instance:
    - Only one diagnostic at a time could be looked at
      in detail.
    - Once it had been opened, it was not possible to close
      such a detailed view again, other than by opening a new one.
We now use a tree view for showing the diagnostics, so users
can show and hide details about as many diagnostics as they
wish. That also gets us sensible item selection capabilities,
so features like suppressing several diagnostics at once can
be implemented in the future.

Change-Id: I840fdbfeca4d936ce600c8f6dde58b2ab93b0d00
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@theqtcompany.com>

src/plugins/valgrind/xmlprotocol/errorlistmodel.cpp

@@ -35,29 +35,74 @@
 #include "stack.h"
 #include "modelhelpers.h"
 
+#include <analyzerbase/diagnosticlocation.h>
 #include <utils/qtcassert.h>
 
+#include <QCoreApplication>
 #include <QDir>
 #include <QVector>
 
+#include <cmath>
+
 namespace Valgrind {
 namespace XmlProtocol {
 
-class ErrorListModel::Private
+class ErrorListModelPrivate
 {
 public:
-    QVector<Error> errors;
-    QVariant errorData(int row, int column, int role) const;
+    QVariant errorData(const QModelIndex &index, int role) const;
     QSharedPointer<const ErrorListModel::RelevantFrameFinder> relevantFrameFinder;
     Frame findRelevantFrame(const Error &error) const;
-    QString formatAbsoluteFilePath(const Error &error) const;
-    QString formatLocation(const Error &error) const;
+    QString errorLocation(const Error &error) const;
 };
 
-ErrorListModel::ErrorListModel(QObject *parent)
-    : QAbstractItemModel(parent)
-    , d(new Private)
+class ErrorItem : public Utils::TreeItem
 {
+public:
+    ErrorItem(const ErrorListModelPrivate *modelPrivate, const Error &error);
+
+    const ErrorListModelPrivate *modelPrivate() const { return m_modelPrivate; }
+    Error error() const { return m_error; }
+
+private:
+    QVariant data(int column, int role) const override;
+
+    const ErrorListModelPrivate * const m_modelPrivate;
+    const Error m_error;
+};
+
+class StackItem : public Utils::TreeItem
+{
+public:
+    StackItem(const Stack &stack);
+
+private:
+    QVariant data(int column, int role) const override;
+
+    const ErrorItem *getErrorItem() const;
+
+    const Stack m_stack;
+};
+
+class FrameItem : public Utils::TreeItem
+{
+public:
+    FrameItem(const Frame &frame);
+
+private:
+    QVariant data(int column, int role) const override;
+
+    const ErrorItem *getErrorItem() const;
+
+    const Frame m_frame;
+};
+
+
+ErrorListModel::ErrorListModel(QObject *parent)
+    : Utils::TreeModel(parent)
+    , d(new ErrorListModelPrivate)
+{
+    setHeader(QStringList() << tr("Issue") << tr("Location"));
 }
 
 ErrorListModel::~ErrorListModel()
@@ -75,27 +120,7 @@ void ErrorListModel::setRelevantFrameFinder(const QSharedPointer<const RelevantF
     d->relevantFrameFinder = finder;
 }
 
-Frame ErrorListModel::findRelevantFrame(const Error &error) const
-{
-    return d->findRelevantFrame(error);
-}
-
-QModelIndex ErrorListModel::index(int row, int column, const QModelIndex &parent) const
-{
-    if (parent.isValid()) {
-        QTC_ASSERT(parent.model() == this, qt_noop());
-        return QModelIndex();
-    }
-    return createIndex(row, column);
-}
-
-QModelIndex ErrorListModel::parent(const QModelIndex &child) const
-{
-    QTC_ASSERT(!child.isValid() || child.model() == this, return QModelIndex());
-    return QModelIndex();
-}
-
-Frame ErrorListModel::Private::findRelevantFrame(const Error &error) const
+Frame ErrorListModelPrivate::findRelevantFrame(const Error &error) const
 {
     if (relevantFrameFinder)
         return relevantFrameFinder->findRelevant(error);
@@ -109,181 +134,194 @@ Frame ErrorListModel::Private::findRelevantFrame(const Error &error) const
     return Frame();
 }
 
-QString ErrorListModel::Private::formatAbsoluteFilePath(const Error &error) const
+static QString makeFrameName(const Frame &frame, bool withLocation)
 {
-    return findRelevantFrame(error).filePath();
-}
+    const QString d = frame.directory();
+    const QString f = frame.fileName();
+    const QString fn = frame.functionName();
+    const QString fullPath = frame.filePath();
 
-QString ErrorListModel::Private::formatLocation(const Error &error) const
-{
-    const Frame frame = findRelevantFrame(error);
-    const QString file = frame.fileName();
-    if (!frame.functionName().isEmpty())
-        return frame.functionName();
-    if (!file.isEmpty()) {
-        const qint64 line = frame.line();
-        if (line > 0)
-            return QString::fromLatin1("%1:%2").arg(file, QString::number(frame.line()));
-        return file;
+    QString path;
+    if (!d.isEmpty() && !f.isEmpty())
+        path = fullPath;
+    else
+        path = frame.object();
+
+    if (QFile::exists(path))
+        path = QFileInfo(path).canonicalFilePath();
+
+    if (frame.line() != -1)
+        path += QLatin1Char(':') + QString::number(frame.line());
+
+    if (!fn.isEmpty()) {
+        const QString location = withLocation || path == frame.object()
+                ? QString::fromLatin1(" in %2").arg(path) : QString();
+        return QCoreApplication::translate("Valgrind::Internal", "%1%2").arg(fn, location);
     }
-    return frame.object();
+    if (!path.isEmpty())
+        return path;
+    return QString::fromLatin1("0x%1").arg(frame.instructionPointer(), 0, 16);
 }
 
-QVariant ErrorListModel::Private::errorData(int row, int column, int role) const
+QString ErrorListModelPrivate::errorLocation(const Error &error) const
 {
-    // A dummy entry.
-    if (row == 0 && errors.isEmpty()) {
-        if (role == Qt::DisplayRole)
-            return tr("No errors found");
-        if (role == ErrorRole)
-            return tr("No errors found");
-        return QVariant();
-    }
-
-    if (row < 0 || row >= errors.size())
-        return QVariant();
-
-    const Error &error = errors.at(row);
-
-    if (error.stacks().count())
-    switch (role) {
-    case Qt::DisplayRole: {
-        switch (column) {
-        case WhatColumn:
-            return error.what();
-        case LocationColumn:
-            return formatLocation(error);
-        case AbsoluteFilePathColumn:
-            return formatAbsoluteFilePath(error);
-        case LineColumn: {
-            const qint64 line = findRelevantFrame(error).line();
-            return line > 0 ? line : QVariant();
-        }
-        case UniqueColumn:
-            return error.unique();
-        case TidColumn:
-            return error.tid();
-        case KindColumn:
-            return error.kind();
-        case LeakedBlocksColumn:
-            return error.leakedBlocks();
-        case LeakedBytesColumn:
-            return error.leakedBytes();
-        case HelgrindThreadIdColumn:
-            return error.helgrindThreadId();
-        default:
-            break;
-        }
-    }
-    case Qt::ToolTipRole:
-        return toolTipForFrame(findRelevantFrame(error));
-    case ErrorRole:
-        return QVariant::fromValue<Error>(error);
-    case AbsoluteFilePathRole:
-        return formatAbsoluteFilePath(error);
-    case FileRole:
-        return findRelevantFrame(error).fileName();
-    case LineRole: {
-        const qint64 line = findRelevantFrame(error).line();
-        return line > 0 ? line : QVariant();
-    }
-    }
-    return QVariant();
-}
-
-QVariant ErrorListModel::data(const QModelIndex &index, int role) const
-{
-    if (!index.isValid())
-        return QVariant();
-
-    if (!index.parent().isValid())
-        return d->errorData(index.row(), index.column(), role);
-
-    return QVariant();
-}
-
-QVariant ErrorListModel::headerData(int section, Qt::Orientation orientation, int role) const
-{
-    if (orientation != Qt::Horizontal || role != Qt::DisplayRole)
-        return QVariant();
-
-    switch (section) {
-    case WhatColumn:
-        return tr("What");
-    case LocationColumn:
-        return tr("Location");
-    case AbsoluteFilePathColumn:
-        return tr("File");
-    case LineColumn:
-        return tr("Line");
-    case UniqueColumn:
-        return tr("Unique");
-    case TidColumn:
-        return tr("Thread ID");
-    case KindColumn:
-        return tr("Kind");
-    case LeakedBlocksColumn:
-        return tr("Leaked Blocks");
-    case LeakedBytesColumn:
-        return tr("Leaked Bytes");
-    case HelgrindThreadIdColumn:
-        return tr("Helgrind Thread ID");
-    }
-
-    return QVariant();
-}
-
-int ErrorListModel::rowCount(const QModelIndex &parent) const
-{
-    if (parent.isValid())
-        return 0;
-    return d->errors.count();
-}
-
-int ErrorListModel::columnCount(const QModelIndex &parent) const
-{
-    QTC_ASSERT(!parent.isValid() || parent.model() == this, return 0);
-    return ColumnCount;
-}
-
-bool ErrorListModel::removeRows(int row, int count, const QModelIndex &parent)
-{
-    QTC_ASSERT(!parent.isValid() || parent.model() == this, return false);
-
-    if (row < 0 || row + count > d->errors.size() || parent.isValid())
-        return false;
-
-    beginRemoveRows(parent, row, row + count);
-    d->errors.remove(row, count);
-    endRemoveRows();
-    return true;
+    return QCoreApplication::translate("Valgrind::Internal", "in %1").
+            arg(makeFrameName(findRelevantFrame(error), true));
 }
 
 void ErrorListModel::addError(const Error &error)
 {
-    beginInsertRows(QModelIndex(), d->errors.size(), d->errors.size());
-    d->errors.push_back(error);
-    endInsertRows();
+    rootItem()->appendChild(new ErrorItem(d, error));
 }
 
-Error ErrorListModel::error(const QModelIndex &index) const
+
+ErrorItem::ErrorItem(const ErrorListModelPrivate *modelPrivate, const Error &error)
+    : m_modelPrivate(modelPrivate), m_error(error)
 {
-    if (!index.isValid())
-        return Error();
+    QTC_ASSERT(!m_error.stacks().isEmpty(), return);
 
-    QTC_ASSERT(index.model() == this, return Error());
-
-    const int r = index.row();
-    if (r < 0 || r >= d->errors.size())
-        return Error();
-    return d->errors.at(r);
+    // If there's more than one stack, we simply map the real tree structure.
+    // Otherwise, we skip the stack level, which has no useful information and would
+    // just annoy the user.
+    // The same goes for the frame level.
+    if (m_error.stacks().count() > 1) {
+        foreach (const Stack &s, m_error.stacks())
+            appendChild(new StackItem(s));
+    } else if (m_error.stacks().first().frames().count() > 1) {
+        foreach (const Frame &f, m_error.stacks().first().frames())
+            appendChild(new FrameItem(f));
+    }
 }
 
-void ErrorListModel::clear()
+static QVariant location(const Frame &frame, int role)
 {
-    beginResetModel();
-    d->errors.clear();
-    endResetModel();
+    switch (role) {
+    case Analyzer::DetailedErrorView::LocationRole:
+        return QVariant::fromValue(Analyzer::DiagnosticLocation(frame.filePath(), frame.line(), 0));
+    case Qt::ToolTipRole:
+        return frame.filePath().isEmpty() ? QVariant() : QVariant(frame.filePath());
+    default:
+        return QVariant();
+    }
+}
+
+QVariant ErrorItem::data(int column, int role) const
+{
+    if (column == Analyzer::DetailedErrorView::LocationColumn) {
+        const Frame frame = m_modelPrivate->findRelevantFrame(m_error);
+        return location(frame, role);
+    }
+
+    // DiagnosticColumn
+    switch (role) {
+    case Analyzer::DetailedErrorView::FullTextRole: {
+        QString content;
+        QTextStream stream(&content);
+
+        stream << m_error.what() << "\n";
+        stream << "  "
+               << m_modelPrivate->errorLocation(m_error)
+               << "\n";
+
+        foreach (const Stack &stack, m_error.stacks()) {
+            if (!stack.auxWhat().isEmpty())
+                stream << stack.auxWhat();
+            int i = 1;
+            foreach (const Frame &frame, stack.frames())
+                stream << "  " << i++ << ": " << makeFrameName(frame, true) << "\n";
+        }
+
+        stream.flush();
+        return content;
+    }
+    case ErrorListModel::ErrorRole:
+        return QVariant::fromValue<Error>(m_error);
+    case Qt::DisplayRole:
+        // If and only if there is exactly one frame, we have omitted creating a child item for it
+        // (see the constructor) and display the function name in the error item instead.
+        if (m_error.stacks().count() != 1 || m_error.stacks().first().frames().count() != 1
+                || m_error.stacks().first().frames().first().functionName().isEmpty()) {
+            return m_error.what();
+        }
+        return ErrorListModel::tr("%1 in function %2")
+                .arg(m_error.what(), m_error.stacks().first().frames().first().functionName());
+    case Qt::ToolTipRole:
+        return toolTipForFrame(m_modelPrivate->findRelevantFrame(m_error));
+    default:
+        return QVariant();
+    }
+}
+
+
+StackItem::StackItem(const Stack &stack) : m_stack(stack)
+{
+    foreach (const Frame &f, m_stack.frames())
+        appendChild(new FrameItem(f));
+}
+
+QVariant StackItem::data(int column, int role) const
+{
+    const ErrorItem * const errorItem = getErrorItem();
+    if (column == Analyzer::DetailedErrorView::LocationColumn)
+        return location(errorItem->modelPrivate()->findRelevantFrame(errorItem->error()), role);
+
+    // DiagnosticColumn
+    switch (role) {
+    case ErrorListModel::ErrorRole:
+        return QVariant::fromValue(errorItem->error());
+    case Qt::DisplayRole:
+        return m_stack.auxWhat().isEmpty() ? errorItem->error().what() : m_stack.auxWhat();
+    case Qt::ToolTipRole:
+        return toolTipForFrame(errorItem->modelPrivate()->findRelevantFrame(errorItem->error()));
+    default:
+        return QVariant();
+    }
+}
+
+const ErrorItem *StackItem::getErrorItem() const
+{
+    return static_cast<ErrorItem *>(parent());
+}
+
+
+FrameItem::FrameItem(const Frame &frame) : m_frame(frame)
+{
+}
+
+QVariant FrameItem::data(int column, int role) const
+{
+    if (column == Analyzer::DetailedErrorView::LocationColumn)
+        return location(m_frame, role);
+
+    // DiagnosticColumn
+    switch (role) {
+    case ErrorListModel::ErrorRole:
+        return QVariant::fromValue(getErrorItem()->error());
+    case Qt::DisplayRole: {
+        const int row = parent()->children().indexOf(const_cast<FrameItem *>(this)) + 1;
+        const int padding = static_cast<int>(std::log10(parent()->rowCount()))
+                - static_cast<int>(std::log10(row));
+        return QString::fromLatin1("%1%2: %3")
+                .arg(QString(padding, QLatin1Char(' ')))
+                .arg(row)
+                .arg(makeFrameName(m_frame, false));
+    }
+    case Qt::ToolTipRole:
+        return toolTipForFrame(m_frame);
+    default:
+        return QVariant();
+    }
+}
+
+const ErrorItem *FrameItem::getErrorItem() const
+{
+    for (const TreeItem *parentItem = parent(); parentItem; parentItem = parentItem->parent()) {
+        const ErrorItem * const errorItem = dynamic_cast<const ErrorItem *>(parentItem);
+        if (errorItem)
+            return errorItem;
+    }
+    QTC_CHECK(false);
+    return nullptr;
 }
 
 } // namespace XmlProtocol

src/plugins/valgrind/xmlprotocol/errorlistmodel.h

@@ -32,39 +32,25 @@
 #ifndef LIBVALGRIND_PROTOCOL_ERRORLISTMODEL_H
 #define LIBVALGRIND_PROTOCOL_ERRORLISTMODEL_H
 
-#include <QAbstractItemModel>
+#include <analyzerbase/detailederrorview.h>
+#include <utils/treemodel.h>
+
 #include <QSharedPointer>
 
 namespace Valgrind {
 namespace XmlProtocol {
 
 class Error;
+class ErrorListModelPrivate;
 class Frame;
 
-class ErrorListModel : public QAbstractItemModel
+class ErrorListModel : public Utils::TreeModel
 {
     Q_OBJECT
 
 public:
-    enum Column {
-        WhatColumn = 0,
-        LocationColumn,
-        AbsoluteFilePathColumn,
-        LineColumn,
-        UniqueColumn,
-        TidColumn,
-        KindColumn,
-        LeakedBlocksColumn,
-        LeakedBytesColumn,
-        HelgrindThreadIdColumn,
-        ColumnCount
-    };
-
     enum Role {
-        ErrorRole = Qt::UserRole,
-        AbsoluteFilePathRole,
-        FileRole,
-        LineRole
+        ErrorRole = Analyzer::DetailedErrorView::FullTextRole + 1,
     };
 
     class RelevantFrameFinder
@@ -80,26 +66,11 @@ public:
     QSharedPointer<const RelevantFrameFinder> relevantFrameFinder() const;
     void setRelevantFrameFinder(const QSharedPointer<const RelevantFrameFinder> &finder);
 
-    QVariant data(const QModelIndex &index, int role) const;
-    QVariant headerData(int section, Qt::Orientation orientation, int role) const;
-    QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const;
-    QModelIndex parent(const QModelIndex &child) const;
-    int rowCount(const QModelIndex &parent = QModelIndex()) const;
-    int columnCount(const QModelIndex &parent = QModelIndex()) const;
-    virtual bool removeRows(int row, int count, const QModelIndex &parent = QModelIndex());
-
-    Error error(const QModelIndex &index) const;
-
-    Frame findRelevantFrame(const Error &error) const;
-
-    void clear();
-
 public slots:
     void addError(const Valgrind::XmlProtocol::Error &error);
 
 private:
-    class Private;
-    Private *const d;
+    ErrorListModelPrivate *const d;
 };
 
 } // namespace XmlProtocol