ProjectExplorer: Clean up IOutputParser interface

- Remove unneeded/unused functions.
- De-virtualize where possible.

In particular, after untangling a number of self-referential
redirections, it became apparent that the outputAdded()
infrastructure was entirely unused.

Change-Id: I51e1beed008df2727b42494b087efa476342397e
Reviewed-by: hjk <hjk@qt.io>

src/plugins/cmakeprojectmanager/servermodereader.cpp

@@ -73,8 +73,6 @@ const int MAX_PROGRESS = 1400;
 
 ServerModeReader::ServerModeReader()
 {
-    connect(&m_parser, &CMakeParser::addOutput,
-            this, [](const QString &m) { Core::MessageManager::write(m); });
     connect(&m_parser, &CMakeParser::addTask, this, [this](const Task &t) {
         Task editable(t);
         if (!editable.file.isEmpty()) {

src/plugins/projectexplorer/abstractprocessstep.cpp

@@ -142,7 +142,6 @@ void AbstractProcessStep::setOutputParser(IOutputParser *parser)
     d->m_outputParserChain.reset(new AnsiFilterParser);
     d->m_outputParserChain->appendOutputParser(parser);
 
-    connect(d->m_outputParserChain.get(), &IOutputParser::addOutput, this, &AbstractProcessStep::outputAdded);
     connect(d->m_outputParserChain.get(), &IOutputParser::addTask, this, &AbstractProcessStep::taskAdded);
 }
 

src/plugins/projectexplorer/customparser.cpp

@@ -134,11 +134,6 @@ void CustomParser::stdOutput(const QString &line)
     IOutputParser::stdOutput(line);
 }
 
-void CustomParser::setWorkingDirectory(const QString &workingDirectory)
-{
-    m_workingDirectory = workingDirectory;
-}
-
 void CustomParser::setSettings(const CustomParserSettings &settings)
 {
     m_error = settings.error;
@@ -152,10 +147,9 @@ Core::Id CustomParser::id()
 
 FilePath CustomParser::absoluteFilePath(const QString &filePath) const
 {
-    if (m_workingDirectory.isEmpty())
+    if (workingDirectory().isEmpty())
         return FilePath::fromUserInput(filePath);
-
+    return workingDirectory().resolvePath(filePath);
-    return FilePath::fromString(m_workingDirectory).resolvePath(filePath);
 }
 
 bool CustomParser::hasMatch(const QString &line, CustomParserExpression::CustomParserChannel channel,
@@ -481,7 +475,7 @@ void ProjectExplorerPlugin::testCustomOutputParsers()
 
     CustomParser *parser = new CustomParser;
     parser->setSettings(settings);
-    parser->setWorkingDirectory(workDir);
+    parser->setWorkingDirectory(FilePath::fromString(workDir));
 
     OutputParserTester testbench;
     testbench.appendOutputParser(parser);

src/plugins/projectexplorer/customparser.h

@@ -89,8 +89,6 @@ public:
     void stdError(const QString &line) override;
     void stdOutput(const QString &line) override;
 
-    void setWorkingDirectory(const QString &workingDirectory) override;
-
     void setSettings(const CustomParserSettings &settings);
 
     static Core::Id id();
@@ -103,8 +101,6 @@ private:
 
     CustomParserExpression m_error;
     CustomParserExpression m_warning;
-
-    QString m_workingDirectory;
 };
 
 } // namespace ProjectExplorer

src/plugins/projectexplorer/gnumakeparser.cpp

@@ -56,12 +56,6 @@ GnuMakeParser::GnuMakeParser()
     QTC_CHECK(m_errorInMakefile.isValid());
 }
 
-void GnuMakeParser::setWorkingDirectory(const QString &workingDirectory)
-{
-    addDirectory(workingDirectory);
-    IOutputParser::setWorkingDirectory(workingDirectory);
-}
-
 bool GnuMakeParser::hasFatalErrors() const
 {
     return (m_fatalErrorCount > 0) || IOutputParser::hasFatalErrors();
@@ -180,7 +174,7 @@ void GnuMakeParser::taskAdded(const Task &task, int linkedLines, int skippedLine
 
     if (!filePath.isEmpty() && !QDir::isAbsolutePath(filePath)) {
         QFileInfoList possibleFiles;
-        foreach (const QString &dir, m_directories) {
+        foreach (const QString &dir, searchDirectories()) {
             QFileInfo candidate(dir + QLatin1Char('/') + filePath);
             if (candidate.exists()
                 && !possibleFiles.contains(candidate)) {
@@ -197,6 +191,14 @@ void GnuMakeParser::taskAdded(const Task &task, int linkedLines, int skippedLine
     IOutputParser::taskAdded(editable, linkedLines, skippedLines);
 }
 
+QStringList GnuMakeParser::searchDirectories() const
+{
+    QStringList dirs = m_directories;
+    if (!workingDirectory().isEmpty())
+        dirs << workingDirectory().toString();
+    return dirs;
+}
+
 } // ProjectExplorer
 
 #ifdef WITH_TESTS
@@ -210,11 +212,6 @@ void GnuMakeParser::taskAdded(const Task &task, int linkedLines, int skippedLine
 
 namespace ProjectExplorer {
 
-QStringList GnuMakeParser::searchDirectories() const
-{
-    return m_directories;
-}
-
 GnuMakeParserTester::GnuMakeParserTester(GnuMakeParser *p, QObject *parent) :
     QObject(parent),
     parser(p)

src/plugins/projectexplorer/gnumakeparser.h

@@ -42,8 +42,6 @@ public:
     void stdOutput(const QString &line) override;
     void stdError(const QString &line) override;
 
-    void setWorkingDirectory(const QString &workingDirectory) override;
-
     QStringList searchDirectories() const;
 
     bool hasFatalErrors() const override;

src/plugins/projectexplorer/ioutputparser.cpp

@@ -138,19 +138,7 @@ void IOutputParser::appendOutputParser(IOutputParser *parser)
     }
 
     m_parser = parser;
-    connect(parser, &IOutputParser::addOutput,
+    connect(parser, &IOutputParser::addTask, this, &IOutputParser::taskAdded);
-            this, &IOutputParser::outputAdded, Qt::DirectConnection);
-    connect(parser, &IOutputParser::addTask,
-            this, &IOutputParser::taskAdded, Qt::DirectConnection);
-}
-
-IOutputParser *IOutputParser::takeOutputParserChain()
-{
-    IOutputParser *parser = m_parser;
-    disconnect(parser, &IOutputParser::addOutput, this, &IOutputParser::outputAdded);
-    disconnect(parser, &IOutputParser::addTask, this, &IOutputParser::taskAdded);
-    m_parser = nullptr;
-    return parser;
 }
 
 IOutputParser *IOutputParser::childParser() const
@@ -163,12 +151,8 @@ void IOutputParser::setChildParser(IOutputParser *parser)
     if (m_parser != parser)
         delete m_parser;
     m_parser = parser;
-    if (parser) {
+    if (parser)
-        connect(parser, &IOutputParser::addOutput,
+        connect(parser, &IOutputParser::addTask, this, &IOutputParser::taskAdded);
-                this, &IOutputParser::outputAdded, Qt::DirectConnection);
-        connect(parser, &IOutputParser::addTask,
-                this, &IOutputParser::taskAdded, Qt::DirectConnection);
-    }
 }
 
 void IOutputParser::stdOutput(const QString &line)
@@ -183,11 +167,6 @@ void IOutputParser::stdError(const QString &line)
         m_parser->stdError(line);
 }
 
-void IOutputParser::outputAdded(const QString &string, BuildStep::OutputFormat format)
-{
-    emit addOutput(string, format);
-}
-
 void IOutputParser::taskAdded(const Task &task, int linkedOutputLines, int skipLines)
 {
     emit addTask(task, linkedOutputLines, skipLines);
@@ -201,15 +180,11 @@ bool IOutputParser::hasFatalErrors() const
     return m_parser && m_parser->hasFatalErrors();
 }
 
-void IOutputParser::setWorkingDirectory(const QString &workingDirectory)
-{
-    if (m_parser)
-        m_parser->setWorkingDirectory(workingDirectory);
-}
-
 void IOutputParser::setWorkingDirectory(const Utils::FilePath &fn)
 {
-    setWorkingDirectory(fn.toString());
+    m_workingDir = fn;
+    if (m_parser)
+        m_parser->setWorkingDirectory(fn);
 }
 
 void IOutputParser::flush()

src/plugins/projectexplorer/ioutputparser.h

@@ -28,7 +28,7 @@
 #include "projectexplorer_export.h"
 #include "buildstep.h"
 
-namespace Utils { class FilePath; }
+#include <utils/fileutils.h>
 
 namespace ProjectExplorer {
 class Task;
@@ -41,9 +41,7 @@ public:
     IOutputParser() = default;
     ~IOutputParser() override;
 
-    virtual void appendOutputParser(IOutputParser *parser);
+    void appendOutputParser(IOutputParser *parser);
-
-    IOutputParser *takeOutputParserChain();
 
     IOutputParser *childParser() const;
     void setChildParser(IOutputParser *parser);
@@ -52,25 +50,26 @@ public:
     virtual void stdError(const QString &line);
 
     virtual bool hasFatalErrors() const;
-    virtual void setWorkingDirectory(const QString &workingDirectory);
+
     void setWorkingDirectory(const Utils::FilePath &fn);
 
     void flush(); // flush out pending tasks
 
     static QString rightTrimmed(const QString &in);
 
+    virtual void taskAdded(const ProjectExplorer::Task &task, int linkedOutputLines = 0, int skipLines = 0);
+
 signals:
-    void addOutput(const QString &string, ProjectExplorer::BuildStep::OutputFormat format);
     void addTask(const ProjectExplorer::Task &task, int linkedOutputLines = 0, int skipLines = 0);
 
-public slots:
+protected:
-    virtual void outputAdded(const QString &string, ProjectExplorer::BuildStep::OutputFormat format);
+    Utils::FilePath workingDirectory() const { return m_workingDir; }
-    virtual void taskAdded(const ProjectExplorer::Task &task, int linkedOutputLines = 0, int skipLines = 0);
 
 private:
     virtual void doFlush();
 
     IOutputParser *m_parser = nullptr;
+    Utils::FilePath m_workingDir;
 };
 
 } // namespace ProjectExplorer

src/plugins/projectexplorer/makestep.cpp

@@ -106,7 +106,7 @@ bool MakeStep::init()
     IOutputParser *parser = target()->kit()->createOutputParser();
     if (parser)
         appendOutputParser(parser);
-    outputParser()->setWorkingDirectory(pp->effectiveWorkingDirectory().toString());
+    outputParser()->setWorkingDirectory(pp->effectiveWorkingDirectory());
 
     return AbstractProcessStep::init();
 }

src/plugins/projectexplorer/outputparser_test.cpp

@@ -38,8 +38,6 @@ static inline QByteArray msgFileComparisonFail(const Utils::FilePath &f1, const
     return result.toLocal8Bit();
 }
 
-OutputParserTester::OutputParserTester() = default;
-
 // test functions:
 void OutputParserTester::testParsing(const QString &lines,
                                      Channel inputChannel,
@@ -48,6 +46,10 @@ void OutputParserTester::testParsing(const QString &lines,
                                      const QString &childStdErrLines,
                                      const QString &outputLines)
 {
+    if (!m_terminator) {
+        m_terminator = new TestTerminator(this);
+        appendOutputParser(m_terminator);
+    }
     reset();
     Q_ASSERT(childParser());
 
@@ -60,18 +62,8 @@ void OutputParserTester::testParsing(const QString &lines,
     }
     childParser()->flush();
 
-     // first disconnect ourselves from the end of the parser chain again
+    // delete the parser(s) to test
-    IOutputParser *parser = this;
-    while ( (parser = parser->childParser()) ) {
-        if (parser->childParser() == this) {
-            childParser()->takeOutputParserChain();
-            break;
-        }
-    }
-    parser = nullptr;
     emit aboutToDeleteParser();
-
-    // then delete the parser(s) to test
     setChildParser(nullptr);
 
     QCOMPARE(m_receivedOutput, outputLines);
@@ -110,39 +102,11 @@ void OutputParserTester::testTaskMangling(const Task &input,
     }
 }
 
-void OutputParserTester::testOutputMangling(const QString &input,
-                                            const QString &output)
-{
-    reset();
-
-    childParser()->outputAdded(input, BuildStep::OutputFormat::Stdout);
-
-    QCOMPARE(m_receivedOutput, output);
-    QVERIFY(m_receivedStdErrChildLine.isNull());
-    QVERIFY(m_receivedStdOutChildLine.isNull());
-    QVERIFY(m_receivedTasks.isEmpty());
-}
-
 void OutputParserTester::setDebugEnabled(bool debug)
 {
     m_debug = debug;
 }
 
-void OutputParserTester::appendOutputParser(IOutputParser *parser)
-{
-    Q_ASSERT(!childParser());
-    parser->appendOutputParser(new TestTerminator(this));
-    IOutputParser::appendOutputParser(parser);
-}
-
-void OutputParserTester::outputAdded(const QString &line, BuildStep::OutputFormat format)
-{
-    Q_UNUSED(format)
-    if (!m_receivedOutput.isEmpty())
-        m_receivedOutput.append('\n');
-    m_receivedOutput.append(line);
-}
-
 void OutputParserTester::taskAdded(const Task &task, int linkedLines, int skipLines)
 {
     Q_UNUSED(linkedLines)

src/plugins/projectexplorer/outputparser_test.h

@@ -46,8 +46,6 @@ public:
         STDERR
     };
 
-    OutputParserTester();
-
     // test functions:
     void testParsing(const QString &lines, Channel inputChannel,
                      Tasks tasks,
@@ -56,18 +54,13 @@ public:
                      const QString &outputLines);
     void testTaskMangling(const Task &input,
                           const Task &output);
-    void testOutputMangling(const QString &input,
-                            const QString &output);
 
     void setDebugEnabled(bool);
 
-    void appendOutputParser(IOutputParser *parser) override;
-
 signals:
     void aboutToDeleteParser();
 
 private:
-    void outputAdded(const QString &string, ProjectExplorer::BuildStep::OutputFormat format) override;
     void taskAdded(const ProjectExplorer::Task &task, int linkedLines, int skipLines) override;
 
     void reset();
@@ -78,6 +71,7 @@ private:
     QString m_receivedStdOutChildLine;
     Tasks m_receivedTasks;
     QString m_receivedOutput;
+    TestTerminator *m_terminator = nullptr;
 
     friend class TestTerminator;
 };

src/plugins/qbsprojectmanager/qbsbuildstep.cpp

@@ -179,10 +179,6 @@ bool QbsBuildStep::init()
     m_activeFileTags = bc->activeFileTags();
     m_products = bc->products();
 
-    connect(m_parser, &ProjectExplorer::IOutputParser::addOutput,
-            this, [this](const QString &string, ProjectExplorer::BuildStep::OutputFormat format) {
-        emit addOutput(string, format);
-    });
     connect(m_parser, &ProjectExplorer::IOutputParser::addTask, this, &QbsBuildStep::addTask);
 
     return true;
@@ -383,7 +379,7 @@ void QbsBuildStep::handleProcessResult(
     if (success && !hasOutput)
         return;
 
-    m_parser->setWorkingDirectory(workingDir.toString());
+    m_parser->setWorkingDirectory(workingDir);
     emit addOutput(executable.toUserOutput() + ' '  + QtcProcess::joinArgs(arguments),
                    OutputFormat::Stdout);
     for (const QString &line : stdErr) {

src/plugins/qbsprojectmanager/qbsparser.cpp

@@ -29,6 +29,7 @@
 
 #include <utils/fileutils.h>
 
+#include <QDir>
 #include <QFileInfo>
 
 namespace QbsProjectManager {
@@ -39,21 +40,13 @@ QbsParser::QbsParser()
     setObjectName(QLatin1String("QbsParser"));
 }
 
-void QbsParser::setWorkingDirectory(const QString &workingDirectory)
+// TODO: Is this really needed? qbs never emits relative paths...
-{
-    m_workingDirectory = QDir(workingDirectory);
-    IOutputParser::setWorkingDirectory(workingDirectory);
-}
-
 void QbsParser::taskAdded(const ProjectExplorer::Task &task, int linkedLines, int skipLines)
 {
     ProjectExplorer::Task editable(task);
-
+    const QString filePath = task.file.toString();
-    QString filePath = task.file.toString();
-
     if (!filePath.isEmpty())
-        editable.file = Utils::FilePath::fromUserInput(m_workingDirectory.absoluteFilePath(filePath));
+        editable.file = workingDirectory().pathAppended(filePath);
-
     IOutputParser::taskAdded(editable, linkedLines, skipLines);
 }
 

src/plugins/qbsprojectmanager/qbsparser.h

@@ -29,8 +29,6 @@
 
 #include <projectexplorer/ioutputparser.h>
 
-#include <QDir>
-
 namespace QbsProjectManager {
 namespace Internal {
 
@@ -42,10 +40,7 @@ public:
     explicit QbsParser();
 
 private:
-    void setWorkingDirectory(const QString &workingDirectory) override;
     void taskAdded(const ProjectExplorer::Task &task, int linkedLines, int skipLines) override;
-
-    QDir m_workingDirectory;
 };
 
 } // namespace Internal

src/plugins/qmakeprojectmanager/qmakestep.cpp

@@ -338,7 +338,7 @@ void QMakeStep::runNextCommand()
     case State::RUN_MAKE_QMAKE_ALL:
         {
             auto *parser = new GnuMakeParser;
-            parser->setWorkingDirectory(processParameters()->workingDirectory().toString());
+            parser->setWorkingDirectory(processParameters()->workingDirectory());
             setOutputParser(parser);
             m_nextState = State::POST_PROCESS;
             startOneCommand(m_makeCommand);