Rework the parsing and fixing detection of changes

Additionally
* fixed some deletion of pointers
* const correctness
* small refactorings for readability
* better separation of responsibilities

plugins/autotest/testcodeparser.cpp

@@ -30,6 +30,8 @@
 
 #include <projectexplorer/session.h>
 
+#include <utils/textfileformat.h>
+
 namespace Autotest {
 namespace Internal {
 
@@ -40,17 +42,18 @@ TestCodeParser::TestCodeParser(TestTreeModel *parent)
 {
 }
 
+TestCodeParser::~TestCodeParser()
+{
+    clearMaps();
+}
+
 void TestCodeParser::updateTestTree()
 {
     qDebug("updating TestTreeModel");
-    m_model->beginResetModel();
+
-    qDeleteAll(m_cppDocMap);
+    clearMaps();
-    m_cppDocMap.clear();
+    m_model->removeAllAutoTests();
-    QModelIndex autoTestRootIndex = m_model->index(0, 0);
+    const ProjectExplorer::SessionManager *session = ProjectExplorer::SessionManager::instance();
-    TestTreeItem *autoTestRootItem = static_cast<TestTreeItem *>(autoTestRootIndex.internalPointer());
-    autoTestRootItem->removeChildren();
-    m_model->endResetModel();
-    ProjectExplorer::SessionManager *session = ProjectExplorer::SessionManager::instance();
     if (!session || !session->hasProjects())
         return;
 
@@ -62,12 +65,30 @@ void TestCodeParser::updateTestTree()
 
 /****** scan for QTest related stuff helpers ******/
 
+static QByteArray getFileContent(QString filePath)
+{
+    QByteArray fileContent;
+    CppTools::CppModelManager *cppMM = CppTools::CppModelManager::instance();
+    CppTools::WorkingCopy wc = cppMM->workingCopy();
+    if (wc.contains(filePath)) {
+        fileContent = wc.source(filePath);
+    } else {
+        QString error;
+        const QTextCodec *codec = Core::EditorManager::defaultTextCodec();
+        if (Utils::TextFileFormat::readFileUTF8(filePath, codec, &fileContent, &error)
+                != Utils::TextFileFormat::ReadSuccess) {
+            qDebug() << "Failed to read file" << filePath << ":" << error;
+        }
+    }
+    return fileContent;
+}
+
 static bool includesQtTest(const CPlusPlus::Document::Ptr &doc,
                            const CppTools::CppModelManager *cppMM)
 {
-    QList<CPlusPlus::Document::Include> includes = doc->resolvedIncludes();
+    const QList<CPlusPlus::Document::Include> includes = doc->resolvedIncludes();
 
-    foreach (CPlusPlus::Document::Include inc, includes) {
+    foreach (const CPlusPlus::Document::Include inc, includes) {
         // TODO this short cut works only for #include <QtTest>
         // bad, as there could be much more different approaches
         if (inc.unresolvedFileName() == QLatin1String("QtTest")
@@ -78,8 +99,8 @@ static bool includesQtTest(const CPlusPlus::Document::Ptr &doc,
 
     if (cppMM) {
         CPlusPlus::Snapshot snapshot = cppMM->snapshot();
-        QSet<QString> includes = snapshot.allIncludesForDocument(doc->fileName());
+        const QSet<QString> allIncludes = snapshot.allIncludesForDocument(doc->fileName());
-        foreach (QString include, includes) {
+        foreach (const QString include, allIncludes) {
             if (include.endsWith(QLatin1String("QtTest/qtest.h"))) {
                 return true;
             }
@@ -91,11 +112,9 @@ static bool includesQtTest(const CPlusPlus::Document::Ptr &doc,
 static bool qtTestLibDefined(const CppTools::CppModelManager *cppMM,
                              const QString &fileName)
 {
-    QList<CppTools::ProjectPart::Ptr> parts = cppMM->projectPart(fileName);
+    const QList<CppTools::ProjectPart::Ptr> parts = cppMM->projectPart(fileName);
-    if (parts.size() > 0) {
+    if (parts.size() > 0)
-        QByteArray projDefines = parts.at(0)->projectDefines;
+        return parts.at(0)->projectDefines.contains("#define QT_TESTLIB_LIB 1");
-        return projDefines.contains("#define QT_TESTLIB_LIB 1");
-    }
     return false;
 }
 
@@ -103,33 +122,16 @@ static QString testClass(const CPlusPlus::Document::Ptr &doc)
 {
     static QByteArray qtTestMacros[] = {"QTEST_MAIN", "QTEST_APPLESS_MAIN", "QTEST_GUILESS_MAIN"};
     QString tC;
-    QList<CPlusPlus::Document::MacroUse> macros = doc->macroUses();
+    const QList<CPlusPlus::Document::MacroUse> macros = doc->macroUses();
 
-    foreach (CPlusPlus::Document::MacroUse macro, macros) {
+    foreach (const CPlusPlus::Document::MacroUse macro, macros) {
         if (!macro.isFunctionLike())
             continue;
-        QByteArray name = macro.macro().name();
+        const QByteArray name = macro.macro().name();
         if (name == qtTestMacros[0] || name == qtTestMacros[1] || name == qtTestMacros[2]) {
             CPlusPlus::Document::Block arg = macro.arguments().at(0);
-            CppTools::CppModelManager *cppMM = CppTools::CppModelManager::instance();
+            tC = QLatin1String(getFileContent(doc->fileName()).mid(arg.bytesBegin(),
-
+                                                                   arg.bytesEnd() - arg.bytesBegin()));
-            // TODO high costs....?!
-            CppTools::WorkingCopy wc = cppMM->workingCopy();
-            if (wc.contains(doc->fileName())) {
-                QByteArray src = wc.source(doc->fileName());
-                tC = QLatin1String(src.mid(arg.bytesBegin(), arg.bytesEnd() - arg.bytesBegin()));
-            } else {
-                QFile current(doc->fileName());
-                if (current.exists() && current.open(QFile::ReadOnly)) {
-                    CPlusPlus::Document::Block arg = macro.arguments().at(0);
-                    if (current.seek(arg.bytesBegin())) {
-                        QByteArray res = current.read(arg.bytesEnd() - arg.bytesBegin());
-                        if (!res.isEmpty())
-                            tC = QLatin1String(res);
-                    }
-                    current.close();
-                }
-            }
             break;
         }
     }
@@ -141,53 +143,64 @@ static QString testClass(const CPlusPlus::Document::Ptr &doc)
 void TestCodeParser::checkDocumentForTestCode(CPlusPlus::Document::Ptr doc)
 {
     const QString file = doc->fileName();
-    CppTools::CppModelManager *cppMM = CppTools::CppModelManager::instance();
+    const CppTools::CppModelManager *cppMM = CppTools::CppModelManager::instance();
 
     if (includesQtTest(doc, cppMM) && qtTestLibDefined(cppMM, file)) {
-        QString tc = testClass(doc);
+        QString tc(testClass(doc));
         if (tc.isEmpty()) {
             // one might have used an approach without macros or defined own macros
-            QString src = QLatin1String(doc->utf8Source()); // does not work anymore - src is always ""
+            const CPlusPlus::Snapshot snapshot = cppMM->snapshot();
-            if (src.contains(QLatin1String("QTest::qExec"))) {
+            const QByteArray fileContent = getFileContent(file);
-                qDebug() << "Src\n===============\n" << src << "\n=============\n";
+            doc = snapshot.preprocessedDocument(fileContent, file);
-                // TODO extract the class name by using the AST - using regex is too problematic as this
+            doc->check();
-                // does not take comments and typedefs or whatever into account....
+            CPlusPlus::AST *ast = doc->translationUnit()->ast();
-                qDebug() << "Currently not supported approach... (self-defined macro / QTest::qExec() call/...)";
+            TestAstVisitor astVisitor(doc);
+            astVisitor.accept(ast);
+            tc = astVisitor.className();
         }
-        } else {
+        if (!tc.isEmpty()) {
-            QModelIndex autoTestRootIndex = m_model->index(0, 0);
+            // construct the new/modified TestTreeItem
+            const QModelIndex autoTestRootIndex = m_model->index(0, 0);
             TestTreeItem *autoTestRootItem = static_cast<TestTreeItem *>(autoTestRootIndex.internalPointer());
             TestTreeItem *ttItem = new TestTreeItem(tc, file, TestTreeItem::TEST_CLASS,
                                                     autoTestRootItem);
+            QString declFileName;
             CPlusPlus::TypeOfExpression toe;
             toe.init(doc, cppMM->snapshot());
             CPlusPlus::Document::Ptr declaringDoc = doc;
-            QList<CPlusPlus::LookupItem>  toeItems = toe(tc.toUtf8(), doc->globalNamespace());
+            const QList<CPlusPlus::LookupItem>  toeItems = toe(tc.toUtf8(), doc->globalNamespace());
             if (toeItems.size()) {
                 CPlusPlus::Class *toeClass = toeItems.first().declaration()->asClass();
-                QString declFileName = QLatin1String(toeClass->fileId()->chars(),
+                declFileName = QLatin1String(toeClass->fileId()->chars(),
                                              toeClass->fileId()->size());
                 declaringDoc = cppMM->snapshot().document(declFileName);
                 ttItem->setFilePath(declFileName);
                 ttItem->setLine(toeClass->line());
                 ttItem->setColumn(toeClass->column() - 1);
             }
-            if (declaringDoc.isNull())
+            if (declaringDoc.isNull()) {
+                delete ttItem;
                 return;
+            }
             TestVisitor myVisitor(tc);
             myVisitor.accept(declaringDoc->globalNamespace());
-            QMap<QString, TestCodeLocation> privSlots = myVisitor.privateSlots();
+            const QMap<QString, TestCodeLocation> privSlots = myVisitor.privateSlots();
             foreach (const QString privS, privSlots.keys()) {
-                TestCodeLocation location = privSlots.value(privS);
+                const TestCodeLocation location = privSlots.value(privS);
                 TestTreeItem *ttSub = new TestTreeItem(privS, location.m_fileName,
                                                        TestTreeItem::TEST_FUNCTION, ttItem);
                 ttSub->setLine(location.m_line);
                 ttSub->setColumn(location.m_column);
                 ttItem->appendChild(ttSub);
             }
+
+            // TODO refactoring?
+            // update model and internal map
+            TestInfo *info;
+            int count;
             if (m_cppDocMap.contains(file)) {
-            TestInfo *info = m_cppDocMap[file];
+                info = m_cppDocMap[file];
-                int count = autoTestRootItem->childCount();
+                count = autoTestRootItem->childCount();
                 for (int i = 0; i < count; ++i) {
                     TestTreeItem *currentItem = autoTestRootItem->child(i);
                     if (currentItem->filePath() == file) {
@@ -199,13 +212,40 @@ void TestCodeParser::checkDocumentForTestCode(CPlusPlus::Document::Ptr doc)
                         break;
                     }
                 }
+                info = m_cppDocMap[declFileName];
+                count = autoTestRootItem->childCount();
+                for (int i = 0; i < count; ++i) {
+                    TestTreeItem *currentItem = autoTestRootItem->child(i);
+                    if (currentItem->filePath() == declFileName) {
+                        m_model->modifyAutoTestSubtree(i, ttItem);
+                        TestInfo *ti = new TestInfo(tc, privSlots.keys(),
+                                                    doc->revision(),
+                                                    doc->editorRevision());
+                        ti->setReferencingFile(file);
+                        m_cppDocMap.insert(declFileName, ti);
+                        delete info;
+                        break;
+                    }
+                }
                 delete ttItem;
             } else {
-                m_model->beginInsertRows(autoTestRootIndex, autoTestRootItem->childCount(), autoTestRootItem->childCount());
+                m_model->addAutoTest(ttItem);
-                autoTestRootItem->appendChild(ttItem);
-                m_model->endInsertRows();
                 m_cppDocMap.insert(file, new TestInfo(tc, privSlots.keys(),
                                                       doc->revision(), doc->editorRevision()));
+                TestInfo *ti = new TestInfo(tc, privSlots.keys(),
+                                            doc->revision(), doc->editorRevision());
+                ti->setReferencingFile(file);
+                m_cppDocMap.insert(declFileName, ti);
+            }
+        }
+    } else {
+        // could not find the class to test, but QTest is included and QT_TESTLIB_LIB defined
+        // maybe file is only a referenced file
+        if (m_cppDocMap.contains(file)) {
+            const TestInfo *info = m_cppDocMap[file];
+            CPlusPlus::Snapshot snapshot = cppMM->snapshot();
+            if (snapshot.contains(info->referencingFile())) {
+                checkDocumentForTestCode(snapshot.find(info->referencingFile()).value());
             }
         }
     }
@@ -215,24 +255,22 @@ void TestCodeParser::onDocumentUpdated(CPlusPlus::Document::Ptr doc)
 {
     if (!m_currentProject)
         return;
-    QString fileName = doc->fileName();
+    const QString fileName = doc->fileName();
-    if (!m_cppDocMap.contains(fileName)) {
+    if (m_cppDocMap.contains(fileName)) {
-        if (!m_currentProject->files(ProjectExplorer::Project::AllFiles).contains(fileName)) {
-            return;
-        }
-    } else {
         if (m_cppDocMap[fileName]->revision() == doc->revision()
                 && m_cppDocMap[fileName]->editorRevision() == doc->editorRevision()) {
             qDebug("Skipped due revision equality"); // added to verify if this ever happens..
             return;
         }
+    } else if (!m_currentProject->files(ProjectExplorer::Project::AllFiles).contains(fileName)) {
+        return;
     }
     checkDocumentForTestCode(doc);
 }
 
 void TestCodeParser::removeFiles(const QStringList &files)
 {
-    foreach (QString file, files) {
+    foreach (const QString file, files) {
         if (m_cppDocMap.contains(file)) {
             TestInfo *info = m_cppDocMap.value(file);
             m_cppDocMap.remove(file);
@@ -244,11 +282,11 @@ void TestCodeParser::removeFiles(const QStringList &files)
 
 void TestCodeParser::scanForTests()
 {
-    QStringList list = m_currentProject->files(ProjectExplorer::Project::AllFiles);
+    const QStringList list = m_currentProject->files(ProjectExplorer::Project::AllFiles);
     CppTools::CppModelManager *cppMM = CppTools::CppModelManager::instance();
     CPlusPlus::Snapshot snapshot = cppMM->snapshot();
 
-    foreach (QString file, list) {
+    foreach (const QString file, list) {
         if (snapshot.contains(file)) {
             CPlusPlus::Document::Ptr doc = snapshot.find(file).value();
             checkDocumentForTestCode(doc);
@@ -256,5 +294,15 @@ void TestCodeParser::scanForTests()
     }
 }
 
+void TestCodeParser::clearMaps()
+{
+    QMap<QString, TestInfo *>::iterator it = m_cppDocMap.begin();
+    while (it != m_cppDocMap.end()) {
+        delete it.value();
+        ++it;
+    }
+    m_cppDocMap.clear();
+}
+
 } // namespace Internal
 } // namespace Autotest

plugins/autotest/testcodeparser.h

@@ -39,6 +39,7 @@ class TestCodeParser : public QObject
     Q_OBJECT
 public:
     explicit TestCodeParser(TestTreeModel *parent = 0);
+    virtual ~TestCodeParser();
 
 signals:
 
@@ -51,6 +52,7 @@ public slots:
 
 private:
     void scanForTests();
+    void clearMaps();
 
     TestTreeModel *m_model;
     QMap<QString, TestInfo*> m_cppDocMap;

plugins/autotest/testinfo.h

@@ -39,12 +39,15 @@ public:
     void setRevision(unsigned revision) { m_revision = revision; }
     unsigned editorRevision() const { return m_editorRevision; }
     void setEditorRevision(unsigned editorRevision) { m_editorRevision = editorRevision; }
+    const QString referencingFile() const { return m_referencingFile; }
+    void setReferencingFile(const QString &refFile) {m_referencingFile = refFile; }
 
 private:
     QString m_className;
     QStringList m_functions;
     unsigned m_revision;
     unsigned m_editorRevision;
+    QString m_referencingFile;
 };
 
 } // namespace Internal

plugins/autotest/testrunner.cpp

@@ -86,7 +86,9 @@ void TestRunner::runTests()
     ProjectExplorer::Internal::ProjectExplorerSettings pes = pep->projectExplorerSettings();
     if (pes.buildBeforeDeploy) {
         if (!project->hasActiveBuildSettings()) {
-            qDebug() << "no active build settings...???"; // let it configure?
+            TestResultsPane::instance()->addTestResult(
+                        TestResult(QString(), QString(), QString(), ResultType::MESSAGE_FATAL,
+                                   tr("*** Project is not configured - canceling Test Run ***")));
             return;
         }
         buildProject(project);

plugins/autotest/testtreemodel.cpp

@@ -263,7 +263,7 @@ static void addProjectInformation(TestConfiguration *config, const QString &file
         project = projParts.at(0)->project; // necessary to grab this here? or should this be the current active startup project anyway?
     }
     if (project) {
-        ProjectExplorer::Target *target = project->activeTarget();
+        if (auto target = project->activeTarget()) {
             ProjectExplorer::BuildTargetInfoList appTargets = target->applicationTargets();
             foreach (ProjectExplorer::BuildTargetInfo bti, appTargets.list) {
                 if (bti.isValid() && bti.projectFilePath.toString() == proFile) {
@@ -288,6 +288,7 @@ static void addProjectInformation(TestConfiguration *config, const QString &file
                 }
             }
         }
+    }
     config->setTargetFile(targetFile);
     config->setTargetName(targetName);
     config->setWorkingDirectory(workDir);
@@ -438,5 +439,19 @@ void TestTreeModel::removeAutoTestSubtreeByFilePath(const QString &file)
     }
 }
 
+void TestTreeModel::addAutoTest(TestTreeItem *newItem)
+{
+    beginInsertRows(index(0, 0), m_autoTestRootItem->childCount(), m_autoTestRootItem->childCount());
+    m_autoTestRootItem->appendChild(newItem);
+    endInsertRows();
+}
+
+void TestTreeModel::removeAllAutoTests()
+{
+    beginResetModel();
+    m_autoTestRootItem->removeChildren();
+    endResetModel();
+}
+
 } // namespace Internal
 } // namespace Autotest

plugins/autotest/testtreemodel.h

@@ -61,13 +61,15 @@ public:
     QList<TestConfiguration *> getAllTestCases() const;
     QList<TestConfiguration *> getSelectedTests() const;
 
+    void modifyAutoTestSubtree(int row, TestTreeItem *newItem);
+    void removeAutoTestSubtreeByFilePath(const QString &file);
+    void addAutoTest(TestTreeItem *newItem);
+    void removeAllAutoTests();
 signals:
 
 public slots:
 
 private:
-    void modifyAutoTestSubtree(int row, TestTreeItem *newItem);
-    void removeAutoTestSubtreeByFilePath(const QString &file);
     explicit TestTreeModel(QObject *parent = 0);
 
     TestTreeItem *m_rootItem;
@@ -75,7 +77,6 @@ private:
 //    TestTreeItem *m_quickTestRootItem;
     TestCodeParser *m_parser;
 
-    friend class TestCodeParser;
 };
 
 } // namespace Internal

plugins/autotest/testvisitor.cpp

@@ -18,9 +18,13 @@
 
 #include "testvisitor.h"
 
+#include <cplusplus/FullySpecifiedType.h>
 #include <cplusplus/LookupContext.h>
 #include <cplusplus/Overview.h>
 #include <cplusplus/Symbols.h>
+#include <cplusplus/TypeOfExpression.h>
+
+#include <cpptools/cppmodelmanager.h>
 
 #include <QList>
 
@@ -57,7 +61,7 @@ bool TestVisitor::visit(CPlusPlus::Class *symbol)
 
         if (auto func = type->asFunctionType()) {
             if (func->isSlot() && member->isPrivate()) {
-                QString name = o.prettyName(func->name());
+                const QString name = o.prettyName(func->name());
                 if (!ignoredFunctions.contains(name) && !name.endsWith(QLatin1String("_data"))) {
                     TestCodeLocation location;
                     location.m_fileName = QLatin1String(member->fileName());
@@ -71,5 +75,54 @@ bool TestVisitor::visit(CPlusPlus::Class *symbol)
     return true;
 }
 
+TestAstVisitor::TestAstVisitor(CPlusPlus::Document::Ptr doc)
+    : ASTVisitor(doc->translationUnit()),
+      m_currentDoc(doc)
+{
+}
+
+TestAstVisitor::~TestAstVisitor()
+{
+}
+
+bool TestAstVisitor::visit(CPlusPlus::CallAST *ast)
+{
+    if (!m_currentScope || m_currentDoc.isNull())
+        return false;
+
+    if (auto expressionAST = ast->base_expression) {
+        if (auto idExpressionAST = expressionAST->asIdExpression()) {
+            if (auto qualifiedNameAST = idExpressionAST->name->asQualifiedName()) {
+                const CPlusPlus::Overview o;
+                const QString prettyName = o.prettyName(qualifiedNameAST->name);
+                if (prettyName == QLatin1String("QTest::qExec")) {
+                    if (auto expressionListAST = ast->expression_list) {
+                        // first argument is the one we need
+                        if (auto argumentExpressionAST = expressionListAST->value) {
+                            CPlusPlus::TypeOfExpression toe;
+                            CppTools::CppModelManager *cppMM = CppTools::CppModelManager::instance();
+                            toe.init(m_currentDoc, cppMM->snapshot());
+                            QList<CPlusPlus::LookupItem> toeItems
+                                    = toe(argumentExpressionAST, m_currentDoc, m_currentScope);
+
+                            if (toeItems.size()) {
+                                if (auto pointerType = toeItems.first().type()->asPointerType())
+                                    m_className = o.prettyType(pointerType->elementType());
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+    return false;
+}
+
+bool TestAstVisitor::visit(CPlusPlus::CompoundStatementAST *ast)
+{
+    m_currentScope = ast->symbol->asScope();
+    return true;
+}
+
 } // namespace Internal
 } // namespace Autotest

plugins/autotest/testvisitor.h

@@ -19,6 +19,9 @@
 #ifndef TESTVISITOR_H
 #define TESTVISITOR_H
 
+#include <cplusplus/ASTVisitor.h>
+#include <cplusplus/CppDocument.h>
+#include <cplusplus/Scope.h>
 #include <cplusplus/SymbolVisitor.h>
 
 #include <QMap>
@@ -48,6 +51,24 @@ private:
     QMap<QString, TestCodeLocation> m_privSlots;
 };
 
+class TestAstVisitor : public CPlusPlus::ASTVisitor
+{
+public:
+    TestAstVisitor(CPlusPlus::Document::Ptr doc);
+    virtual ~TestAstVisitor();
+
+    bool visit(CPlusPlus::CallAST *ast);
+    bool visit(CPlusPlus::CompoundStatementAST *ast);
+
+    QString className() const { return m_className; }
+
+private:
+    QString m_className;
+    CPlusPlus::Scope *m_currentScope;
+    CPlusPlus::Document::Ptr m_currentDoc;
+
+};
+
 } // namespace Internal
 } // namespace Autotest