ClangTools: Clean up ClangToolRunner

Change-Id: Icf0b8910ed236e48cfa775983e181491e6e9b6b0
Reviewed-by: Cristian Adam <cristian.adam@qt.io>
Reviewed-by: Nikolai Kosjar <nikolai.kosjar@qt.io>

src/plugins/clangtools/clangtidyclazyrunner.cpp

@@ -126,7 +126,7 @@ ClangTidyRunner::ClangTidyRunner(const ClangDiagnosticConfig &config, QObject *p
     setArgsCreator([this, config](const QStringList &baseOptions) {
         return QStringList()
             << tidyChecksArguments(config)
-            << mainToolArguments(filePath(), m_logFile)
+            << mainToolArguments(fileToAnalyze(), outputFilePath())
             << "--"
             << clangArguments(config, baseOptions);
     });
@@ -141,7 +141,7 @@ ClazyStandaloneRunner::ClazyStandaloneRunner(const ClangDiagnosticConfig &config
     setArgsCreator([this, config](const QStringList &baseOptions) {
         return QStringList()
             << clazyChecksArguments(config)
-            << mainToolArguments(filePath(), m_logFile)
+            << mainToolArguments(fileToAnalyze(), outputFilePath())
             << "--"
             << clangArguments(config, baseOptions);
     });
@@ -154,10 +154,10 @@ ClazyPluginRunner::ClazyPluginRunner(const ClangDiagnosticConfig &config, QObjec
     setOutputFileFormat(OutputFileFormat::Serialized);
     setExecutable(Core::ICore::clangExecutable(CLANG_BINDIR));
     setArgsCreator([this, config](const QStringList &baseOptions) {
-        return serializeDiagnosticsArguments(baseOptions, m_logFile)
+        return serializeDiagnosticsArguments(baseOptions, outputFilePath())
             << clazyPluginArguments(config)
             << clangArguments(config, baseOptions)
-            << QDir::toNativeSeparators(filePath());
+            << QDir::toNativeSeparators(fileToAnalyze());
     });
 }
 

src/plugins/clangtools/clangtoolruncontrol.cpp

@@ -403,22 +403,22 @@ void ClangToolRunWorker::analyzeNextFile()
 void ClangToolRunWorker::onRunnerFinishedWithSuccess(const QString &filePath)
 {
     auto runner = qobject_cast<ClangToolRunner *>(sender());
-    const QString logFilePath = runner->logFilePath();
+    const QString outputFilePath = runner->outputFilePath();
-    qCDebug(LOG) << "onRunnerFinishedWithSuccess:" << logFilePath;
+    qCDebug(LOG) << "onRunnerFinishedWithSuccess:" << outputFilePath;
 
     QString errorMessage;
     const Diagnostics diagnostics = tool()->read(runner->outputFileFormat(),
-                                                 logFilePath,
+                                                 outputFilePath,
                                                  filePath,
                                                  m_projectFiles,
                                                  &errorMessage);
-    QFile::remove(logFilePath); // Clean-up.
+    QFile::remove(outputFilePath); // Clean-up.
 
     if (!errorMessage.isEmpty()) {
         m_filesAnalyzed.remove(filePath);
         m_filesNotAnalyzed.insert(filePath);
         qCDebug(LOG) << "onRunnerFinishedWithSuccess: Error reading log file:" << errorMessage;
-        const QString filePath = qobject_cast<ClangToolRunner *>(sender())->filePath();
+        const QString filePath = qobject_cast<ClangToolRunner *>(sender())->fileToAnalyze();
         appendMessage(tr("Failed to analyze \"%1\": %2").arg(filePath, errorMessage),
                       Utils::StdErrFormat);
     } else {
@@ -438,17 +438,17 @@ void ClangToolRunWorker::onRunnerFinishedWithFailure(const QString &errorMessage
                            << errorMessage << '\n' << errorDetails;
 
     auto *toolRunner = qobject_cast<ClangToolRunner *>(sender());
-    const QString filePath = toolRunner->filePath();
+    const QString fileToAnalyze = toolRunner->fileToAnalyze();
-    const QString logFilePath = toolRunner->logFilePath();
+    const QString outputFilePath = toolRunner->outputFilePath();
 
     // Even in the error case the log file was created, so clean it up here, too.
-    QFile::remove(logFilePath);
+    QFile::remove(outputFilePath);
 
-    m_filesAnalyzed.remove(filePath);
+    m_filesAnalyzed.remove(fileToAnalyze);
-    m_filesNotAnalyzed.insert(filePath);
+    m_filesNotAnalyzed.insert(fileToAnalyze);
     m_success = false;
 
-    const QString message = tr("Failed to analyze \"%1\": %2").arg(filePath, errorMessage);
+    const QString message = tr("Failed to analyze \"%1\": %2").arg(fileToAnalyze, errorMessage);
     appendMessage(message, Utils::StdErrFormat);
     appendMessage(errorDetails, Utils::StdErrFormat);
     TaskHub::addTask(Task::Error, message, Debugger::Constants::ANALYZERTASK_ID);

src/plugins/clangtools/clangtoolrunner.cpp

@@ -53,21 +53,20 @@ static QString finishedDueToCrash(const QString &name)
     return ClangToolRunner::tr("%1 crashed.").arg(name);
 }
 
-QString finishedWithBadExitCode(const QString &name, int exitCode)
+static QString finishedWithBadExitCode(const QString &name, int exitCode)
 {
     return ClangToolRunner::tr("%1 finished with exit code: %2.").arg(name).arg(exitCode);
 }
 
-void ClangToolRunner::init(const QString &clangLogFileDir,
+void ClangToolRunner::init(const QString &outputDirPath,
                            const Utils::Environment &environment)
 {
-    m_clangLogFileDir = clangLogFileDir;
+    m_outputDirPath = outputDirPath;
-    QTC_CHECK(!m_clangLogFileDir.isEmpty());
+    QTC_CHECK(!m_outputDirPath.isEmpty());
 
     m_process.setProcessChannelMode(QProcess::MergedChannels);
     m_process.setProcessEnvironment(environment.toProcessEnvironment());
-    m_process.setWorkingDirectory(m_clangLogFileDir); // Current clang-cl puts log file into working dir.
+    m_process.setWorkingDirectory(m_outputDirPath); // Current clang-cl puts log file into working dir.
-    connect(&m_process, &QProcess::started, this, &ClangToolRunner::onProcessStarted);
     connect(&m_process, QOverload<int, QProcess::ExitStatus>::of(&QProcess::finished),
             this, &ClangToolRunner::onProcessFinished);
     connect(&m_process, &QProcess::errorOccurred, this, &ClangToolRunner::onProcessError);
@@ -79,63 +78,10 @@ ClangToolRunner::~ClangToolRunner()
     Utils::SynchronousProcess::stopProcess(m_process);
 }
 
-bool ClangToolRunner::run(const QString &filePath, const QStringList &compilerOptions)
+static QString createOutputFilePath(const QString &dirPath, const QString &fileToAnalyze)
 {
-    QTC_ASSERT(!m_executable.isEmpty(), return false);
+    const QString fileName = QFileInfo(fileToAnalyze).fileName();
-    QTC_CHECK(!compilerOptions.contains(QLatin1String("-o")));
+    const QString fileTemplate = dirPath
-    QTC_CHECK(!compilerOptions.contains(filePath));
-
-    m_filePath = filePath;
-    m_processOutput.clear();
-
-    m_logFile = createLogFile(filePath);
-    QTC_ASSERT(!m_logFile.isEmpty(), return false);
-    const QStringList arguments = m_argsCreator(compilerOptions);
-    m_commandLine = Utils::QtcProcess::joinArgs(QStringList(m_executable) + arguments);
-
-    qCDebug(LOG).noquote() << "Starting" << m_commandLine;
-    m_process.start(m_executable, arguments);
-    return true;
-}
-
-void ClangToolRunner::onProcessStarted()
-{
-    emit started();
-}
-
-void ClangToolRunner::onProcessFinished(int exitCode, QProcess::ExitStatus exitStatus)
-{
-    if (exitStatus == QProcess::NormalExit) {
-        if (exitCode == 0) {
-            qCDebug(LOG).noquote() << "Output:\n" << Utils::SynchronousProcess::normalizeNewlines(
-                                                        QString::fromLocal8Bit(m_processOutput));
-            emit finishedWithSuccess(m_filePath);
-        } else {
-            emit finishedWithFailure(finishedWithBadExitCode(m_name, exitCode),
-                                     processCommandlineAndOutput());
-        }
-    } else { // == QProcess::CrashExit
-        emit finishedWithFailure(finishedDueToCrash(m_name), processCommandlineAndOutput());
-    }
-}
-
-void ClangToolRunner::onProcessError(QProcess::ProcessError error)
-{
-    if (error == QProcess::Crashed)
-        return; // handled by slot of finished()
-
-    emit finishedWithFailure(generalProcessError(m_name), processCommandlineAndOutput());
-}
-
-void ClangToolRunner::onProcessOutput()
-{
-    m_processOutput.append(m_process.readAll());
-}
-
-QString ClangToolRunner::createLogFile(const QString &filePath) const
-{
-    const QString fileName = QFileInfo(filePath).fileName();
-    const QString fileTemplate = m_clangLogFileDir
             + QLatin1String("/report-") + fileName + QLatin1String("-XXXXXX");
 
     Utils::TemporaryFile temporaryFile("clangtools");
@@ -148,15 +94,62 @@ QString ClangToolRunner::createLogFile(const QString &filePath) const
     return QString();
 }
 
-QString ClangToolRunner::processCommandlineAndOutput() const
+bool ClangToolRunner::run(const QString &fileToAnalyze, const QStringList &compilerOptions)
+{
+    QTC_ASSERT(!m_executable.isEmpty(), return false);
+    QTC_CHECK(!compilerOptions.contains(QLatin1String("-o")));
+    QTC_CHECK(!compilerOptions.contains(fileToAnalyze));
+
+    m_fileToAnalyze = fileToAnalyze;
+    m_processOutput.clear();
+
+    m_outputFilePath = createOutputFilePath(m_outputDirPath, fileToAnalyze);
+    QTC_ASSERT(!m_outputFilePath.isEmpty(), return false);
+    const QStringList arguments = m_argsCreator(compilerOptions);
+    m_commandLine = Utils::QtcProcess::joinArgs(QStringList(m_executable) + arguments);
+
+    qCDebug(LOG).noquote() << "Starting" << m_commandLine;
+    m_process.start(m_executable, arguments);
+    return true;
+}
+
+void ClangToolRunner::onProcessFinished(int exitCode, QProcess::ExitStatus exitStatus)
+{
+    if (exitStatus == QProcess::NormalExit) {
+        if (exitCode == 0) {
+            qCDebug(LOG).noquote() << "Output:\n" << Utils::SynchronousProcess::normalizeNewlines(
+                                                        QString::fromLocal8Bit(m_processOutput));
+            emit finishedWithSuccess(m_fileToAnalyze);
+        } else {
+            emit finishedWithFailure(finishedWithBadExitCode(m_name, exitCode),
+                                     commandlineAndOutput());
+        }
+    } else { // == QProcess::CrashExit
+        emit finishedWithFailure(finishedDueToCrash(m_name), commandlineAndOutput());
+    }
+}
+
+void ClangToolRunner::onProcessError(QProcess::ProcessError error)
+{
+    if (error == QProcess::Crashed)
+        return; // handled by slot of finished()
+
+    emit finishedWithFailure(generalProcessError(m_name), commandlineAndOutput());
+}
+
+void ClangToolRunner::onProcessOutput()
+{
+    m_processOutput.append(m_process.readAll());
+}
+
+QString ClangToolRunner::commandlineAndOutput() const
 {
     return tr("Command line: %1\n"
               "Process Error: %2\n"
               "Output:\n%3")
         .arg(m_commandLine,
              QString::number(m_process.error()),
-                                 Utils::SynchronousProcess::normalizeNewlines(
+             Utils::SynchronousProcess::normalizeNewlines(QString::fromLocal8Bit(m_processOutput)));
-                                     QString::fromLocal8Bit(m_processOutput)));
 }
 
 } // namespace Internal

src/plugins/clangtools/clangtoolrunner.h

@@ -27,11 +27,11 @@
 
 #include "clangtoolslogfilereader.h"
 
-#include <memory>
-
 #include <QString>
 #include <QProcess>
 
+#include <memory>
+
 namespace Utils { class Environment; }
 
 namespace ClangTools {
@@ -39,8 +39,6 @@ namespace Internal {
 
 using ArgsCreator = std::function<QStringList(const QStringList &baseOptions)>;
 
-QString finishedWithBadExitCode(const QString &name, int exitCode); // exposed for tests
-
 class ClangToolRunner : public QObject
 {
     Q_OBJECT
@@ -49,51 +47,46 @@ public:
     ClangToolRunner(QObject *parent = nullptr) : QObject(parent) {}
     ~ClangToolRunner() override;
 
-    void init(const QString &clangLogFileDir, const Utils::Environment &environment);
+    void init(const QString &outputDirPath, const Utils::Environment &environment);
     void setName(const QString &name) { m_name = name; }
     void setExecutable(const QString &executable) { m_executable = executable; }
     void setArgsCreator(const ArgsCreator &argsCreator) { m_argsCreator = argsCreator; }
     void setOutputFileFormat(const OutputFileFormat &format) { m_outputFileFormat = format; }
 
-    // compilerOptions is expected to contain everything except:
-    //   (1) filePath, that is the file to analyze
-    //   (2) -o output-file
-    bool run(const QString &filePath, const QStringList &compilerOptions = QStringList());
-
     QString name() const { return m_name; }
-    OutputFileFormat outputFileFormat() const { return m_outputFileFormat; }
     QString executable() const { return m_executable; }
-    QString filePath() const { return m_filePath; }
+    OutputFileFormat outputFileFormat() const { return m_outputFileFormat; }
-    QString logFilePath() const { return m_logFile; }
+    QString fileToAnalyze() const { return m_fileToAnalyze; }
+    QString outputFilePath() const { return m_outputFilePath; }
+
+    // compilerOptions is expected to contain everything except:
+    //   (1) file to analyze
+    //   (2) -o output-file
+    bool run(const QString &fileToAnalyze, const QStringList &compilerOptions = QStringList());
 
 signals:
-    void started();
+    void finishedWithSuccess(const QString &fileToAnalyze);
-    void finishedWithSuccess(const QString &filePath);
     void finishedWithFailure(const QString &errorMessage, const QString &errorDetails);
 
 private:
-    virtual void onProcessOutput();
+    void onProcessOutput();
-    void onProcessStarted();
     void onProcessFinished(int exitCode, QProcess::ExitStatus exitStatus);
     void onProcessError(QProcess::ProcessError error);
 
-    QString createLogFile(const QString &filePath) const;
+    QString commandlineAndOutput() const;
-    QString processCommandlineAndOutput() const;
-
-protected:
-    QString m_logFile;
-    QProcess m_process;
-    QByteArray m_processOutput;
 
 private:
-    QString m_clangLogFileDir;
+    QString m_outputDirPath;
+    QProcess m_process;
+    QByteArray m_processOutput;
 
     QString m_name;
     QString m_executable;
     ArgsCreator m_argsCreator;
     OutputFileFormat m_outputFileFormat = OutputFileFormat::Yaml;
 
-    QString m_filePath;
+    QString m_fileToAnalyze;
+    QString m_outputFilePath;
     QString m_commandLine;
 };