Utils: Replace Environment::find iterator use

Task-number: QTCREATORBUG-28357
Change-Id: I2723ffd6b7842f88009701eccea9aacac8cbf516
Reviewed-by: Christian Kandeler <christian.kandeler@qt.io>

src/libs/utils/commandline.cpp

@@ -362,12 +362,12 @@ static QStringList splitArgsUnix(const QString &args, bool abortOnMeta,
                         if (var == pwdName && pwd && !pwd->isEmpty()) {
                             cret += *pwd;
                         } else {
-                            Environment::const_iterator vit = env->constFind(var);
+                            const Environment::FindResult res = env->find(var);
-                            if (vit == env->constEnd()) {
+                            if (!res) {
                                 if (abortOnMeta)
                                     goto metaerr; // Assume this is a shell builtin
                             } else {
-                                cret += env->expandedValueForKey(env->key(vit));
+                                cret += env->expandedValueForKey(res->key);
                             }
                         }
                         if (!braced)
@@ -412,12 +412,12 @@ static QStringList splitArgsUnix(const QString &args, bool abortOnMeta,
                 if (var == pwdName && pwd && !pwd->isEmpty()) {
                     val = *pwd;
                 } else {
-                    Environment::const_iterator vit = env->constFind(var);
+                    const Environment::FindResult res = env->find(var);
-                    if (vit == env->constEnd()) {
+                    if (!res) {
                         if (abortOnMeta)
                             goto metaerr; // Assume this is a shell builtin
                     } else {
-                        val = env->expandedValueForKey(env->key(vit));
+                        val = env->expandedValueForKey(res->key);
                     }
                 }
                 for (int i = 0; i < val.length(); i++) {

src/libs/utils/environment.cpp

@@ -24,6 +24,14 @@ NameValueItems Environment::diff(const Environment &other, bool checkAppendPrepe
     return m_dict.diff(other.m_dict, checkAppendPrepend);
 }
 
+Environment::FindResult Environment::find(const QString &name) const
+{
+    const auto it = m_dict.constFind(name);
+    if (it == m_dict.constEnd())
+        return {};
+     return Entry{it.key().name, it.value().first, it.value().second};
+}
+
 void Environment::forEachEntry(const std::function<void(const QString &, const QString &, bool)> &callBack) const
 {
     for (auto it = m_dict.m_values.constBegin(); it != m_dict.m_values.constEnd(); ++it)
@@ -356,28 +364,30 @@ QString Environment::expandVariables(const QString &input) const
                 }
             } else if (state == BRACEDVARIABLE) {
                 if (c == '}') {
-                    const_iterator it = constFind(result.mid(vStart, i - 1 - vStart));
+                    const QString key = result.mid(vStart, i - 1 - vStart);
-                    if (it != constEnd()) {
+                    const Environment::FindResult res = find(key);
-                        result.replace(vStart - 2, i - vStart + 2, it->first);
+                    if (res) {
-                        i = vStart - 2 + it->first.length();
+                        result.replace(vStart - 2, i - vStart + 2, res->value);
+                        i = vStart - 2 + res->value.length();
                     }
                     state = BASE;
                 }
             } else if (state == VARIABLE) {
                 if (!c.isLetterOrNumber() && c != '_') {
-                    const_iterator it = constFind(result.mid(vStart, i - vStart - 1));
+                    const QString key = result.mid(vStart, i - vStart - 1);
-                    if (it != constEnd()) {
+                    const Environment::FindResult res = find(key);
-                        result.replace(vStart - 1, i - vStart, it->first);
+                    if (res) {
-                        i = vStart - 1 + it->first.length();
+                        result.replace(vStart - 1, i - vStart, res->value);
+                        i = vStart - 1 + res->value.length();
                     }
                     state = BASE;
                 }
             }
         }
         if (state == VARIABLE) {
-            const_iterator it = constFind(result.mid(vStart));
+            const Environment::FindResult res = find(result.mid(vStart));
-            if (it != constEnd())
+            if (res)
-                result.replace(vStart - 1, result.length() - vStart + 1, it->first);
+                result.replace(vStart - 1, result.length() - vStart + 1, res->value);
         }
     }
     return result;

src/libs/utils/environment.h

@@ -91,7 +91,10 @@ public:
 
     const_iterator constBegin() const { return m_dict.constBegin(); } // FIXME: avoid
     const_iterator constEnd() const { return m_dict.constEnd(); } // FIXME: avoid
-    const_iterator constFind(const QString &name) const { return m_dict.constFind(name); } // FIXME: avoid
+
+    struct Entry { QString key; QString value; bool enabled; };
+    using FindResult = std::optional<Entry>;
+    FindResult find(const QString &name) const; // Note res->key may differ in case from name.
 
     void forEachEntry(const std::function<void (const QString &, const QString &, bool)> &callBack) const;
 

tests/auto/environment/tst_environment.cpp

@@ -270,8 +270,9 @@ void tst_Environment::incrementalChanges()
     newEnv.modify(changes);
     QVERIFY(!newEnv.hasKey("VAR1"));
     QCOMPARE(newEnv.value("VAR2"), QString());
-    QCOMPARE(newEnv.constFind("VAR2")->first, "VALUE2");
+    Environment::FindResult res = newEnv.find("VAR2");
-    QVERIFY(!newEnv.isEnabled(newEnv.constFind("VAR2")));
+    QCOMPARE(res->value, "VALUE2");
+    QVERIFY(!res->enabled);
     const QChar sep = HostOsInfo::pathListSeparator();
     QCOMPARE(newEnv.value("PATH"),
              QString("/tmp").append(sep).append("/usr/bin").append(sep).append("/usr/local/bin"));
@@ -317,13 +318,12 @@ void tst_Environment::find()
 
     Environment env(QStringList({"Foo=bar", "Hi=HO"}), osType);
 
-    auto end = env.constEnd();
+    Environment::FindResult res = env.find(variable);
-    auto it = env.constFind(variable);
 
-    QCOMPARE((end != it), contains);
+    QCOMPARE(bool(res), contains);
 
     if (contains)
-        QCOMPARE(env.value(it), QString("bar"));
+        QCOMPARE(res->value, QString("bar"));
 
 }