Fix code generation for entries with min/max

Summary:
- When GenerateProperties and Mutators are activated, the generated code
  did not handle min/max properly

- In the case of a parameterized entry, generated code also did not
  handle min/max

BUG: 418146

Test Plan: - auto tests included

Reviewers: meven, crossi, ervin, bport, tcanabrava

Reviewed By: meven, ervin

Subscribers: kde-frameworks-devel

Tags: #frameworks

Differential Revision: https://phabricator.kde.org/D27497

4 files changed, 89 insertions, 37 deletions

diff --git a/src/kconfig_compiler/KConfigCommonStructs.h b/src/kconfig_compiler/KConfigCommonStructs.h
index 71bf666e..06c8b80f 100644
--- a/src/kconfig_compiler/KConfigCommonStructs.h
+++ b/src/kconfig_compiler/KConfigCommonStructs.h
@@ -155,6 +155,11 @@ QString varPath(const QString &n, const KConfigParameters &cfg);
 // like using d-> in case of dpointer
 QString itemVar(const CfgEntry *e, const KConfigParameters &cfg);
 
+// returns the name of the local inner item if there is one
+// (before wrapping with KConfigCompilerSignallingItem)
+// Otherwise return itemVar()
+QString innerItemVar(const CfgEntry *e, const KConfigParameters &cfg);
+
 QString itemPath(const CfgEntry *e, const KConfigParameters &cfg);
 
 QString filenameOnly(const QString &path);
