From 6dc9ed5d2cb7cbbabfb03f94e28ded83be0d4763 Mon Sep 17 00:00:00 2001 From: D8H Date: Wed, 20 May 2026 16:35:51 +0200 Subject: [PATCH] Fix minus sign color for negative numbers (#8621) Don't show in changelog --- .../Events/Parsers/ExpressionParser2.cpp | 10 +- .../GDCore/Events/Parsers/ExpressionParser2.h | 10 +- Core/GDCore/Events/Parsers/GrammarTerminals.h | 4 + .../Extensions/Metadata/ValueTypeMetadata.cpp | 1 + .../Extensions/Metadata/ValueTypeMetadata.h | 1 + .../Events/ExpressionSyntaxColoringHelper.h | 7 +- Core/tests/ExpressionCodeGenerator.cpp | 65 ++- Core/tests/ExpressionNodeLocationFinder.cpp | 4 +- Core/tests/ExpressionParser2.cpp | 430 +++++++++++++++++- Core/tests/ExpressionParser2NodePrinter.cpp | 23 + 10 files changed, 528 insertions(+), 27 deletions(-) diff --git a/Core/GDCore/Events/Parsers/ExpressionParser2.cpp b/Core/GDCore/Events/Parsers/ExpressionParser2.cpp index e943398781..b9b0a18bb3 100644 --- a/Core/GDCore/Events/Parsers/ExpressionParser2.cpp +++ b/Core/GDCore/Events/Parsers/ExpressionParser2.cpp @@ -87,8 +87,11 @@ std::unique_ptr ExpressionParser2::ReadText() { return text; } -std::unique_ptr ExpressionParser2::ReadNumber() { - size_t numberStartPosition = GetCurrentPosition(); +std::unique_ptr +ExpressionParser2::ReadNumber(size_t minusSignPosition) { + bool isNegative = minusSignPosition != -1; + size_t numberStartPosition = + isNegative ? minusSignPosition : GetCurrentPosition(); SkipAllWhitespaces(); gd::String parsedNumber; @@ -131,7 +134,8 @@ std::unique_ptr ExpressionParser2::ReadNumber() { // Note that parsedNumber can finish by a dot (1., 2., 0.). This is // valid in most languages so we allow this. - auto number = gd::make_unique(parsedNumber); + auto number = gd::make_unique(isNegative ? "-" + parsedNumber + : parsedNumber); number->location = ExpressionParserLocation(numberStartPosition, GetCurrentPosition()); if (!numberHasStarted || !digitFound) { diff --git a/Core/GDCore/Events/Parsers/ExpressionParser2.h b/Core/GDCore/Events/Parsers/ExpressionParser2.h index 9bd9295b3d..50b5b79547 100644 --- a/Core/GDCore/Events/Parsers/ExpressionParser2.h +++ b/Core/GDCore/Events/Parsers/ExpressionParser2.h @@ -181,8 +181,14 @@ class GD_CORE_API ExpressionParser2 { return factor; } else if (CheckIfChar(IsUnaryOperator)) { auto unaryOperatorCharacter = GetCurrentChar(); - SkipChar(); + bool isNumberSign = CheckIfChar(IsNumberSign); + SkipChar(); + if (isNumberSign && CheckIfChar(IsNumberFirstChar)) { + std::unique_ptr numberNode = + ReadNumber(expressionStartPosition); + return numberNode; + } auto operatorOperand = Factor(); auto unaryOperator = gd::make_unique( @@ -665,7 +671,7 @@ class GD_CORE_API ExpressionParser2 { std::unique_ptr ReadText(); - std::unique_ptr ReadNumber(); + std::unique_ptr ReadNumber(size_t minusSignPosition = -1); std::unique_ptr ReadUntilWhitespace() { size_t startPosition = GetCurrentPosition(); diff --git a/Core/GDCore/Events/Parsers/GrammarTerminals.h b/Core/GDCore/Events/Parsers/GrammarTerminals.h index 99528af606..315cbd137a 100644 --- a/Core/GDCore/Events/Parsers/GrammarTerminals.h +++ b/Core/GDCore/Events/Parsers/GrammarTerminals.h @@ -59,6 +59,10 @@ inline bool IsUnaryOperator(gd::String::value_type character) { return character == '+' || character == '-'; } +inline bool IsNumberSign(gd::String::value_type character) { + return character == '-'; +} + inline bool IsTermOperator(gd::String::value_type character) { return character == '/' || character == '*'; } diff --git a/Core/GDCore/Extensions/Metadata/ValueTypeMetadata.cpp b/Core/GDCore/Extensions/Metadata/ValueTypeMetadata.cpp index 9e3f7f1ed0..fe89e374a3 100644 --- a/Core/GDCore/Extensions/Metadata/ValueTypeMetadata.cpp +++ b/Core/GDCore/Extensions/Metadata/ValueTypeMetadata.cpp @@ -125,6 +125,7 @@ const gd::String ValueTypeMetadata::objectAnimationNameValueType = "objectAnimat const gd::String ValueTypeMetadata::objectSkinNameValueType = "objectSkinName"; const gd::String ValueTypeMetadata::keyboardKeyValueType = "keyboardKey"; const gd::String ValueTypeMetadata::layerValueType = "layer"; +const gd::String ValueTypeMetadata::layerValueType2 = "layer"; const gd::String &ValueTypeMetadata::ConvertPropertyTypeToValueType( const gd::String &propertyType) { diff --git a/Core/GDCore/Extensions/Metadata/ValueTypeMetadata.h b/Core/GDCore/Extensions/Metadata/ValueTypeMetadata.h index 1d78270288..e5c3deb650 100644 --- a/Core/GDCore/Extensions/Metadata/ValueTypeMetadata.h +++ b/Core/GDCore/Extensions/Metadata/ValueTypeMetadata.h @@ -355,6 +355,7 @@ class GD_CORE_API ValueTypeMetadata { static const gd::String objectSkinNameValueType; static const gd::String keyboardKeyValueType; static const gd::String layerValueType; + static const gd::String layerValueType2; }; } // namespace gd diff --git a/Core/GDCore/IDE/Events/ExpressionSyntaxColoringHelper.h b/Core/GDCore/IDE/Events/ExpressionSyntaxColoringHelper.h index b1dd7140be..9b5be8b06a 100644 --- a/Core/GDCore/IDE/Events/ExpressionSyntaxColoringHelper.h +++ b/Core/GDCore/IDE/Events/ExpressionSyntaxColoringHelper.h @@ -301,10 +301,7 @@ private: const gd::ProjectScopedContainers &projectScopedContainers_, const gd::String &rootType_) : platform(platform_), projectScopedContainers(projectScopedContainers_), - rootType(rootType_), - rootObjectName("") // Always empty, might be changed if variable fields - // in the editor are changed to use coloration. - {}; + rootType(rootType_){}; void AddColoration( gd::ExpressionColorationDescription::ColorationKind colorationKind, @@ -321,12 +318,10 @@ private: } std::vector colorations; - size_t searchedPosition; const gd::Platform &platform; const gd::ProjectScopedContainers &projectScopedContainers; const gd::String rootType; - const gd::String rootObjectName; }; } // namespace gd diff --git a/Core/tests/ExpressionCodeGenerator.cpp b/Core/tests/ExpressionCodeGenerator.cpp index b01c2af06e..8cdbc51836 100644 --- a/Core/tests/ExpressionCodeGenerator.cpp +++ b/Core/tests/ExpressionCodeGenerator.cpp @@ -166,7 +166,7 @@ TEST_CASE("ExpressionCodeGenerator", "[common][events]") { } } - SECTION("Valid unary operator generation") { + SECTION("Valid negative number generation") { { auto node = parser.ParseExpression("-12.45"); gd::ExpressionCodeGenerator expressionCodeGenerator("number", @@ -175,10 +175,71 @@ TEST_CASE("ExpressionCodeGenerator", "[common][events]") { context); REQUIRE(node); node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "-12.45"); + } + { + auto node = parser.ParseExpression("12.5 + -2. / (.3)"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "12.5 + -2. / (0.3)"); + } + // `--3` is a unary minus on the negative literal -3 (no extra unary wrap + // around the inner -3 because it's a NumberNode, not a UnaryOperatorNode). + { + auto node = parser.ParseExpression("--3"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "-(-3)"); + } + // `1 - -2` becomes JS-friendly `1 - -2` (the negative literal is emitted + // as-is on the right side of the binary subtract). + { + auto node = parser.ParseExpression("1 - -2"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == "1 - -2"); + } + // A negative literal as a function argument is emitted without an extra + // unary wrapper. + { + auto node = parser.ParseExpression( + "MyExtension::GetNumberWith3Params(-1, \"hello\", -2.5)"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + REQUIRE(node); + node->Visit(expressionCodeGenerator); + REQUIRE(expressionCodeGenerator.GetOutput() == + "getNumberWith3Params(-1, \"hello\", -2.5)"); + } + } + + SECTION("Valid unary operator generation") { + { + auto node = parser.ParseExpression("- 12.45"); + gd::ExpressionCodeGenerator expressionCodeGenerator("number", + "", + codeGenerator, + context); + REQUIRE(node); + node->Visit(expressionCodeGenerator); REQUIRE(expressionCodeGenerator.GetOutput() == "-(12.45)"); } { - auto node = parser.ParseExpression("12.5 + -2. / (.3)"); + auto node = parser.ParseExpression("12.5 + - 2. / (.3)"); gd::ExpressionCodeGenerator expressionCodeGenerator("number", "", codeGenerator, diff --git a/Core/tests/ExpressionNodeLocationFinder.cpp b/Core/tests/ExpressionNodeLocationFinder.cpp index fbe7934c5e..460feaf22e 100644 --- a/Core/tests/ExpressionNodeLocationFinder.cpp +++ b/Core/tests/ExpressionNodeLocationFinder.cpp @@ -141,7 +141,7 @@ TEST_CASE("ExpressionNodeLocationFinder", "[common][events]") { SECTION("Valid unary operators") { SECTION("Test 1") { - REQUIRE(CheckNodeAtLocationIs( + REQUIRE(CheckNodeAtLocationIs( parser, "-123", 0) == true); REQUIRE(CheckNodeAtLocationIs( parser, "+123", 0) == true); @@ -158,7 +158,7 @@ TEST_CASE("ExpressionNodeLocationFinder", "[common][events]") { parser, "-+-123", 0) == true); REQUIRE(CheckNodeAtLocationIs( parser, "-+-123", 1) == true); - REQUIRE(CheckNodeAtLocationIs( + REQUIRE(CheckNodeAtLocationIs( parser, "-+-123", 2) == true); REQUIRE(CheckNodeAtLocationIs( parser, "-+-123", 3) == true); diff --git a/Core/tests/ExpressionParser2.cpp b/Core/tests/ExpressionParser2.cpp index cd8b9d511d..beb8cd10db 100644 --- a/Core/tests/ExpressionParser2.cpp +++ b/Core/tests/ExpressionParser2.cpp @@ -578,6 +578,25 @@ TEST_CASE("ExpressionParser2", "[common][events]") { node->Visit(validator); REQUIRE(validator.GetFatalErrors().size() == 0); } + { + auto node = parser.ParseExpression("-123-456"); + REQUIRE(node != nullptr); + auto &operatorNode = dynamic_cast(*node); + auto type = gd::ExpressionTypeFinder::GetType( + platform, projectScopedContainers, "number|string", operatorNode); + REQUIRE(operatorNode.op == '-'); + REQUIRE(type == "number"); + auto &leftNumberNode = + dynamic_cast(*operatorNode.leftHandSide); + REQUIRE(leftNumberNode.number == "-123"); + auto &rightNumberNode = + dynamic_cast(*operatorNode.rightHandSide); + REQUIRE(rightNumberNode.number == "456"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } { auto node = parser.ParseExpression("\"abc\" + \"def\""); REQUIRE(node != nullptr); @@ -603,6 +622,19 @@ TEST_CASE("ExpressionParser2", "[common][events]") { { auto node = parser.ParseExpression("-123"); REQUIRE(node != nullptr); + auto &numberNode = dynamic_cast(*node); + auto type = gd::ExpressionTypeFinder::GetType( + platform, projectScopedContainers, "number", numberNode); + REQUIRE(type == "number"); + REQUIRE(numberNode.number == "-123"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + { + auto node = parser.ParseExpression("- 123"); + REQUIRE(node != nullptr); auto &unaryOperatorNode = dynamic_cast(*node); auto type = gd::ExpressionTypeFinder::GetType( platform, projectScopedContainers, "number", unaryOperatorNode); @@ -635,6 +667,19 @@ TEST_CASE("ExpressionParser2", "[common][events]") { { auto node = parser.ParseExpression("-123.2"); REQUIRE(node != nullptr); + auto &numberNode = dynamic_cast(*node); + auto type = gd::ExpressionTypeFinder::GetType( + platform, projectScopedContainers, "number", numberNode); + REQUIRE(type == "number"); + REQUIRE(numberNode.number == "-123.2"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + { + auto node = parser.ParseExpression("- 123.2"); + REQUIRE(node != nullptr); auto &unaryOperatorNode = dynamic_cast(*node); auto type = gd::ExpressionTypeFinder::GetType( platform, projectScopedContainers, "number", unaryOperatorNode); @@ -653,14 +698,11 @@ TEST_CASE("ExpressionParser2", "[common][events]") { { auto node = parser.ParseExpression("-123"); REQUIRE(node != nullptr); - auto &unaryOperatorNode = dynamic_cast(*node); + auto &numberNode = dynamic_cast(*node); auto type = gd::ExpressionTypeFinder::GetType( - platform, projectScopedContainers, "number|string", unaryOperatorNode); - REQUIRE(unaryOperatorNode.op == '-'); + platform, projectScopedContainers, "number|string", numberNode); REQUIRE(type == "number"); - auto &numberNode = - dynamic_cast(*unaryOperatorNode.factor); - REQUIRE(numberNode.number == "123"); + REQUIRE(numberNode.number == "-123"); gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); node->Visit(validator); @@ -685,14 +727,11 @@ TEST_CASE("ExpressionParser2", "[common][events]") { { auto node = parser.ParseExpression("-123.2"); REQUIRE(node != nullptr); - auto &unaryOperatorNode = dynamic_cast(*node); + auto &numberNode = dynamic_cast(*node); auto type = gd::ExpressionTypeFinder::GetType( - platform, projectScopedContainers, "number|string", unaryOperatorNode); - REQUIRE(unaryOperatorNode.op == '-'); + platform, projectScopedContainers, "number|string", numberNode); REQUIRE(type == "number"); - auto &numberNode = - dynamic_cast(*unaryOperatorNode.factor); - REQUIRE(numberNode.number == "123.2"); + REQUIRE(numberNode.number == "-123.2"); gd::ExpressionValidator validator(platform, projectScopedContainers, "number|string"); node->Visit(validator); @@ -700,6 +739,373 @@ TEST_CASE("ExpressionParser2", "[common][events]") { } } + SECTION("Negative numbers (edge cases)") { + // A negative number with a leading dot is normalized like a positive one. + { + auto node = parser.ParseExpression("-.5"); + REQUIRE(node != nullptr); + auto &numberNode = dynamic_cast(*node); + REQUIRE(numberNode.number == "-0.5"); + REQUIRE(numberNode.location.GetStartPosition() == 0); + REQUIRE(numberNode.location.GetEndPosition() == 3); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A negative number with a trailing dot is allowed. + { + auto node = parser.ParseExpression("-3."); + REQUIRE(node != nullptr); + auto &numberNode = dynamic_cast(*node); + REQUIRE(numberNode.number == "-3."); + REQUIRE(numberNode.location.GetStartPosition() == 0); + REQUIRE(numberNode.location.GetEndPosition() == 3); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // Leading zeros are stripped but the minus sign is kept. + { + auto node = parser.ParseExpression("-007"); + REQUIRE(node != nullptr); + auto &numberNode = dynamic_cast(*node); + REQUIRE(numberNode.number == "-7"); + REQUIRE(numberNode.location.GetStartPosition() == 0); + REQUIRE(numberNode.location.GetEndPosition() == 4); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // Negative zero is preserved. + { + auto node = parser.ParseExpression("-0"); + REQUIRE(node != nullptr); + auto &numberNode = dynamic_cast(*node); + REQUIRE(numberNode.number == "-0"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // Leading whitespace before a negative number does not break it. + { + auto node = parser.ParseExpression(" -123"); + REQUIRE(node != nullptr); + auto &numberNode = dynamic_cast(*node); + REQUIRE(numberNode.number == "-123"); + // The minus sign position becomes the location start. + REQUIRE(numberNode.location.GetStartPosition() == 3); + REQUIRE(numberNode.location.GetEndPosition() == 7); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // A `+` is never treated as a number sign: `+123` stays a unary operator + // applied to NumberNode("123"). + { + auto node = parser.ParseExpression("+123"); + REQUIRE(node != nullptr); + auto &unaryOperatorNode = dynamic_cast(*node); + REQUIRE(unaryOperatorNode.op == '+'); + auto &numberNode = + dynamic_cast(*unaryOperatorNode.factor); + REQUIRE(numberNode.number == "123"); + } + } + + SECTION("Negative numbers in binary operations") { + // `2*-3` becomes 2 * NumberNode("-3"): the unary minus is folded into the + // number, but the `*` remains a real binary operator. + { + auto node = parser.ParseExpression("2*-3"); + REQUIRE(node != nullptr); + auto &operatorNode = dynamic_cast(*node); + REQUIRE(operatorNode.op == '*'); + auto &leftNumberNode = + dynamic_cast(*operatorNode.leftHandSide); + REQUIRE(leftNumberNode.number == "2"); + auto &rightNumberNode = + dynamic_cast(*operatorNode.rightHandSide); + REQUIRE(rightNumberNode.number == "-3"); + // Location of the negative number should cover the minus. + REQUIRE(rightNumberNode.location.GetStartPosition() == 2); + REQUIRE(rightNumberNode.location.GetEndPosition() == 4); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `2/-3` + { + auto node = parser.ParseExpression("2/-3"); + REQUIRE(node != nullptr); + auto &operatorNode = dynamic_cast(*node); + REQUIRE(operatorNode.op == '/'); + auto &rightNumberNode = + dynamic_cast(*operatorNode.rightHandSide); + REQUIRE(rightNumberNode.number == "-3"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `1+-2` parses cleanly into NumberNode("1") + NumberNode("-2"). + { + auto node = parser.ParseExpression("1+-2"); + REQUIRE(node != nullptr); + auto &operatorNode = dynamic_cast(*node); + REQUIRE(operatorNode.op == '+'); + auto &rightNumberNode = + dynamic_cast(*operatorNode.rightHandSide); + REQUIRE(rightNumberNode.number == "-2"); + REQUIRE(rightNumberNode.location.GetStartPosition() == 2); + REQUIRE(rightNumberNode.location.GetEndPosition() == 4); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `1--2` (subtract a negative literal) parses to a binary `-` whose right + // hand side is the literal -2 (not a unary minus on 2). + { + auto node = parser.ParseExpression("1--2"); + REQUIRE(node != nullptr); + auto &operatorNode = dynamic_cast(*node); + REQUIRE(operatorNode.op == '-'); + auto &leftNumberNode = + dynamic_cast(*operatorNode.leftHandSide); + REQUIRE(leftNumberNode.number == "1"); + auto &rightNumberNode = + dynamic_cast(*operatorNode.rightHandSide); + REQUIRE(rightNumberNode.number == "-2"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `-1 + 2`: the negative literal is the left hand side of `+`. + { + auto node = parser.ParseExpression("-1 + 2"); + REQUIRE(node != nullptr); + auto &operatorNode = dynamic_cast(*node); + REQUIRE(operatorNode.op == '+'); + auto &leftNumberNode = + dynamic_cast(*operatorNode.leftHandSide); + REQUIRE(leftNumberNode.number == "-1"); + auto &rightNumberNode = + dynamic_cast(*operatorNode.rightHandSide); + REQUIRE(rightNumberNode.number == "2"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // Multiplicative precedence: `-2*3 + 1` → ((-2) * 3) + 1. + { + auto node = parser.ParseExpression("-2*3 + 1"); + REQUIRE(node != nullptr); + auto &plusNode = dynamic_cast(*node); + REQUIRE(plusNode.op == '+'); + auto &mulNode = dynamic_cast(*plusNode.leftHandSide); + REQUIRE(mulNode.op == '*'); + auto &mulLeft = dynamic_cast(*mulNode.leftHandSide); + REQUIRE(mulLeft.number == "-2"); + auto &mulRight = dynamic_cast(*mulNode.rightHandSide); + REQUIRE(mulRight.number == "3"); + auto &plusRight = dynamic_cast(*plusNode.rightHandSide); + REQUIRE(plusRight.number == "1"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // Whitespace around `-` keeps it a binary subtract. Ensure `2 - 3` is + // not collapsed into `2` followed by NumberNode("-3"). + { + auto node = parser.ParseExpression("2 - 3"); + REQUIRE(node != nullptr); + auto &operatorNode = dynamic_cast(*node); + REQUIRE(operatorNode.op == '-'); + auto &leftNumberNode = + dynamic_cast(*operatorNode.leftHandSide); + REQUIRE(leftNumberNode.number == "2"); + auto &rightNumberNode = + dynamic_cast(*operatorNode.rightHandSide); + // The right operand is the positive literal 3. + REQUIRE(rightNumberNode.number == "3"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + } + + SECTION("Negative numbers with parenthesis") { + // `(-3)` → SubExpression(NumberNode("-3")). + { + auto node = parser.ParseExpression("(-3)"); + REQUIRE(node != nullptr); + auto &subExpression = dynamic_cast(*node); + auto &numberNode = + dynamic_cast(*subExpression.expression); + REQUIRE(numberNode.number == "-3"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `-(3)` is *not* a negative literal: the `(` is not a digit so the parser + // takes the unary-operator path and wraps a SubExpression. + { + auto node = parser.ParseExpression("-(3)"); + REQUIRE(node != nullptr); + auto &unaryOperatorNode = dynamic_cast(*node); + REQUIRE(unaryOperatorNode.op == '-'); + auto &subExpression = + dynamic_cast(*unaryOperatorNode.factor); + auto &numberNode = + dynamic_cast(*subExpression.expression); + REQUIRE(numberNode.number == "3"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `-(-3)` is a unary minus around a sub-expression containing a negative + // literal. + { + auto node = parser.ParseExpression("-(-3)"); + REQUIRE(node != nullptr); + auto &unaryOperatorNode = dynamic_cast(*node); + REQUIRE(unaryOperatorNode.op == '-'); + auto &subExpression = + dynamic_cast(*unaryOperatorNode.factor); + auto &numberNode = + dynamic_cast(*subExpression.expression); + REQUIRE(numberNode.number == "-3"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + } + + SECTION("Stacked unary operators with negative numbers") { + // `--3` is a unary minus on a NumberNode("-3"). + { + auto node = parser.ParseExpression("--3"); + REQUIRE(node != nullptr); + auto &unaryOperatorNode = dynamic_cast(*node); + REQUIRE(unaryOperatorNode.op == '-'); + auto &numberNode = + dynamic_cast(*unaryOperatorNode.factor); + REQUIRE(numberNode.number == "-3"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `+-3` is unary `+` around NumberNode("-3"). + { + auto node = parser.ParseExpression("+-3"); + REQUIRE(node != nullptr); + auto &unaryOperatorNode = dynamic_cast(*node); + REQUIRE(unaryOperatorNode.op == '+'); + auto &numberNode = + dynamic_cast(*unaryOperatorNode.factor); + REQUIRE(numberNode.number == "-3"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `-+3` is unary `-` around a unary `+` around NumberNode("3") (the `+` is + // never folded into the literal). + { + auto node = parser.ParseExpression("-+3"); + REQUIRE(node != nullptr); + auto &outerUnary = dynamic_cast(*node); + REQUIRE(outerUnary.op == '-'); + auto &innerUnary = + dynamic_cast(*outerUnary.factor); + REQUIRE(innerUnary.op == '+'); + auto &numberNode = dynamic_cast(*innerUnary.factor); + REQUIRE(numberNode.number == "3"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + } + + SECTION("Unary minus on non-numbers stays a unary operator") { + // `-MyExtension::GetNumber()`: the `-` is not followed by a digit, so it + // must stay a unary operator. + { + auto node = parser.ParseExpression("-MyExtension::GetNumber()"); + REQUIRE(node != nullptr); + auto &unaryOperatorNode = dynamic_cast(*node); + REQUIRE(unaryOperatorNode.op == '-'); + auto &functionNode = + dynamic_cast(*unaryOperatorNode.factor); + REQUIRE(functionNode.functionName == "MyExtension::GetNumber"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + // `-(123)`: `-` followed by an opening parenthesis is also a unary + // operator (the `(` is not a number first char). + { + auto node = parser.ParseExpression("-(123)"); + REQUIRE(node != nullptr); + auto &unaryOperatorNode = dynamic_cast(*node); + REQUIRE(unaryOperatorNode.op == '-'); + auto &subExpression = + dynamic_cast(*unaryOperatorNode.factor); + auto &numberNode = + dynamic_cast(*subExpression.expression); + REQUIRE(numberNode.number == "123"); + } + } + + SECTION("Negative numbers as function arguments") { + auto node = parser.ParseExpression( + "MyExtension::GetNumberWith3Params(-1, \"hello\", -2.5)"); + REQUIRE(node != nullptr); + auto &functionNode = dynamic_cast(*node); + REQUIRE(functionNode.parameters.size() == 3); + auto &firstArg = + dynamic_cast(*functionNode.parameters[0]); + REQUIRE(firstArg.number == "-1"); + auto &thirdArg = + dynamic_cast(*functionNode.parameters[2]); + REQUIRE(thirdArg.number == "-2.5"); + + gd::ExpressionValidator validator(platform, projectScopedContainers, "number"); + node->Visit(validator); + REQUIRE(validator.GetFatalErrors().size() == 0); + } + + SECTION("Negative number in a string-only context is rejected") { + auto node = parser.ParseExpression("-123"); + 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)."); + // The error should report the position of the literal, including the + // minus sign. + REQUIRE(validator.GetFatalErrors()[0]->GetStartPosition() == 0); + REQUIRE(validator.GetFatalErrors()[0]->GetEndPosition() == 4); + } + SECTION("Invalid unary operators") { { auto node = parser.ParseExpression("*123"); diff --git a/Core/tests/ExpressionParser2NodePrinter.cpp b/Core/tests/ExpressionParser2NodePrinter.cpp index c0584ad9a4..5638a7b387 100644 --- a/Core/tests/ExpressionParser2NodePrinter.cpp +++ b/Core/tests/ExpressionParser2NodePrinter.cpp @@ -91,6 +91,29 @@ TEST_CASE("ExpressionParser2NodePrinter", "[common][events]") { testPrinter("number", "- + - 000123.4", "-+-123.4"); } + SECTION("Negative number literals round-trip") { + // A negative number literal is preserved as-is, with no extra space. + testPrinter("number", "-123"); + testPrinter("number", "-3.14"); + testPrinter("number", "-3."); + testPrinter("number", "-.5", "-0.5"); + testPrinter("number", "-0"); + testPrinter("number", "-007", "-7"); + } + + SECTION("Negative numbers inside expressions round-trip") { + // `1+-2` keeps the inner negative literal compact while the outer `+` is + // spaced like any other binary operator. + testPrinter("number", "1+-2", "1 + -2"); + testPrinter("number", "1--2", "1 - -2"); + testPrinter("number", "2*-3", "2 * -3"); + testPrinter("number", "2/-3", "2 / -3"); + testPrinter("number", "-1+2", "-1 + 2"); + testPrinter("number", "-1*-2", "-1 * -2"); + // No spaces are introduced between the leading `-` and the literal. + testPrinter("number", " -123 ", "-123"); + } + SECTION("Valid unary operators with parenthesis") { testPrinter("number", "-(123)"); testPrinter("number", "+((123))");