fix filename macro modifiers for $** and $?

The filename macros $** and $? return lists of values.
All of those values must be modified, if a filename modifier is given.

getFileNameMacroValue was renamed to getFileNameMacroValues and returns
a QStringList.

Task-number: QTCREATORBUG-6932

Change-Id: I44108ff45db29e8691bd30315ba8f9a6d5c49f36
Reviewed-by: Joerg Bornemann <joerg.bornemann@digia.com>

3 files changed, 70 insertions, 53 deletions

diff --git a/src/jomlib/makefile.cpp b/src/jomlib/makefile.cpp
index 15f65a8..6136444 100644
--- a/src/jomlib/makefile.cpp
+++ b/src/jomlib/makefile.cpp
@@ -158,6 +158,23 @@ inline void quoteStringIfNeeded(QString& str)
     }
 }
 
+static QString joinMacroValues(const QStringList &macroValues, bool mustQuote)
+{
+    QString result;
+    for (int i = 0; i < macroValues.count(); ++i) {
+        if (i > 0)
+            result += QLatin1Char(' ');
+        if (mustQuote) {
+            QString value = macroValues.at(i);
+            quoteStringIfNeeded(value);
+            result += value;
+        } else  {
+            result += macroValues.at(i);
+        }
+    }
+    return result;
+}
+
 /**
  * Expands the filename macros in a given string.
  *
@@ -195,30 +212,44 @@ void DescriptionBlock::expandFileNameMacros(QString& str, int depIdx, bool depen
             int substitutionIdx = -1;
             bool substitutionStateKnown = false;
             bool fileNameReturned;
-            QString macroValue = getFileNameMacroValue(str.midRef(idx+1), replacementLength,
-                                                       depIdx, dependentsForbidden, fileNameReturned);
-            if (!macroValue.isNull()) {
-                int k;
+            QStringList macroValues = getFileNameMacroValues(str.midRef(idx+1), replacementLength,
+                                                             depIdx, dependentsForbidden,
+                                                             fileNameReturned);
+            if (macroValues.isEmpty()) {
+                str.remove(idx - 1, replacementLength + 3);
+            } else {
                 ch = str.at(idx + replacementLength + 1).toLatin1();
                 switch (ch)
                 {
                 case 'D':
-                    k = macroValue.lastIndexOf(QLatin1Char('\\'));
-                    if (k == -1)
-                        macroValue = QLatin1String(".");
-                    else
-                        macroValue = macroValue.left(k);
+                    for (int i = 0; i < macroValues.count(); ++i) {
+                        QString &macroValue = macroValues[i];
+                        int k = macroValue.lastIndexOf(QLatin1Char('\\'));
+                        if (k == -1)
+                            macroValue = QLatin1String(".");
+                        else
+                            macroValue = macroValue.left(k);
+                    }
                     break;
                 case 'B':
-                    macroValue = QFileInfo(macroValue).baseName();
+                    for (int i = 0; i < macroValues.count(); ++i) {
+                        QString &macroValue = macroValues[i];
+                        macroValue = QFileInfo(macroValue).baseName();
+                    }
                     break;
                 case 'F':
-                    macroValue = QFileInfo(macroValue).fileName();
+                    for (int i = 0; i < macroValues.count(); ++i) {
+                        QString &macroValue = macroValues[i];
+                        macroValue = QFileInfo(macroValue).fileName();
+                    }
                     break;
                 case 'R':
-                    k = macroValue.lastIndexOf(QLatin1Char('.'));
-                    if (k > -1)
-                        macroValue = macroValue.left(k);
+                    for (int i = 0; i < macroValues.count(); ++i) {
+                        QString &macroValue = macroValues[i];
+                        int k = macroValue.lastIndexOf(QLatin1Char('.'));
+                        if (k > -1)
+                            macroValue = macroValue.left(k);
+                    }
                     break;
                 case ':':
                     substitutionStateKnown = true;
@@ -241,35 +272,35 @@ void DescriptionBlock::expandFileNameMacros(QString& str, int depIdx, bool depen
                     int macroInvokationEnd;
                     const MacroTable::Substitution substitution =
                             MacroTable::parseSubstitutionStatement(str, substitutionIdx, macroInvokationEnd);
-                    MacroTable::applySubstitution(substitution, macroValue);
+                    for (int i = 0; i < macroValues.count(); ++i) {
+                        MacroTable::applySubstitution(substitution, macroValues[i]);
+                    }
                     replacementLength = macroInvokationEnd - idx - 2;  // because we're later adding 4
                 }
 
-                if (fileNameReturned)
-                    quoteStringIfNeeded(macroValue);
+                const QString macroValue = joinMacroValues(macroValues, fileNameReturned);
                 str.replace(idx - 1, replacementLength + 4, macroValue);
-            } else {
-                str.remove(idx - 1, replacementLength + 3);
             }
         } else {
             bool fileNameReturned;
-            QString macroValue = getFileNameMacroValue(str.midRef(idx), replacementLength, depIdx,
-                                                       dependentsForbidden, fileNameReturned);
-            if (!macroValue.isNull()) {
-                if (fileNameReturned)
-                    quoteStringIfNeeded(macroValue);
-                str.replace(idx - 1, replacementLength + 1, macroValue);
-            } else {
+            QStringList macroValues = getFileNameMacroValues(str.midRef(idx), replacementLength,
+                                                             depIdx, dependentsForbidden,
+                                                             fileNameReturned);
+            if (macroValues.isEmpty()) {
                 str.remove(idx - 1, replacementLength + 1);
+            } else {
+                const QString macroValue = joinMacroValues(macroValues, fileNameReturned);
+                str.replace(idx - 1, replacementLength + 1, macroValue);
             }
         }
     }
 }
 
-QString DescriptionBlock::getFileNameMacroValue(const QStringRef& str, int& replacementLength,
-                                                int depIdx, bool dependentsForbidden, bool& returnsFileName)
+QStringList DescriptionBlock::getFileNameMacroValues(const QStringRef& str, int& replacementLength,
+                                                     int depIdx, bool dependentsForbidden,
+                                                     bool& returnsFileName)
 {
-    QString result;
+    QStringList results;
     QStringList dependentCandidates;
     returnsFileName = false;
     if (!dependentsForbidden) {
@@ -282,7 +313,7 @@ QString DescriptionBlock::getFileNameMacroValue(const QStringRef& str, int& repl
     switch (str.at(0).toLatin1()) {
         case '@':
             replacementLength = 1;
-            result = targetName();
+            results += targetName();
             returnsFileName = true;
             break;
         case '*':
@@ -292,14 +323,15 @@ QString DescriptionBlock::getFileNameMacroValue(const QStringRef& str, int& repl
                         throw Exception(QLatin1String("Macro $** not allowed here."));
                     }
                     replacementLength = 2;
-                    result = dependentCandidates.join(QLatin1String(" "));
+                    results = dependentCandidates;
                 } else {
                     returnsFileName = true;
                     replacementLength = 1;
-                    result = targetName();
-                    int idx = result.lastIndexOf(QLatin1Char('.'));
+                    QString tgt = targetName();
+                    int idx = tgt.lastIndexOf(QLatin1Char('.'));
                     if (idx > -1)
-                        result.resize(idx);
+                        tgt.resize(idx);
+                    results += tgt;
                 }
             }
             break;
@@ -309,8 +341,6 @@ QString DescriptionBlock::getFileNameMacroValue(const QStringRef& str, int& repl
                     throw Exception(QLatin1String("Macro $? not allowed here."));
                 }
                 replacementLength = 1;
-                result.clear();
-                bool firstAppend = true;
                 const FileTime currentTimeStamp = FileTime::currentTime();
                 FileTime targetTimeStamp = FastFileInfo(targetName()).lastModified();
                 if (!targetTimeStamp.isValid())
@@ -322,19 +352,14 @@ QString DescriptionBlock::getFileNameMacroValue(const QStringRef& str, int& repl
                         dependentTimeStamp = currentTimeStamp;
 
                     if (targetTimeStamp <= dependentTimeStamp) {
-                        if (firstAppend)
-                            firstAppend = false;
-                        else
-                            result.append(QLatin1Char(' '));
-
-                        result.append(dependentName);
+                        results += dependentName;
                     }
                 }
             }
             break;
     }
 
-    return result;
+    return results;
 }
 
 InferenceRule::InferenceRule()
diff --git a/src/jomlib/makefile.h b/src/jomlib/makefile.h
index 4a24be0..d7b7fc4 100644
--- a/src/jomlib/makefile.h
+++ b/src/jomlib/makefile.h
@@ -104,8 +104,8 @@ public:
 private:
     void expandFileNameMacros(Command& command, int depIdx);
     void expandFileNameMacros(QString& str, int depIdx, bool dependentsForbidden);
-    QString getFileNameMacroValue(const QStringRef& str, int& replacementLength, int depIdx,
-                                  bool dependentsForbidden, bool& returnsFileName);
+    QStringList getFileNameMacroValues(const QStringRef& str, int& replacementLength, int depIdx,
+                                       bool dependentsForbidden, bool& returnsFileName);
 
 private:
     QString m_targetName;
diff --git a/tests/tests.cpp b/tests/tests.cpp
index 220e825..d1414bd 100644
--- a/tests/tests.cpp
+++ b/tests/tests.cpp
@@ -708,34 +708,26 @@ void ParserTest::fileNameMacros()
     target->expandFileNameMacros();
     QVERIFY(!target->m_commands.isEmpty());
     command = target->m_commands.takeFirst();
-    QEXPECT_FAIL("", "Needs to be fixed. See QTCREATORBUG-6932.", Continue);
     QCOMPARE(command.m_commandLine, QLatin1String("echo $(**D) subdir subdir\\subsubdir . . . ."));
     QVERIFY(!target->m_commands.isEmpty());
     command = target->m_commands.takeFirst();
-    QEXPECT_FAIL("", "Needs to be fixed. See QTCREATORBUG-6932.", Continue);
     QCOMPARE(command.m_commandLine, QLatin1String("echo $(**B) Timmy Jimmy Kenny Eric Kyle Stan"));
     QVERIFY(!target->m_commands.isEmpty());
     command = target->m_commands.takeFirst();
-    QEXPECT_FAIL("", "Needs to be fixed. See QTCREATORBUG-6932.", Continue);
     QCOMPARE(command.m_commandLine, QLatin1String("echo $(**F) Timmy.txt Jimmy.txt Kenny.txt Eric.txt Kyle.txt Stan.txt"));
     QVERIFY(!target->m_commands.isEmpty());
     command = target->m_commands.takeFirst();
-    QEXPECT_FAIL("", "Needs to be fixed. See QTCREATORBUG-6932.", Continue);
     QCOMPARE(command.m_commandLine, QLatin1String("echo $(**R) subdir\\Timmy subdir\\subsubdir\\Jimmy Kenny Eric Kyle Stan"));
     command = target->m_commands.takeFirst();
-    QEXPECT_FAIL("", "Needs to be fixed. See QTCREATORBUG-6932.", Continue);
     QCOMPARE(command.m_commandLine, QLatin1String("echo $(?D) subdir subdir\\subsubdir . . . ."));
     QVERIFY(!target->m_commands.isEmpty());
     command = target->m_commands.takeFirst();
-    QEXPECT_FAIL("", "Needs to be fixed. See QTCREATORBUG-6932.", Continue);
     QCOMPARE(command.m_commandLine, QLatin1String("echo $(?B) Timmy Jimmy Kenny Eric Kyle Stan"));
     QVERIFY(!target->m_commands.isEmpty());
     command = target->m_commands.takeFirst();
-    QEXPECT_FAIL("", "Needs to be fixed. See QTCREATORBUG-6932.", Continue);
     QCOMPARE(command.m_commandLine, QLatin1String("echo $(?F) Timmy.txt Jimmy.txt Kenny.txt Eric.txt Kyle.txt Stan.txt"));
     QVERIFY(!target->m_commands.isEmpty());
     command = target->m_commands.takeFirst();
-    QEXPECT_FAIL("", "Needs to be fixed. See QTCREATORBUG-6932.", Continue);
     QCOMPARE(command.m_commandLine, QLatin1String("echo $(?R) subdir\\Timmy subdir\\subsubdir\\Jimmy Kenny Eric Kyle Stan"));
 
     system("del generated.txt gen1.txt gen2.txt gen3.txt > NUL 2>&1");