@@ -170,9 +175,16 @@ QString translatedString(
 
 // TODO: Sanitize those functions.
 QString newItem(
-    const CfgEntry* entry, 
+    const CfgEntry *entry,
+    const QString &key,
+    const QString &defaultValue,
+    const KConfigParameters &cfg,
+    const QString &param = QString());
+
+QString newInnerItem(
+    const CfgEntry *entry,
     const QString &key,
-    const QString& defaultValue,
+    const QString &defaultValue,
     const KConfigParameters &cfg,
     const QString &param = QString());
 
diff --git a/src/kconfig_compiler/KConfigSourceGenerator.cpp b/src/kconfig_compiler/KConfigSourceGenerator.cpp
index 63f5b6b4..33e0ed69 100644
--- a/src/kconfig_compiler/KConfigSourceGenerator.cpp
+++ b/src/kconfig_compiler/KConfigSourceGenerator.cpp
@@ -313,14 +313,21 @@ void KConfigSourceGenerator::createEnums(const CfgEntry *entry)
 
 void KConfigSourceGenerator::createNormalEntry(const CfgEntry *entry, const QString &key)
 {
-   stream() << "  " << itemPath(entry, cfg()) << " = "
+    const QString innerItemVarStr = innerItemVar(entry, cfg());
+    if (!entry->signalList.isEmpty()) {
+        stream() << "  " << innerItemVarStr << " = "
+            << newInnerItem(entry, key, entry->defaultValue, cfg()) << '\n';
+    }
+
+    stream() << "  " << itemPath(entry, cfg()) << " = "
         << newItem(entry, key, entry->defaultValue, cfg()) << '\n';
 
     if (!entry->min.isEmpty()) {
-        stream() << "  " << itemPath(entry, cfg()) << "->setMinValue(" << entry->min << ");\n";
+        stream() << "  " << innerItemVarStr << "->setMinValue(" << entry->min << ");\n";
     }
+
     if (!entry->max.isEmpty()) {
-        stream() << "  " << itemPath(entry, cfg()) << "->setMaxValue(" << entry->max << ");\n";
+        stream() << "  " << innerItemVarStr << "->setMaxValue(" << entry->max << ");\n";
     }
 
     if (cfg().setUserTexts) {
@@ -343,14 +350,29 @@ void KConfigSourceGenerator::createNormalEntry(const CfgEntry *entry, const QStr
 void KConfigSourceGenerator::createIndexedEntry(const CfgEntry *entry, const QString &key)
 {
     for (int i = 0; i <= entry->paramMax; i++) {
-        QString itemVarStr(itemPath(entry, cfg()) + QStringLiteral("[%1]").arg(i));
+        const QString argBracket = QStringLiteral("[%1]").arg(i);
+        const QString innerItemVarStr = innerItemVar(entry, cfg()) + argBracket;
+
+        const QString defaultStr = !entry->paramDefaultValues[i].isEmpty()
+            ? entry->paramDefaultValues[i]
+            : !entry->defaultValue.isEmpty() ? paramString(entry->defaultValue, entry, i) : defaultValue(entry->type);
+
+        if (!entry->signalList.isEmpty()) {
+            stream() << "  " << innerItemVarStr << " = "
+                     << newInnerItem(entry, paramString(key, entry, i), defaultStr, cfg(), argBracket) << '\n';
+        }
+
+        const QString itemVarStr = itemPath(entry, cfg()) + argBracket;
 
-        QString defaultStr = !entry->paramDefaultValues[i].isEmpty() ? entry->paramDefaultValues[i]
-                           : !entry->defaultValue.isEmpty() ? paramString(entry->defaultValue, entry, i)
-                           : defaultValue(entry->type);
-        
         stream() << "  " << itemVarStr << " = "
-            << newItem(entry, paramString(key, entry, i), defaultStr, cfg(), QStringLiteral("[%1]").arg(i)) << '\n';
+                 << newItem(entry, paramString(key, entry, i), defaultStr, cfg(), argBracket) << '\n';
+
+        if (!entry->min.isEmpty()) {
+            stream() << "  " << innerItemVarStr << "->setMinValue(" << entry->min << ");\n";
+        }
+        if (!entry->max.isEmpty()) {
+            stream() << "  " << innerItemVarStr << "->setMaxValue(" << entry->max << ");\n";
+        }
 
         if (cfg().setUserTexts) {
             stream() << userTextsFunctions(entry, cfg(), itemVarStr, entry->paramName);
@@ -366,7 +388,7 @@ void KConfigSourceGenerator::createIndexedEntry(const CfgEntry *entry, const QSt
         QString paramName = entry->paramName;
 
         stream() << "  addItem( " << itemVarStr << ", QStringLiteral( \"";
-        stream() << paramName.replace(QStringLiteral("$(") + entry->param + QLatin1Char(')'), QLatin1String("%1")).arg( arg );
+        stream() << paramName.replace(QStringLiteral("$(") + entry->param + QLatin1Char(')'), QLatin1String("%1")).arg(arg);
         stream() << "\" ) );\n";
     }
 }
@@ -440,9 +462,7 @@ void KConfigSourceGenerator::doConstructor()
         }
         createEnums(entry);
 
-        if (!cfg().dpointer) {
-            stream() << itemDeclaration(entry, cfg());
-        }
+        stream() << itemDeclaration(entry, cfg());
 
         if (entry->param.isEmpty()) {
             createNormalEntry(entry, key);
diff --git a/src/kconfig_compiler/KConfigXmlParser.cpp b/src/kconfig_compiler/KConfigXmlParser.cpp
index b403dbd5..2f31beb9 100644
--- a/src/kconfig_compiler/KConfigXmlParser.cpp
+++ b/src/kconfig_compiler/KConfigXmlParser.cpp
@@ -362,7 +362,6 @@ CfgEntry *KConfigXmlParser::parseEntry(const QString &group, const QDomElement &
 
     readGroupElements(readEntry, element);
 
-    createChangedSignal(readEntry);
     validateNameAndKey(readEntry, element);
 
     if (readEntry.label.isEmpty()) {
@@ -431,6 +430,7 @@ CfgEntry *KConfigXmlParser::parseEntry(const QString &group, const QDomElement &
     }
     result->min = readEntry.min;
     result->max = readEntry.max;
+    createChangedSignal(*result);
 
     return result;
 }
diff --git a/src/kconfig_compiler/kconfig_compiler.cpp b/src/kconfig_compiler/kconfig_compiler.cpp
index 989e2609..4e76f375 100644
--- a/src/kconfig_compiler/kconfig_compiler.cpp
+++ b/src/kconfig_compiler/kconfig_compiler.cpp
@@ -394,21 +394,23 @@ QString itemType(const QString &type)
 
 QString itemDeclaration(const CfgEntry *e, const KConfigParameters &cfg)
 {
-    if (cfg.itemAccessors) {
-        return QString();
+    const QString type = cfg.inherits + "::Item" + itemType(e->type);
+
+    QString fCap = e->name;
+    fCap[0] = fCap[0].toUpper();
+    const QString argSuffix = (!e->param.isEmpty()) ? (QStringLiteral("[%1]").arg(e->paramMax + 1)) : QString();
+    QString result;
+
+    if (!cfg.itemAccessors && !cfg.dpointer) {
+        result += "  " + (!e->signalList.isEmpty() ? QStringLiteral("KConfigCompilerSignallingItem") : type) +
+            "  *item" + fCap + argSuffix + ";\n";
     }
 
-    QString type;
     if (!e->signalList.isEmpty()) {
-        type = QStringLiteral("KConfigCompilerSignallingItem");
-    } else {
-        type = cfg.inherits + "::Item" + itemType(e->type);
+        result += "  " + type + "  *" + innerItemVar(e, cfg) + argSuffix + ";\n";
     }
 
-    QString fCap = e->name;
-    fCap[0] = fCap[0].toUpper();
-    return "  " + type + "  *item" + fCap +
-            ( (!e->param.isEmpty())?(QStringLiteral("[%1]").arg(e->paramMax+1)) : QString()) + ";\n";
+    return result;
 }
 
 // returns the name of an item variable
@@ -432,6 +434,20 @@ QString itemVar(const CfgEntry *e, const KConfigParameters &cfg)
     return result;
 }
 
+// returns the name of the local inner item if there is one
+// (before wrapping with KConfigCompilerSignallingItem)
+// Otherwise return itemVar()
+QString innerItemVar(const CfgEntry *e, const KConfigParameters &cfg)
+{
+    if (e->signalList.isEmpty()) {
+        return itemVar(e, cfg);
+    } else {
+        QString result = "innerItem" + e->name;
+        result[9] = result[9].toUpper();
+        return result;
+    }
+}
+
 QString itemPath(const CfgEntry *e, const KConfigParameters &cfg)
 {
     QString result;
@@ -443,15 +459,9 @@ QString itemPath(const CfgEntry *e, const KConfigParameters &cfg)
     return result;
 }
 
-QString newItem(const CfgEntry* entry, const QString &key, const QString& defaultValue,
+QString newInnerItem(const CfgEntry *entry, const QString &key, const QString &defaultValue,
                 const KConfigParameters &cfg, const QString &param) {
-
-    QList<Signal> sigs = entry->signalList;
-    QString t;
-    if (!sigs.isEmpty()) {
-        t += QLatin1String("new KConfigCompilerSignallingItem(");
-    }
-    t += "new "+ cfg.inherits + "::Item" + itemType(entry->type) + "( currentGroup(), "
+    QString t = "new "+ cfg.inherits + "::Item" + itemType(entry->type) + "( currentGroup(), "
             + key + ", " + varPath( entry->name, cfg ) + param;
 
     if (entry->type == QLatin1String("Enum")) {
@@ -460,9 +470,18 @@ QString newItem(const CfgEntry* entry, const QString &key, const QString& defaul
     if (!defaultValue.isEmpty()) {
         t += QLatin1String(", ") + defaultValue;
     }
-    t += QLatin1String(" )");
+    t += QLatin1String(" );");
+
+    return t;
+}
+
+QString newItem(const CfgEntry *entry, const QString &key, const QString &defaultValue,
+                const KConfigParameters &cfg, const QString &param) {
 
+    QList<Signal> sigs = entry->signalList;
+    QString t;
     if (!sigs.isEmpty()) {
+        t += QLatin1String("new KConfigCompilerSignallingItem(") + innerItemVar(entry, cfg) + param;
         t += QLatin1String(", this, notifyFunction, ");
         //append the signal flags
         for (int i = 0; i < sigs.size(); ++i) {
@@ -470,9 +489,10 @@ QString newItem(const CfgEntry* entry, const QString &key, const QString& defaul
                 t += QLatin1String(" | ");
             t += signalEnumName(sigs[i].name);
         }
-        t += QLatin1String(")");
+        t += QLatin1String(");");
+    } else {
+        t += newInnerItem(entry, key, defaultValue, cfg, param);
     }
-    t += QLatin1String(";");
     return t;
 }