JavaScriptMinifier: Fix bad state after ternary in object literal

The following pattern of input (found in jquery.js) triggered
this bug:

    call( {
        key: 1 ? 0 : function () {
            return this;
        }
    } );

The open brace changes state to PROPERTY_ASSIGNMENT (for object literals).
The colon after 'key' sets state to PROPERTY_EXPRESSION.

Each individual parts of an expression (identifiers and literal values)
is recognised with state *_EXPRESSION_OP, such as PROPERTY_EXPRESSION_OP.

The '1' after 'key:' correctly sets the state to PROPERTY_EXPRESSION_OP.
Upto there it goes well, but after that it goes wrong.

The question mark (TYPE_HOOK) in this context was wrongly switching
back to PROPERTY_EXPRESSION. That is a problem because that does not
handle TYPE_COLON, which meant '0: function' was seen together as a
sequence of continuous PROPERTY_EXPRESSION_OP where TYPE_FUNC may
not be handled.

Fixed by changing handling of TYPE_HOOK in PROPERTY_EXPRESSION to
switch states to EXPRESSION_TERNARY, and also performing a push
so that ternary handling can pop back to the property expression.

This mirrors the handling that already exists for ternaries in
the regular handling of EXPRESSION/EXPRESSION_OP (as opposed to
the variant for object literal properties).

Bug: T201606
Change-Id: I6104c839cfc3416257543b54a91b74cb4aa4193b

diff --git a/includes/libs/JavaScriptMinifier.php b/includes/libs/JavaScriptMinifier.php
index 3015825..73f3e70 100644 (file)
--- a/includes/libs/JavaScriptMinifier.php
+++ b/includes/libs/JavaScriptMinifier.php
@@ -334,6 +334,8 @@ class JavaScriptMinifier {
                                        self::ACTION_GOTO => self::PAREN_EXPRESSION,
                                ],
                        ],
+                       // Property assignment - This is an object literal declaration.
+                       // For example: `{ key: value }`
                        self::PROPERTY_ASSIGNMENT => [
                                self::TYPE_COLON => [
                                        self::ACTION_GOTO => self::PROPERTY_EXPRESSION,
@@ -520,6 +522,7 @@ class JavaScriptMinifier {
                                        self::ACTION_GOTO => self::STATEMENT,
                                ],
                        ],
+                       // Property expression - The value of a key in an object literal.
                        self::PROPERTY_EXPRESSION => [
                                self::TYPE_BRACE_OPEN => [
                                        self::ACTION_PUSH => self::PROPERTY_EXPRESSION_OP,
@@ -547,7 +550,8 @@ class JavaScriptMinifier {
                                        self::ACTION_GOTO => self::PROPERTY_EXPRESSION,
                                ],
                                self::TYPE_HOOK => [
-                                       self::ACTION_GOTO => self::PROPERTY_EXPRESSION,
+                                       self::ACTION_PUSH => self::PROPERTY_EXPRESSION,
+                                       self::ACTION_GOTO => self::EXPRESSION_TERNARY,
                                ],
                                self::TYPE_COMMA => [
                                        self::ACTION_GOTO => self::PROPERTY_ASSIGNMENT,

diff --git a/tests/phpunit/includes/libs/JavaScriptMinifierTest.php b/tests/phpunit/includes/libs/JavaScriptMinifierTest.php
index 16048bf..54dc583 100644 (file)
--- a/tests/phpunit/includes/libs/JavaScriptMinifierTest.php
+++ b/tests/phpunit/includes/libs/JavaScriptMinifierTest.php
@@ -317,7 +317,8 @@ JAVASCRIPT
                        [
                                // Regression test for T201606.
                                // Must not break between 'return' and Expression.
-                               // FIXME: Cause?
+                               // Was caused by bad state after a ternary in the expression value
+                               // for a key in an object literal.
                                <<<JAVASCRIPT
 call( {
        key: 1 ? 0 : function () {
@@ -340,7 +341,7 @@ JAVASCRIPT
                                        '(',
                                        ')',
                                        '{',
-                                       'return', 'this', // FIXME
+                                       'return this',
                                        ';',
                                        '}',
                                        '}',