MakeParser: Better parsing based on gnumake manual

The gnumake manual states that the make errors start either with
a make executable or a makefile, followed by a message.

This message starts with "warning: " for warnings and with "*** "
for fatal errors. Normal errors have no prefix.

This patch should make sure all these are recognized and also
simplifies the code a bit.

It also adds a new unit test for the lines starting with a Makefile.
That one addresses QTCREATORBUG-10576.

Task-number: QTCREATORBUG-10576
Change-Id: Ib3ef2ff5e324e773d7e78bb148bf09e9875b985e
Reviewed-by: Tobias Hunger <tobias.hunger@digia.com>

2 files changed, 65 insertions, 31 deletions

diff --git a/src/plugins/projectexplorer/gnumakeparser.cpp b/src/plugins/projectexplorer/gnumakeparser.cpp
index 9fc970ee35..115a8e296d 100644
--- a/src/plugins/projectexplorer/gnumakeparser.cpp
+++ b/src/plugins/projectexplorer/gnumakeparser.cpp
@@ -41,7 +41,8 @@ using namespace ProjectExplorer;
 
 namespace {
     // optional full path, make executable name, optional exe extension, optional number in square brackets, colon space
-    const char * const MAKE_PATTERN("^(([A-Za-z]:)?[/\\\\][^:]*[/\\\\])?(mingw(32|64)-|g)?make(.exe)?(\\[\\d+\\])?:\\s");
+    const char * const MAKEEXEC_PATTERN("^(.*[/\\\\])?(mingw(32|64)-|g)?make(.exe)?(\\[\\d+\\])?:\\s");
+    const char * const MAKEFILE_PATTERN("^((.*[/\\\\])?[Mm]akefile(\\.[a-zA-Z]+)?):(\\d+):\\s");
 }
 
 GnuMakeParser::GnuMakeParser() :
@@ -49,16 +50,16 @@ GnuMakeParser::GnuMakeParser() :
     m_fatalErrorCount(0)
 {
     setObjectName(QLatin1String("GnuMakeParser"));
-    m_makeDir.setPattern(QLatin1String(MAKE_PATTERN) +
+    m_makeDir.setPattern(QLatin1String(MAKEEXEC_PATTERN) +
                          QLatin1String("(\\w+) directory .(.+).$"));
     m_makeDir.setMinimal(true);
     QTC_CHECK(m_makeDir.isValid());
-    m_makeLine.setPattern(QLatin1String(MAKE_PATTERN) + QLatin1String("(\\*\\*\\*\\s)?(.*)$"));
+    m_makeLine.setPattern(QLatin1String(MAKEEXEC_PATTERN) + QLatin1String("(.*)$"));
     m_makeLine.setMinimal(true);
     QTC_CHECK(m_makeLine.isValid());
-    m_makefileError.setPattern(QLatin1String("^(.*):(\\d+):\\s\\*\\*\\*\\s(.*)$"));
-    m_makefileError.setMinimal(true);
-    QTC_CHECK(m_makefileError.isValid());
+    m_errorInMakefile.setPattern(QLatin1String(MAKEFILE_PATTERN) + QLatin1String("(.*)$"));
+    m_errorInMakefile.setMinimal(true);
+    QTC_CHECK(m_errorInMakefile.isValid());
 }
 
 void GnuMakeParser::setWorkingDirectory(const QString &workingDirectory)
@@ -77,48 +78,68 @@ void GnuMakeParser::stdOutput(const QString &line)
     const QString lne = rightTrimmed(line);
 
     if (m_makeDir.indexIn(lne) > -1) {
-        if (m_makeDir.cap(7) == QLatin1String("Leaving"))
-            removeDirectory(m_makeDir.cap(8));
+        if (m_makeDir.cap(6) == QLatin1String("Leaving"))
+            removeDirectory(m_makeDir.cap(7));
         else
-            addDirectory(m_makeDir.cap(8));
+            addDirectory(m_makeDir.cap(7));
         return;
     }
 
     IOutputParser::stdOutput(line);
 }
 
