From 7caf81117d409238d25ceb300920f6d6ba7e2060 Mon Sep 17 00:00:00 2001 From: Ali Kianian Date: Thu, 20 Jun 2024 17:33:12 +0300 Subject: [PATCH] QmlDesigner: Unify invalid ids Fixes: QDS-13056 Change-Id: I8d99e0e9eba148fe4d31c3c9a2d28f46d91b377a Reviewed-by: Miikka Heikkinen Reviewed-by: Marco Bubke --- src/libs/qmljs/qmljscheck.cpp | 34 +++++--- .../designercore/model/modelnode.cpp | 43 ++-------- .../designercore/model/modelutils.cpp | 65 +++++++++++++++ .../designercore/model/modelutils.h | 6 ++ .../qmldesigner/designercore/uniquename.cpp | 28 +------ .../tests/unittests/model/modelutils-test.cpp | 81 +++++++++++++++++++ 6 files changed, 186 insertions(+), 71 deletions(-) diff --git a/src/libs/qmljs/qmljscheck.cpp b/src/libs/qmljs/qmljscheck.cpp index 7ceff0e78de..99d14269a25 100644 --- a/src/libs/qmljs/qmljscheck.cpp +++ b/src/libs/qmljs/qmljscheck.cpp @@ -546,19 +546,33 @@ private: uint _block = 0; }; -class IdsThatShouldNotBeUsedInDesigner : public QStringList +class IdsThatShouldNotBeUsedInDesigner : public QStringList { public: IdsThatShouldNotBeUsedInDesigner() - : QStringList({"top", "bottom", "left", "right", "width", "height", - "x", "y", "opacity", "parent", "item", "flow", - "color", "margin", "padding", "print", "border", "font", - "text", "source", "state", "visible", "focus", "data", - "clip", "layer", "scale", "enabled", "anchors", - "texture", "shaderInfo", "sprite", "spriteSequence", "baseState" - "vector", "string", "url", "var", "point", "date", "size", "list", - "enumeration"}) + : QStringList({ + "action", "alias", "anchors", "as", "baseState", "bool", + "border", "bottom", "break", "case", "catch", "clip", + "color", "continue", "data", "date", "debugger", "default", + "delete", "do", "double", "else", "enabled", "enumeration", + "finally", "flow", "focus", "font", "for", "function", + "height", "id", "if", "import", "in", "instanceof", + "int", "item", "layer", "left", "list", "margin", + "matrix4x4", "new", "opacity", "padding", "parent", "point", + "print", "quaternion", "real", "rect", "return", "right", + "scale", "shaderInfo", "size", "source", "sprite", "spriteSequence", + "state", "string", "switch", "text", "texture", "this", + "throw", "time", "top", "try", "typeof", "url", + "var", "variant", "vector", "vector2d", "vector3d", "vector4d", + "visible", "void", "while", "width", "with", "x", + "y", "z", + }) {} + + bool containsId(const QString &id) const + { + return std::binary_search(constBegin(), constEnd(), id); + } }; class VisualAspectsPropertyBlackList : public QStringList @@ -682,7 +696,7 @@ QList Check::defaultDisabledMessagesForNonQuickUi() bool Check::incompatibleDesignerQmlId(const QString &id) { - return idsThatShouldNotBeUsedInDesigner->contains(id); + return idsThatShouldNotBeUsedInDesigner->containsId(id); } Check::Check(Document::Ptr doc, const ContextPtr &context, Utils::QtcSettings *qtcSettings) diff --git a/src/plugins/qmldesigner/designercore/model/modelnode.cpp b/src/plugins/qmldesigner/designercore/model/modelnode.cpp index c4fc203fd2b..f483f5ca6b3 100644 --- a/src/plugins/qmldesigner/designercore/model/modelnode.cpp +++ b/src/plugins/qmldesigner/designercore/model/modelnode.cpp @@ -93,42 +93,10 @@ QString ModelNode::validId() const return id(); } -namespace { -bool isQmlKeyWord(QStringView id) -{ - static constexpr auto keywords = Utils::to_array( - {u"as", u"break", u"case", u"catch", u"continue", u"debugger", - u"default", u"delete", u"do", u"else", u"finally", u"for", - u"function", u"if", u"import", u"in", u"instanceof", u"new", - u"print", u"return", u"switch", u"this", u"throw", u"try", - u"typeof", u"var", u"void", u"while", u"with"}); - - return std::binary_search(keywords.begin(), keywords.end(), ModelUtils::toStdStringView(id)); -} - -bool isIdToAvoid(QStringView id) -{ - static constexpr auto token = Utils::to_array( - {u"anchors", u"baseState", u"border", u"bottom", u"clip", u"color", - u"data", u"enabled", u"flow", u"focus", u"font", u"height", - u"item", u"layer", u"left", u"margin", u"opacity", u"padding", - u"parent", u"rect", u"right", u"scale", u"shaderInfo", u"source", - u"sprite", u"spriteSequence", u"state", u"text", u"texture", u"top", - u"visible", u"width", u"x", u"y"}); - - return std::binary_search(token.begin(), token.end(), ModelUtils::toStdStringView(id)); -} - -bool idContainsWrongLetter(const QString &id) -{ - static QRegularExpression idExpr(QStringLiteral("^[a-z_][a-zA-Z0-9_]*$")); - return !id.contains(idExpr); -} - -} // namespace bool ModelNode::isValidId(const QString &id) { - return id.isEmpty() || (!idContainsWrongLetter(id) && !isQmlKeyWord(id) && !isIdToAvoid(id)); + using namespace ModelUtils; + return isValidQmlIdentifier(id) && !isBannedQmlId(id); } QString ModelNode::getIdValidityErrorMessage(const QString &id) @@ -145,10 +113,13 @@ QString ModelNode::getIdValidityErrorMessage(const QString &id) if (id.contains(' ')) return QObject::tr("ID cannot include whitespace (%1).").arg(id); - if (isQmlKeyWord(id)) + if (ModelUtils::isQmlKeyword(id)) return QObject::tr("%1 is a reserved QML keyword.").arg(id); - if (isIdToAvoid(id)) + if (ModelUtils::isQmlBuiltinType(id)) + return QObject::tr("%1 is a reserved Qml type.").arg(id); + + if (ModelUtils::isDiscouragedQmlId(id)) return QObject::tr("%1 is a reserved property keyword.").arg(id); return QObject::tr("ID includes invalid characters (%1).").arg(id); diff --git a/src/plugins/qmldesigner/designercore/model/modelutils.cpp b/src/plugins/qmldesigner/designercore/model/modelutils.cpp index 6c3e1ea50f5..d034278831e 100644 --- a/src/plugins/qmldesigner/designercore/model/modelutils.cpp +++ b/src/plugins/qmldesigner/designercore/model/modelutils.cpp @@ -16,12 +16,50 @@ #include +#include + namespace QmlDesigner::ModelUtils { namespace { enum class ImportError { EmptyImportName, HasAlreadyImport, NoModule }; +constexpr std::u16string_view qmlKeywords[]{ + u"alias", u"as", u"break", u"case", u"catch", u"continue", u"debugger", u"default", + u"delete", u"do", u"else", u"finally", u"for", u"function", u"if", u"import", + u"in", u"instanceof", u"new", u"print", u"return", u"switch", u"this", u"throw", + u"try", u"typeof", u"var", u"void", u"while", u"with", +}; + +constexpr std::u16string_view qmlDiscouragedIds[]{ + u"action", u"anchors", u"baseState", u"border", u"bottom", u"clip", + u"data", u"enabled", u"flow", u"focus", u"font", u"height", + u"id", u"item", u"layer", u"left", u"margin", u"opacity", + u"padding", u"parent", u"right", u"scale", u"shaderInfo", u"source", + u"sprite", u"spriteSequence", u"state", u"text", u"texture", u"time", + u"top", u"visible", u"width", u"x", u"y", u"z", +}; + +constexpr std::u16string_view qmlBuiltinTypes[]{ + u"bool", u"color", u"date", u"double", u"enumeration", u"font", + u"int", u"list", u"matrix4x4", u"point", u"quaternion", u"real", + u"rect", u"size", u"string", u"url", u"var", u"variant", + u"vector", u"vector2d", u"vector3d", u"vector4d", +}; + +constexpr auto createBannedQmlIds() +{ + std::array ids; + + auto idsEnd = std::copy(std::begin(qmlKeywords), std::end(qmlKeywords), ids.begin()); + idsEnd = std::copy(std::begin(qmlDiscouragedIds), std::end(qmlDiscouragedIds), idsEnd); + std::copy(std::begin(qmlBuiltinTypes), std::end(qmlBuiltinTypes), idsEnd); + + std::sort(ids.begin(), ids.end()); + + return ids; +} + ::Utils::expected findImport(const QString &importName, const std::function &predicate, const Imports &imports, @@ -275,4 +313,31 @@ ModelNode lowestCommonAncestor(Utils::span nodes) return accumulatedNode; } +bool isQmlKeyword(QStringView id) +{ + return std::binary_search(std::begin(qmlKeywords), std::end(qmlKeywords), id); +} + +bool isDiscouragedQmlId(QStringView id) +{ + return std::binary_search(std::begin(qmlDiscouragedIds), std::end(qmlDiscouragedIds), id); +} + +bool isQmlBuiltinType(QStringView id) +{ + return std::binary_search(std::begin(qmlBuiltinTypes), std::end(qmlBuiltinTypes), id); +} + +bool isBannedQmlId(QStringView id) +{ + static constexpr auto invalidIds = createBannedQmlIds(); + return std::binary_search(invalidIds.begin(), invalidIds.end(), id); +} + +bool isValidQmlIdentifier(QStringView id) +{ + static QRegularExpression idExpr(R"(^[a-z_]\w*$)"); + return id.contains(idExpr); +} + } // namespace QmlDesigner::ModelUtils diff --git a/src/plugins/qmldesigner/designercore/model/modelutils.h b/src/plugins/qmldesigner/designercore/model/modelutils.h index 2cef2e1fb26..fc575411553 100644 --- a/src/plugins/qmldesigner/designercore/model/modelutils.h +++ b/src/plugins/qmldesigner/designercore/model/modelutils.h @@ -46,6 +46,12 @@ QMLDESIGNERCORE_EXPORT QList allModelNodesWithId(AbstractView *view); QMLDESIGNERCORE_EXPORT bool isThisOrAncestorLocked(const ModelNode &node); QMLDESIGNERCORE_EXPORT ModelNode lowestCommonAncestor(Utils::span nodes); +QMLDESIGNERCORE_EXPORT bool isQmlKeyword(QStringView id); +QMLDESIGNERCORE_EXPORT bool isDiscouragedQmlId(QStringView id); +QMLDESIGNERCORE_EXPORT bool isQmlBuiltinType(QStringView id); +QMLDESIGNERCORE_EXPORT bool isBannedQmlId(QStringView id); +QMLDESIGNERCORE_EXPORT bool isValidQmlIdentifier(QStringView id); + constexpr std::u16string_view toStdStringView(QStringView view) { return {view.utf16(), Utils::usize(view)}; diff --git a/src/plugins/qmldesigner/designercore/uniquename.cpp b/src/plugins/qmldesigner/designercore/uniquename.cpp index 0948e9155e2..b0bf81207ba 100644 --- a/src/plugins/qmldesigner/designercore/uniquename.cpp +++ b/src/plugins/qmldesigner/designercore/uniquename.cpp @@ -3,6 +3,8 @@ #include "uniquename.h" +#include + #include #include @@ -12,30 +14,6 @@ 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) @@ -156,7 +134,7 @@ QString generateId(const QString &id, std::function predi newId = toCamelCase(newId); // prepend _ if starts with a digit or invalid id (such as reserved words) - if (newId.at(0).isDigit() || std::binary_search(std::begin(keywords), std::end(keywords), newId)) + if (newId.at(0).isDigit() || ModelUtils::isBannedQmlId(newId)) newId.prepend('_'); if (!predicate) diff --git a/tests/unit/tests/unittests/model/modelutils-test.cpp b/tests/unit/tests/unittests/model/modelutils-test.cpp index 240ef698b87..ff800587f91 100644 --- a/tests/unit/tests/unittests/model/modelutils-test.cpp +++ b/tests/unit/tests/unittests/model/modelutils-test.cpp @@ -159,4 +159,85 @@ TEST_F(ModelUtils, lowest_common_ancestor_for_uncle_and_nephew_should_return_the ASSERT_THAT(commonAncestor, grandFatherNode); } +TEST_F(ModelUtils, isValidQmlIdentifier_fails_on_empty_values) +{ + QStringView emptyValue = u""; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(emptyValue); + + ASSERT_FALSE(result); +} + +TEST_F(ModelUtils, isValidQmlIdentifier_fails_on_upper_case_first_letter) +{ + QStringView id = u"Lmn"; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(id); + + ASSERT_FALSE(result); +} + +TEST_F(ModelUtils, isValidQmlIdentifier_fails_on_digital_first_letter) +{ + QStringView id = u"6mn"; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(id); + + ASSERT_FALSE(result); +} + +TEST_F(ModelUtils, isValidQmlIdentifier_fails_on_unicodes) +{ + QStringView id = u"sähköverkko"; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(id); + + ASSERT_FALSE(result); +} + +TEST_F(ModelUtils, isValidQmlIdentifier_passes_on_lower_case_first_letter) +{ + QStringView id = u"mn"; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(id); + + ASSERT_TRUE(result); +} + +TEST_F(ModelUtils, isValidQmlIdentifier_passes_on_underscored_first_letter) +{ + QStringView id = u"_m"; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(id); + + ASSERT_TRUE(result); +} + +TEST_F(ModelUtils, isValidQmlIdentifier_passes_on_digital_non_first_letter) +{ + QStringView id = u"_6"; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(id); + + ASSERT_TRUE(result); +} + +TEST_F(ModelUtils, isValidQmlIdentifier_passes_on_upper_case_non_first_letter) +{ + QStringView id = u"mN"; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(id); + + ASSERT_TRUE(result); +} + +TEST_F(ModelUtils, isValidQmlIdentifier_passes_on_underscore_only) +{ + QStringView id = u"_"; + + bool result = QmlDesigner::ModelUtils::isValidQmlIdentifier(id); + + ASSERT_TRUE(result); +} + } // namespace