QmlDesigner: Convert UniqueName keywords to array of stringviews

Also change the class to namespace, rename "get" to "generate"
and add unit tests.

Change-Id: Ib52bf7e3e0110e33acb40ca6e3d9bfc61cefdca0
Reviewed-by: Qt CI Patch Build Bot <ci_patchbuild_bot@qt.io>
Reviewed-by: Miikka Heikkinen <miikka.heikkinen@qt.io>
Reviewed-by: Marco Bubke <marco.bubke@qt.io>

src/plugins/qmldesigner/components/assetslibrary/assetslibrarymodel.cpp

@@ -154,7 +154,7 @@ bool AssetsLibraryModel::renameFolder(const QString &folderPath, const QString &
 
 QString AssetsLibraryModel::addNewFolder(const QString &folderPath)
 {
-    Utils::FilePath uniqueDirPath = Utils::FilePath::fromString(UniqueName::getPath(folderPath));
+    Utils::FilePath uniqueDirPath = Utils::FilePath::fromString(UniqueName::generatePath(folderPath));
 
     auto res = uniqueDirPath.ensureWritableDir();
     if (!res.has_value()) {

src/plugins/qmldesigner/components/assetslibrary/assetslibrarywidget.cpp

@@ -178,7 +178,7 @@ QString AssetsLibraryWidget::getUniqueEffectPath(const QString &parentFolder, co
     QString effectsDir = ModelNodeOperations::getEffectsDefaultDirectory(parentFolder);
     QString effectPath = QLatin1String("%1/%2.qep").arg(effectsDir, effectName);
 
-    return UniqueName::getPath(effectPath);
+    return UniqueName::generatePath(effectPath);
 }
 
 bool AssetsLibraryWidget::createNewEffect(const QString &effectPath, bool openInEffectComposer)

src/plugins/qmldesigner/components/contentlibrary/contentlibraryusermodel.cpp

@@ -317,11 +317,11 @@ QPair<QString, QString> ContentLibraryUserModel::getUniqueLibItemNames(const QSt
     baseQml[0] = baseQml.at(0).toUpper();
     baseQml.prepend("My");
 
-    QString uniqueQml = UniqueName::get(baseQml, [&] (const QString &name) {
+    QString uniqueQml = UniqueName::generate(baseQml, [&] (const QString &name) {
         return itemQmls.contains(name);
     });
 
-    QString uniqueIcon = UniqueName::get(defaultName, [&] (const QString &name) {
+    QString uniqueIcon = UniqueName::generate(defaultName, [&] (const QString &name) {
         return itemIcons.contains(name);
     });
 

src/plugins/qmldesigner/designercore/model/model.cpp

@@ -1874,7 +1874,7 @@ QString Model::generateNewId(const QString &prefixName, const QString &fallbackP
     if (!isDuplicate.has_value()) // TODO: to be removed separately to not break build
         isDuplicate = std::bind(&Model::hasId, this, std::placeholders::_1);
 
-    return UniqueName::getId(prefixName, [&] (const QString &id) {
+    return UniqueName::generateId(prefixName, [&] (const QString &id) {
         // Properties of the root node are not allowed for ids, because they are available in the
         // complete context without qualification.
         return isDuplicate.value()(id) || d->rootNode()->property(id.toUtf8());

src/plugins/qmldesigner/designercore/uniquename.cpp

@@ -3,10 +3,58 @@
 
 #include "uniquename.h"
 
+#include <utils/span.h>
+
 #include <QFileInfo>
 #include <QRegularExpression>
 
-namespace QmlDesigner {
+namespace QmlDesigner::UniqueName {
+
+using namespace Qt::Literals;
+
+constexpr QLatin1StringView keywords[] {
+    "anchors"_L1,        "as"_L1,         "baseState"_L1,
+    "border"_L1,         "bottom"_L1,     "break"_L1,
+    "case"_L1,           "catch"_L1,      "clip"_L1,
+    "color"_L1,          "continue"_L1,   "data"_L1,
+    "debugger"_L1,       "default"_L1,    "delete"_L1,
+    "do"_L1,             "else"_L1,       "enabled"_L1,
+    "finally"_L1,        "flow"_L1,       "focus"_L1,
+    "font"_L1,           "for"_L1,        "function"_L1,
+    "height"_L1,         "if"_L1,         "import"_L1,
+    "in"_L1,             "instanceof"_L1, "item"_L1,
+    "layer"_L1,          "left"_L1,       "margin"_L1,
+    "new"_L1,            "opacity"_L1,    "padding"_L1,
+    "parent"_L1,         "print"_L1,      "rect"_L1,
+    "return"_L1,         "right"_L1,      "scale"_L1,
+    "shaderInfo"_L1,     "source"_L1,     "sprite"_L1,
+    "spriteSequence"_L1, "state"_L1,      "switch"_L1,
+    "text"_L1,           "this"_L1,       "throw"_L1,
+    "top"_L1,            "try"_L1,        "typeof"_L1,
+    "var"_L1,            "visible"_L1,    "void"_L1,
+    "while"_L1,          "with"_L1,       "x"_L1,
+    "y"_L1
+};
+
+namespace {
+
+QString toCamelCase(const QString &input)
+{
+    QString result = input.at(0).toLower();
+    bool capitalizeNext = false;
+
+    for (const QChar &c : Utils::span{input}.subspan(1)) {
+        bool isValidChar = c.isLetterOrNumber() || c == '_';
+        if (isValidChar)
+            result += capitalizeNext ? c.toUpper() : c;
+
+        capitalizeNext = !isValidChar;
+    }
+
+    return result;
+}
+
+} // namespace
 
 /**
  * @brief Generates a unique name based on the provided name.
@@ -19,7 +67,7 @@ namespace QmlDesigner {
  *        false if name is unique.
  * @return A unique name derived from the provided name.
  */
-QString UniqueName::get(const QString &name, std::function<bool(const QString &)> predicate)
+QString generate(const QString &name, std::function<bool(const QString &)> predicate)
 {
     if (!predicate(name))
         return name;
@@ -61,7 +109,7 @@ QString UniqueName::get(const QString &name, std::function<bool(const QString &)
  * @param path The original path to be made unique.
  * @return A unique path derived from the provided path.
  */
-QString UniqueName::getPath(const QString &path)
+QString generatePath(const QString &path)
 {
     // Remove the trailing slash if it exists (otherwise QFileInfo::path() returns empty)
     QString adjustedPath = path;
@@ -77,8 +125,8 @@ QString UniqueName::getPath(const QString &path)
     QString parentDir = fileInfo.path();
     QString pathTemplate = parentDir + "/%1" + suffix;
 
-    QString uniqueBaseName = UniqueName::get(baseName, [&] (const QString &currName) {
+    QString uniqueBaseName = UniqueName::generate(baseName, [&] (const QString &currName) {
-        return !QFileInfo::exists(pathTemplate.arg(currName));
+        return QFileInfo::exists(pathTemplate.arg(currName));
     });
 
     return pathTemplate.arg(uniqueBaseName);
@@ -97,25 +145,18 @@ QString UniqueName::getPath(const QString &path)
  * @param id The original id to be made unique.
  * @return A unique Id (when predicate() returns false)
  */
-QString UniqueName::getId(const QString &id, std::function<bool(const QString &)> predicate)
+QString generateId(const QString &id, std::function<bool(const QString &)> predicate)
 {
-    // remove non word (non A-Z, a-z, 0-9) characters
+    // remove non word (non A-Z, a-z, 0-9) or space characters
-    static const QRegularExpression nonWordCharsRgx("\\W");
+    QString newId = id.trimmed();
-    QString newId = id.simplified();
-    newId.remove(nonWordCharsRgx);
 
-    // convert to camel case
+    newId = toCamelCase(newId);
-    QStringList idParts = newId.split(" ");
-    idParts[0] = idParts[0].at(0).toLower() + idParts[0].mid(1);
-    for (int i = 1; i < idParts.size(); ++i)
-        idParts[i] = idParts[i].at(0).toUpper() + idParts[i].mid(1);
-    newId = idParts.join("");
 
     // prepend _ if starts with a digit or invalid id (such as reserved words)
-    if (newId.at(0).isDigit() || std::binary_search(keywords.begin(), keywords.end(), newId))
+    if (newId.at(0).isDigit() || std::binary_search(std::begin(keywords), std::end(keywords), newId))
         newId.prepend('_');
 
-    return UniqueName::get(newId, predicate);
+    return UniqueName::generate(newId, predicate);
 }
 
-} // namespace QmlDesigner
+} // namespace QmlDesigner::UniqueName

src/plugins/qmldesigner/designercore/uniquename.h

@@ -6,27 +6,11 @@
 #include <qmldesignercorelib_exports.h>
 
 #include <QString>
-#include <QStringList>
 
-namespace QmlDesigner {
+namespace QmlDesigner::UniqueName {
 
-class QMLDESIGNERCORE_EXPORT UniqueName
+QString generate(const QString &name, std::function<bool(const QString &)> predicate);
-{
+QString generatePath(const QString &path);
-public:
+QMLDESIGNERCORE_EXPORT QString generateId(const QString &id, std::function<bool(const QString &)> predicate);
-    static QString get(const QString &name, std::function<bool(const QString &)> predicate);
-    static QString getPath(const QString &path);
-    static QString getId(const QString &id, std::function<bool(const QString &)> predicate);
 
-private:
+} // namespace QmlDesigner::UniqueName
-    static inline const QStringList keywords {
-        "anchors", "as", "baseState", "border", "bottom", "break", "case", "catch", "clip", "color",
-        "continue", "data", "debugger", "default", "delete", "do", "else", "enabled", "finally",
-        "flow", "focus", "font", "for", "function", "height", "if", "import", "in", "instanceof",
-        "item", "layer", "left", "margin", "new", "opacity", "padding", "parent", "print", "rect",
-        "return", "right", "scale", "shaderInfo", "source", "sprite", "spriteSequence", "state",
-        "switch", "text", "this", "throw", "top", "try", "typeof", "var", "visible", "void",
-        "while", "with", "x", "y"
-    };
-};
-
-} // namespace QmlDesigner

tests/unit/tests/unittests/model/CMakeLists.txt

@@ -7,5 +7,5 @@ extend_qtc_test(unittest
     modelresourcemanagement-test.cpp
     modelutils-test.cpp
     nodelistproperty-test.cpp
-
+    uniquename-test.cpp
 )

tests/unit/tests/unittests/model/uniquename-test.cpp

@@ -0,0 +1,103 @@
+// Copyright (C) 2024 The Qt Company Ltd.
+// SPDX-License-Identifier: LicenseRef-Qt-Commercial OR GPL-3.0-only WITH Qt-GPL-exception-1.0
+
+#include <googletest.h>
+
+#include <uniquename.h>
+
+namespace {
+
+namespace UniqueName = QmlDesigner::UniqueName;
+
+TEST(UniqueName, generate_returns_same_input_if_predicate_returns_false)
+{
+    auto pred = [] (const QString &name) -> bool {
+        return false;
+    };
+    QString name = "abc";
+
+    QString uniqueName = UniqueName::generate(name, pred);
+
+    ASSERT_THAT(uniqueName, "abc");
+}
+
+TEST(UniqueName, generateId_returns_properly_formatted_id)
+{
+    auto pred = [] (const QString &id) -> bool {
+        return false;
+    };
+    QString id = "  A    bc   d _";
+
+    QString uniqueId = UniqueName::generateId(id, pred);
+
+    ASSERT_THAT(uniqueId, "aBcD_");
+}
+
+TEST(UniqueName, generateId_returns_properly_formatted_unique_id_when_id_exists)
+{
+    QStringList existingIds = {"aBcD009", "aBcD010"};
+    auto pred = [&] (const QString &id) -> bool {
+        return existingIds.contains(id);
+    };
+    QString id = "  A    bc   d 0 \t 0 9\n";
+
+    QString uniqueId = UniqueName::generateId(id, pred);
+
+    ASSERT_THAT(uniqueId, "aBcD011");
+}
+
+TEST(UniqueName, generateId_properly_handles_dot_separated_words)
+{
+    auto pred = [&] (const QString &id) -> bool {
+        return false;
+    };
+    QString id = "Foo.bar*foo";
+
+    QString uniqueId = UniqueName::generateId(id, pred);
+
+    ASSERT_THAT(uniqueId, "fooBarFoo");
+}
+
+TEST(UniqueName, generateId_prefixes_with_underscore_if_id_is_a_reserved_word)
+{
+    auto pred = [&] (const QString &id) -> bool {
+        return false;
+    };
+    QString id = "for";
+
+    QString uniqueId = UniqueName::generateId(id, pred);
+
+    ASSERT_THAT(uniqueId, "_for");
+}
+
+TEST(UniqueName, generateId_prefixes_with_underscore_if_id_is_a_number)
+{
+    auto pred = [&] (const QString &id) -> bool {
+        return false;
+    };
+    QString id = "436";
+
+    QString uniqueId = UniqueName::generateId(id, pred);
+
+    ASSERT_THAT(uniqueId, "_436");
+}
+
+TEST(UniqueName, generatePath_returns_same_path_when_path_doesnt_exist)
+{
+    QString path = "<<<non/existing/path>>>";
+
+    QString uniquePath = UniqueName::generatePath(path);
+
+    ASSERT_THAT(uniquePath, path);
+}
+
+TEST(UniqueName, generatePath_returns_unique_path_when_path_exists)
+{
+    QString path = UNITTEST_DIR;
+
+    QString uniquePath = UniqueName::generatePath(path);
+
+    ASSERT_THAT(uniquePath, QString(UNITTEST_DIR).append("1"));
+}
+
+} // namespace