+class Result {
+public:
+    Result() : isFatal(false), type(Task::Error) { }
+
+    QString description;
+    bool isFatal;
+    Task::TaskType type;
+};
+
+static Result parseDescription(const QString &description)
+{
+    Result result;
+    if (description.startsWith(QLatin1String("warning: "), Qt::CaseInsensitive)) {
+        result.description = description.mid(9);
+        result.type = Task::Warning;
+        result.isFatal = false;
+    } else if (description.startsWith(QLatin1String("*** "))) {
+        result.description = description.mid(4);
+        result.type = Task::Error;
+        result.isFatal = true;
+    } else {
+        result.description = description;
+        result.type = Task::Error;
+        result.isFatal = false;
+    }
+    return result;
+}
+
 void GnuMakeParser::stdError(const QString &line)
 {
     const QString lne = rightTrimmed(line);
 
-    if (m_makefileError.indexIn(lne) > -1) {
-        ++m_fatalErrorCount;
+    if (m_errorInMakefile.indexIn(lne) > -1) {
+        Result res = parseDescription(m_errorInMakefile.cap(5));
+        if (res.isFatal)
+            ++m_fatalErrorCount;
         if (!m_suppressIssues) {
-            m_suppressIssues = true;
-            emit addTask(Task(Task::Error,
-                              m_makefileError.cap(3),
-                              Utils::FileName::fromUserInput(m_makefileError.cap(1)),
-                              m_makefileError.cap(2).toInt(),
-                              Core::Id(Constants::TASK_CATEGORY_BUILDSYSTEM)));
+            taskAdded(Task(res.type, res.description,
+                           Utils::FileName::fromUserInput(m_errorInMakefile.cap(1)) /* filename */,
+                           m_errorInMakefile.cap(4).toInt(), /* line */
+                           Core::Id(Constants::TASK_CATEGORY_BUILDSYSTEM)));
         }
         return;
     }
     if (m_makeLine.indexIn(lne) > -1) {
-        if (!m_makeLine.cap(7).isEmpty())
+        Result res = parseDescription(m_makeLine.cap(6));
+        if (res.isFatal)
             ++m_fatalErrorCount;
         if (!m_suppressIssues) {
-            m_suppressIssues = true;
-            QString description = m_makeLine.cap(8);
-            Task::TaskType type = Task::Error;
-            if (description.startsWith(QLatin1String("warning: "), Qt::CaseInsensitive)) {
-                description = description.mid(9);
-                type = Task::Warning;
-            }
-
-            emit addTask(Task(type, description,
-                              Utils::FileName() /* filename */,
-                              -1, /* line */
-                              Core::Id(Constants::TASK_CATEGORY_BUILDSYSTEM)));
+            taskAdded(Task(res.type, res.description,
+                           Utils::FileName() /* filename */, -1, /* line */
+                           Core::Id(Constants::TASK_CATEGORY_BUILDSYSTEM)));
         }
         return;
     }
@@ -368,6 +389,18 @@ void ProjectExplorerPlugin::testGnuMakeParserParsing_data()
                         Core::Id(Constants::TASK_CATEGORY_BUILDSYSTEM)))
             << QString()
             << QStringList();
+    QTest::newRow("warning in Makefile")
+            << QStringList()
+            << QString::fromLatin1("Makefile:794: warning: overriding commands for target `xxxx.app/Contents/Info.plist'")
+            << OutputParserTester::STDERR
+            << QString() << QString()
+            << (QList<Task>()
+                << Task(Task::Warning,
+                        QString::fromLatin1("overriding commands for target `xxxx.app/Contents/Info.plist'"),
+                        Utils::FileName::fromString(QLatin1String("Makefile")), 794,
+                        Core::Id(Constants::TASK_CATEGORY_BUILDSYSTEM)))
+            << QString()
+            << QStringList();
 }
 
 void ProjectExplorerPlugin::testGnuMakeParserParsing()
diff --git a/src/plugins/projectexplorer/gnumakeparser.h b/src/plugins/projectexplorer/gnumakeparser.h
index 7719a8118c..99ddc78152 100644
--- a/src/plugins/projectexplorer/gnumakeparser.h
+++ b/src/plugins/projectexplorer/gnumakeparser.h
@@ -62,7 +62,8 @@ private:
 
     QRegExp m_makeDir;
     QRegExp m_makeLine;
-    QRegExp m_makefileError;
+    QRegExp m_threeStarError;
+    QRegExp m_errorInMakefile;
 
     QStringList m_directories;