diff --git a/Core/GDCore/IDE/Events/ExpressionValidator.h b/Core/GDCore/IDE/Events/ExpressionValidator.h index b1170e3000..5be469d964 100644 --- a/Core/GDCore/IDE/Events/ExpressionValidator.h +++ b/Core/GDCore/IDE/Events/ExpressionValidator.h @@ -86,8 +86,10 @@ class GD_CORE_API ExpressionValidator : public ExpressionParser2NodeWorker { void OnVisitOperatorNode(OperatorNode& node) override { ReportAnyError(node); + // The "required" type ("parentType") will be used when visiting the first operand. + // Note that it may be refined thanks to this first operand (see later). node.leftHandSide->Visit(*this); - const Type leftType = childType; + const Type leftType = childType; // Store the type of the first operand. if (leftType == Type::Number) { if (node.op == ' ') { @@ -120,15 +122,19 @@ class GD_CORE_API ExpressionValidator : public ExpressionParser2NodeWorker { node.rightHandSide->location); } - parentType = leftType; + // The "required" type ("parentType") of the second operator is decided by: + // - the parent type. Unless it can (`number|string`) or should (`unknown`) be refined, then: + // - the first operand. + parentType = ShouldTypeBeRefined(parentType) ? leftType : parentType; node.rightHandSide->Visit(*this); const Type rightType = childType; - // The type is decided by the first operand, unless it can (`number|string`) - // or should (`unknown`) be refined, in which case we go for the right - // operand (which got visited knowing the type of the first operand, so it's + // The type of the overall operator ("childType") is decided by: + // - the parent type. Unless it can (`number|string`) or should (`unknown`) be refined, then: + // - the first operand. Unless it can (`number|string`) or should (`unknown`) be refined, then: + // - the right operand (which got visited knowing the type of the first operand, so it's // equal or strictly more precise than the left operand). - childType = (leftType == Type::Unknown || leftType == Type::NumberOrString) ? leftType : rightType; + childType = ShouldTypeBeRefined(parentType) ? (ShouldTypeBeRefined(leftType) ? leftType : rightType) : parentType; } void OnVisitUnaryOperatorNode(UnaryOperatorNode& node) override { ReportAnyError(node); @@ -395,6 +401,10 @@ class GD_CORE_API ExpressionValidator : public ExpressionParser2NodeWorker { } } + static bool ShouldTypeBeRefined(Type type) { + return (type == Type::Unknown || type == Type::NumberOrString); + } + static Type StringToType(const gd::String &type); static const gd::String &TypeToString(Type type); static const gd::String unknownTypeString; diff --git a/Core/tests/ExpressionCodeGenerator.cpp b/Core/tests/ExpressionCodeGenerator.cpp index 83ee5aeb58..2ca11ca2fe 100644 --- a/Core/tests/ExpressionCodeGenerator.cpp +++ b/Core/tests/ExpressionCodeGenerator.cpp @@ -30,6 +30,21 @@ TEST_CASE("ExpressionCodeGenerator", "[common][events]") { layout1.GetVariables().InsertNew("MySceneBooleanVariable").SetBool(true); layout1.GetVariables().InsertNew("MySceneStructureVariable").GetChild("MyChild"); layout1.GetVariables().InsertNew("MySceneStructureVariable2").GetChild("MyChild"); + layout1.GetVariables().InsertNew("MySceneEmptyArrayVariable").CastTo(gd::Variable::Type::Array); + { + auto& variable = layout1.GetVariables().InsertNew("MySceneNumberArrayVariable"); + variable.CastTo(gd::Variable::Type::Array); + variable.PushNew().SetValue(1); + variable.PushNew().SetValue(2); + variable.PushNew().SetValue(3); + } + { + auto& variable = layout1.GetVariables().InsertNew("MySceneStringArrayVariable"); + variable.CastTo(gd::Variable::Type::Array); + variable.PushNew().SetString("1"); + variable.PushNew().SetString("2"); + variable.PushNew().SetString("3"); + } auto &mySpriteObject = layout1.InsertNewObject(project, "MyExtension::Sprite", "MySpriteObject", 0); mySpriteObject.GetVariables().InsertNew("MyNumberVariable").SetValue(123); @@ -1295,6 +1310,221 @@ TEST_CASE("ExpressionCodeGenerator", "[common][events]") { REQUIRE(output == "getVariableForObject(MySpriteObject, MyOtherSpriteObject).getChild(\"Child\").getChild(\"Grandchild\")"); } } + SECTION("Type conversions (valid operators with variables having different types than the expression)") { + SECTION("Expression/parent type is 'string'") { + { + auto node = + parser.ParseExpression("\"You have \" + MySceneVariable + \" points\""); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "\"You have \" + getLayoutVariable(MySceneVariable).getAsString() + \" points\""); + } + { + auto node = + parser.ParseExpression("MySceneVariable + MySceneStringVariable"); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "getLayoutVariable(MySceneVariable).getAsString() + getLayoutVariable(MySceneStringVariable).getAsString()"); + } + } + SECTION("Expression/parent type is 'string' (with an unknown variable)") { + { + auto node = + parser.ParseExpression("\"You have \" + MySceneStructureVariable.MyChild.CantKnownTheTypeSoStayGeneric + \" points\""); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "\"You have \" + getLayoutVariable(MySceneStructureVariable).getChild(\"MyChild\").getChild(\"CantKnownTheTypeSoStayGeneric\").getAsString() + \" points\""); + } + } + SECTION("Expression/parent type is 'string' (2 number variables)") { + { + auto node = + parser.ParseExpression("MySceneVariable + MySceneVariable2 + \"world\""); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "getLayoutVariable(MySceneVariable).getAsString() + getLayoutVariable(MySceneVariable2).getAsString() + \"world\""); + } + { + auto node = + parser.ParseExpression("MySceneVariable + MySceneVariable2 + MySceneStringVariable"); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "getLayoutVariable(MySceneVariable).getAsString() + getLayoutVariable(MySceneVariable2).getAsString() + getLayoutVariable(MySceneStringVariable).getAsString()"); + } + } + SECTION("Expression/parent type is 'string' (array variable)") { + { + auto node = + parser.ParseExpression("\"hello\" + MySceneNumberArrayVariable[2] + \"world\""); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "\"hello\" + getLayoutVariable(MySceneNumberArrayVariable).getChild(2).getAsString() + \"world\""); + } + { + auto node = + parser.ParseExpression("\"hello\" + MySceneEmptyArrayVariable[2] + \"world\""); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "\"hello\" + getLayoutVariable(MySceneEmptyArrayVariable).getChild(2).getAsString() + \"world\""); + } + } + + SECTION("Expression/parent type is 'number'") { + { + auto node = + parser.ParseExpression("123 + MySceneVariable + 456"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "123 + getLayoutVariable(MySceneVariable).getAsNumber() + 456"); + } + { + auto node = + parser.ParseExpression("MySceneStringVariable + MySceneVariable"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "getLayoutVariable(MySceneStringVariable).getAsNumber() + getLayoutVariable(MySceneVariable).getAsNumber()"); + } + } + SECTION("Expression/parent type is 'string' (with an unknown variable)") { + { + auto node = + parser.ParseExpression("123 + MySceneStructureVariable.MyChild.CantKnownTheTypeSoStayGeneric + 456"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "123 + getLayoutVariable(MySceneStructureVariable).getChild(\"MyChild\").getChild(\"CantKnownTheTypeSoStayGeneric\").getAsNumber() + 456"); + } + } + SECTION("Expression/parent type is 'number' (2 string variables)") { + { + auto node = + parser.ParseExpression("MySceneStringVariable + MySceneStringVariable + 456"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "getLayoutVariable(MySceneStringVariable).getAsNumber() + getLayoutVariable(MySceneStringVariable).getAsNumber() + 456"); + } + { + auto node = + parser.ParseExpression("MySceneStringVariable + MySceneStringVariable + MySceneVariable"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "getLayoutVariable(MySceneStringVariable).getAsNumber() + getLayoutVariable(MySceneStringVariable).getAsNumber() + getLayoutVariable(MySceneVariable).getAsNumber()"); + } + } + SECTION("Expression/parent type is 'number' (array variable)") { + { + auto node = + parser.ParseExpression("123 + MySceneNumberArrayVariable[2] + 456"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "123 + getLayoutVariable(MySceneNumberArrayVariable).getChild(2).getAsNumber() + 456"); + } + { + auto node = + parser.ParseExpression("123 + MySceneEmptyArrayVariable[2] + 456"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "123 + getLayoutVariable(MySceneEmptyArrayVariable).getChild(2).getAsNumber() + 456"); + } + } + + + SECTION("Multiple type conversions in sub expressions or same expression") { + { + auto node = + parser.ParseExpression("\"hello\" + MySceneNumberArrayVariable[2 + MySceneStringVariable] + \"world\" + MySceneVariable + \"world 2\""); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "\"hello\" + getLayoutVariable(MySceneNumberArrayVariable).getChild(2 + getLayoutVariable(MySceneStringVariable).getAsNumber()).getAsString() + \"world\" + getLayoutVariable(MySceneVariable).getAsString() + \"world 2\""); + } + { + auto node = + parser.ParseExpression("\"hello\" + MySceneNumberArrayVariable[\"foo\" + MySceneVariable + \"bar\"] + \"world\" + MySceneVariable + \"world 2\""); + gd::ExpressionCodeGenerator expressionCodeGenerator("string", + "", + codeGenerator, + context); + + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "\"hello\" + getLayoutVariable(MySceneNumberArrayVariable).getChild(\"foo\" + getLayoutVariable(MySceneVariable).getAsString() + \"bar\").getAsString() + \"world\" + getLayoutVariable(MySceneVariable).getAsString() + \"world 2\""); + } + } + } SECTION("Mixed test (1)") { { auto node = parser.ParseExpression("-+-MyExtension::MouseX(,)"); diff --git a/Core/tests/ExpressionParser2.cpp b/Core/tests/ExpressionParser2.cpp index 956fca064a..4476ebaedc 100644 --- a/Core/tests/ExpressionParser2.cpp +++ b/Core/tests/ExpressionParser2.cpp @@ -2954,6 +2954,238 @@ TEST_CASE("ExpressionParser2", "[common][events]") { } } + SECTION("Valid operators with variables having different types than the expression") { + SECTION("Expression/parent type is 'string'") { + // A trivial test (everything is a string). + { + auto node = parser.ParseExpression("\"You have \" + MySceneStringVariable + \" points\""); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "string"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A string concatenated with a number variable (will have to be casted to a string in code generation) + { + auto node = parser.ParseExpression("\"You have \" + MySceneNumberVariable"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A string concatenated with a number variable (will have to be casted to a string in code generation) + // and then with a string again. + { + auto node = parser.ParseExpression("\"You have \" + MySceneNumberVariable + \" points\""); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A string concatenated with an unknown variable (will have to be casted to a string in code generation) + // and then with a string again. + { + auto node = parser.ParseExpression("\"You have \" + MySceneStructureVariable.MyChild.CantKnownTheTypeSoStayGeneric + \" points\""); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + } + SECTION("Expression/parent type is 'number'") { + // A trivial test (everything is a string). + { + auto node = parser.ParseExpression("123 + MySceneNumberVariable + 456"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A number concatenated with a string variable (will have to be casted to a number in code generation) + { + auto node = parser.ParseExpression("123 + MySceneStringVariable"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A number concatenated with a string variable (will have to be casted to a number in code generation) + // and then with a number again. + { + auto node = parser.ParseExpression("123 + MySceneStringVariable + 456"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A number concatenated with an unknown variable (will have to be casted to a number in code generation) + // and then with a number again. + { + auto node = parser.ParseExpression("123 + MySceneStructureVariable.MyChild.CantKnownTheTypeSoStayGeneric + 456"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + } + SECTION("Expression/parent type is 'number|string'") { + SECTION("Expression/parent inferred type is 'string'") { + // A trivial test (everything is a string). + { + auto node = parser.ParseExpression("\"You have \" + MySceneStringVariable + \" points\""); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A string concatenated with a number variable (will have to be casted to a string in code generation) + { + auto node = parser.ParseExpression("\"You have \" + MySceneNumberVariable"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A string concatenated with a number variable (will have to be casted to a string in code generation) + // and then with a string again. + { + auto node = parser.ParseExpression("\"You have \" + MySceneNumberVariable + \" points\""); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A string concatenated with an unknown variable (will have to be casted to a string in code generation) + // and then with a string again. + { + auto node = parser.ParseExpression("\"You have \" + MySceneStructureVariable.MyChild.CantKnownTheTypeSoStayGeneric + \" points\""); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + } + SECTION("Expression/parent inferred type is 'number'") { + // A trivial test (everything is a string). + { + auto node = parser.ParseExpression("123 + MySceneNumberVariable + 456"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A number concatenated with a string variable (will have to be casted to a number in code generation) + { + auto node = parser.ParseExpression("123 + MySceneStringVariable"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A number concatenated with a string variable (will have to be casted to a number in code generation) + // and then with a number again. + { + auto node = parser.ParseExpression("123 + MySceneStringVariable + 456"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A number concatenated with an unknown variable (will have to be casted to a number in code generation) + // and then with a number again. + { + auto node = parser.ParseExpression("123 + MySceneStructureVariable.MyChild.CantKnownTheTypeSoStayGeneric + 456"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 0); + } + } + } + } + + SECTION("Invalid operators with variables having different types than the expression") { + // Try to do a sum between numbers in a string expression + { + auto node = parser.ParseExpression("\"You have \" + MySceneNumberVariable + 2 + \" points\""); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 1); + REQUIRE(validator.GetFatalErrors()[0]->GetMessage() == "You entered a number, but a text was expected (in quotes)."); + REQUIRE(validator.GetFatalErrors()[0]->GetStartPosition() == 38); + REQUIRE(validator.GetFatalErrors()[0]->GetEndPosition() == 39); + } + // Try to do a sum between numbers in a number|string expression (that is inferred as a string with the first operand) + { + auto node = parser.ParseExpression("\"You have \" + MySceneNumberVariable + 2 + \" points\""); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 1); + REQUIRE(validator.GetFatalErrors()[0]->GetMessage() == "You entered a number, but a text was expected (in quotes)."); + REQUIRE(validator.GetFatalErrors()[0]->GetStartPosition() == 38); + REQUIRE(validator.GetFatalErrors()[0]->GetEndPosition() == 39); + } + // Try to do a string concatenation in a number expression + { + auto node = parser.ParseExpression("123 + MySceneStringVariable + \"hello\" + 456"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 1); + REQUIRE(validator.GetFatalErrors()[0]->GetMessage() == "You entered a text, but a number was expected."); + REQUIRE(validator.GetFatalErrors()[0]->GetStartPosition() == 30); + REQUIRE(validator.GetFatalErrors()[0]->GetEndPosition() == 37); + } + // Try to do a string concatenation in a number|string expression (that is inferred as a number with the first operand) + { + auto node = parser.ParseExpression("123 + MySceneStringVariable + \"hello\" + 456"); + REQUIRE(node != nullptr); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + + REQUIRE(validator.GetFatalErrors().size() == 1); + REQUIRE(validator.GetFatalErrors()[0]->GetMessage() == "You entered a text, but a number was expected."); + REQUIRE(validator.GetFatalErrors()[0]->GetStartPosition() == 30); + REQUIRE(validator.GetFatalErrors()[0]->GetEndPosition() == 37); + } + } + SECTION("Valid function call with object variable") { { // Note that in this test we need to use an expression with "objectvar",