Page MenuHomePhabricator

Bad newline after return statement by JavaScript minifier
Closed, ResolvedPublic

Description

After checkout dec800968eb618c1f8632df7be49783fc82078d2 I got in the JavaScript console of Firefox the warning

unreachable code after return statement

The debugger points the following code part:

function createInputPseudo(type){return function(elem){var name=elem.nodeName.toLowerCase();return
name==="input"&&elem.type===type;}

In pretty format this is

function createInputPseudo(type) {
  return function (elem) {
    var name = elem.nodeName.toLowerCase();
    return
    name === 'input' && elem.type === type;
  };
}

The source of this code is resources/lib/jquery/jquery.js with the code:

/**
 * Returns a function to use in pseudos for input types
 * @param {String} type
 */
function createInputPseudo( type ) {
	return function( elem ) {
		var name = elem.nodeName.toLowerCase();
		return name === "input" && elem.type === type;
	};
}

In the minified JavaScript code the line with the return at the end has 998 characters. Obviously the JavaScript minifier inserts here a newline as whitespace between the return and the name to get a line with less than 1000 characters. Firefox interprets the newline as missing semicolon and make a return; without return value. The return value get ignored and causes the warning.

I don't know why the JavaScript minifier do this with jquery.js. I can not reproduce the behavior with other files.


Diffs used for debugging this task:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

@Fomafix Nicely found. Thank you.

I can confirm the bug affecting both Chrome and Firefox. You can confirm it using https://codepen.io/Krinkle/full/GBPoBY.

In Firefox there is the benefit of a warning in the console:

unreachable code after return statement

Chrome does not have such warning, but it is equally broken in Chrome. As expected, because the ECMAScript spec requires this behaviour. New lines are not allowed between return and Expression in ReturnStatement. See https://www.ecma-international.org/ecma-262/6.0/index.html#sec-return-statement.

The fact that our JavaScriptMinifier tries to break lines after 1000 character is not new, and the fact that is (wrongly) assumes it is safe to insert a line break between return and the next token is a very bad bug that will have most certainly broken code before in production.

What has changed is that:

  1. Our minifier processes code in chunks of ~ 1000 characters, and a recent commit has, by pure chance and bad luck, resulted in the a return statement somewhere being close to the end of such chunk - triggering this bug.
  2. Firefox now detects this kind of issue, whereas previously we were blind to it.
Krinkle triaged this task as Unbreak Now! priority.

Beta case

I can see this in Beta consistently at load.php?...

With the offending lines as follows:

beta.wmflabs.org/w/load.php
newSelector));return results;}catch(qsaError){}finally{if(nid===expando){context.removeAttribute("id");}}}}}}return select(selector.replace(rtrim,"$1"),context,results,seed);}function createCache(){var keys=[];function cache(key,value){if(keys.push(key+" ")>Expr.cacheLength){delete cache[keys.shift()];}return(cache[key+" "]=value);}return cache;}function markFunction(fn){fn[expando]=true;return fn;}function assert(fn){var el=document.createElement("fieldset");try{return!!fn(el);}catch(e){return false;}finally{if(el.parentNode){el.parentNode.removeChild(el);}el=null;}}function addHandle(attrs,handler){var arr=attrs.split("|"),i=arr.length;while(i--){Expr.attrHandle[arr[i]]=handler;}}function siblingCheck(a,b){var cur=b&&a,diff=cur&&a.nodeType===1&&b.nodeType===1&&a.sourceIndex-b.sourceIndex;if(diff){return diff;}if(cur){while((cur=cur.nextSibling)){if(cur===b){return-1;}}}return a?1:-1;}function createInputPseudo(type){return function(elem){var name=elem.nodeName.toLowerCase();return

/* unexpected line break */

name==="input"&&elem.type===type;};}function createButtonPseudo(type){return function(elem){var name=elem.nodeName.toLowerCase();return(name==="input"||name==="button")&&elem.type===type;};}function createDisabledPseudo(disabled){return function(elem){if("form"in elem){if(elem.parentNode&&elem.disabled===false){if("label"in elem){if("label"in elem.parentNode){return elem.parentNode.disabled===disabled;}else{return elem.disabled===disabled;}}return elem.isDisabled===disabled||elem.isDisabled!==!disabled&&disabledAncestor(elem)===disabled;}return elem.disabled===disabled;}else if("label"in elem){return elem.disabled===disabled;}return false;};}function createPositionalPseudo(fn){return markFunction(function(argument){argument=+argument;return markFunction(function(seed,matches){var j,matchIndexes=fn([],seed.length,argument),i=matchIndexes.length;while(i--){if(seed[(j=matchIndexes[i])]){seed[j]=!(matches[j]=seed[j]);}}});});}function testContext(context){return context&&typeof context.

Locally

I am able to reproduce this locally with the the following:

global $IP;
$code = file_get_contents( $IP .'/resources/lib/jquery/jquery.js' );
$minified = JavaScriptMinifier::minify(
	'mw.loader.implement("jquery@057yo40",function($,jQuery,require,module){'
	. $code
	. '});'
);
foreach (explode("\n",$minified) as $i => $line) {
	if (strpos($line,' createInputPseudo') !== false) {
		var_dump($line);
	}
}
string(997) "newSelector));return results;}catch(qsaError){}finally{if(nid===expando){context.removeAttribute("id");}}}}}}return select(selector.replace(rtrim,"$1"),context,results,seed);}function createCache(){var keys=[];function cache(key,value){if(keys.push(key+" ")>Expr.cacheLength){delete cache[keys.shift()];}return(cache[key+" "]=value);}return cache;}function markFunction(fn){fn[expando]=true;return fn;}function assert(fn){var el=document.createElement("fieldset");try{return!!fn(el);}catch(e){return false;}finally{if(el.parentNode){el.parentNode.removeChild(el);}el=null;}}function addHandle(attrs,handler){var arr=attrs.split("|"),i=arr.length;while(i--){Expr.attrHandle[arr[i]]=handler;}}function siblingCheck(a,b){var cur=b&&a,diff=cur&&a.nodeType===1&&b.nodeType===1&&a.sourceIndex-b.sourceIndex;if(diff){return diff;}if(cur){while((cur=cur.nextSibling)){if(cur===b){return-1;}}}return a?1:-1;}function createInputPseudo(type){return function(elem){var name=elem.nodeName.toLowerCase();return"

Debug

When JavaScriptMinifier inserted the bad line break, it has the following state:

## Line break after return
array(7) {
  'lineLength' =>
  int(997)
  'end' =>
  int(25191)
  'pos' =>
  int(25187)
  'token' =>
  string(4) "name"
  'state' =>
  int(10) # PAREN_EXPRESSION
  'type' =>
  int(17) # TYPE_LITERAL
  'semicolon_state_type' =>
  NULL
}
JavaScriptMinifier.php.diff
            $type = $tokenTypes[$token] ?? self::TYPE_LITERAL;
 
+
+           $badLineEnding = 'return';
+           $isAfterReturn = substr($out, -(strlen($badLineEnding))) === $badLineEnding;
+           if ($isAfterReturn) {
+               echo "\n\n ## Return statement\n";
+               var_dump([
+                   'lineLength' => $lineLength,
+                   'end' => $end,
+                   'pos' => $pos,
+                   'token' => $token, // "name"
+                   'state' => $state, // 10 = PAREN_EXPRESSION
+                   'type' => $type, // 17 = TYPE_LITERAL
+                   'semicolon_state_type' => @$semicolon[$state][$type],
+               ]);
+           }
+
            if ( $newlineFound && isset( $semicolon[$state][$type] ) ) {
                // This token triggers the semicolon insertion mechanism of javascript. While we
                // could add the ; token here ourselves, keeping the newline has a few advantages.
@@ -588,6 +604,10 @@ class JavaScriptMinifier {
                // This line would get too long if we added $token, so add a newline first.
                // Only do this if it won't trigger semicolon insertion and if it won't
                // put a postfix increment operator on its own line, which is illegal in js.
+               $badLineEnding = 'return';
+               if ($isAfterReturn) {
+                   echo "... Inserting a bad line ending?\n";
+               }
                $out .= "\n";

The input of jquery.js is somehow making JavaScriptMinifier reach a state where it thinks a return statement is actually a parenthesised expression (PAREN_EXPRESSION).

Thus far, I've not been able to reproduce this with other input. Each time, JavaScriptMinifier is behaving correctly. And this makes sense, since its parser does actually know that return statements must not have line breaks before their expression, and it knows that semi colons (or line breaks) must not be inserted in between return and an expression.

Every kind of input I've thrown at it so far, makes it break either before return or after return <something>. Never (incorrectly) between return and <something>.

The below example has all hierarchical context from createInputPseudo in jquery.js, and this does behave correctly.

	$minified = JavaScriptMinifier::minify(
<<<JAVASCRIPT
mw.loader.implement("jquery@057yo40",function($,jQuery,require,module){
	( function( global, factory ) {
		"use strict";
		factory( global );
	} )( typeof window !== "undefined" ? window : this, function( window, noGlobal ) {
		"use strict";

		var Sizzle = (function(window){
			var a, b, c = 0;
			function createInputPseudo(type) {
				return function(elem) {
					var name = elem.nodeName.toLowerCase();
					return name === "input" && elem.type === type;
				};
			}
		}());
		return jQuery;
	} );
});
JAVASCRIPT
);
	echo $minified;
 ## Return statement
array(7) {
  'lineLength' =>
  int(6)
  'end' =>
  int(424)
  'pos' =>
  int(420)
  'token' =>
  string(4) "name"
  'state' =>
  int(4) # EXPRESSION_NO_NL
  'type' =>
  int(17) # TYPE_LITERAL
  'semicolon_state_type' =>
  bool(true)
}

For this input, JavaScriptMinifier is correctly seeing the return statement as EXPRESSION_NO_NL, producing the following output with MAX_LINE_LENGTH=1.

output
toLowerCase
(
)
;
return name # Works as expected
===
"input"
&&
# [..]

Okay, I've reduced it to the following.

call(function(){
	try {
	} catch ( e ) {
		push = {
			apply: true ? 0 : {}
		};
	}
	return name === "input";
});

Log of state changes (using P7445)

statetokentypestack
state: STATEMENTtoken: calltype: TYPE_LITERALstack:
state: EXPRESSION_OPtoken: (type: TYPE_PAREN_OPENstack: EXPRESSION_OP
state: PAREN_EXPRESSIONtoken: functiontype: TYPE_FUNCstack: EXPRESSION_OP
state: PAREN_EXPRESSION_FUNCtoken: (type: TYPE_PAREN_OPENstack: EXPRESSION_OP
state: PAREN_EXPRESSION_FUNCtoken: )type: TYPE_PAREN_CLOSEstack: EXPRESSION_OP
state: PAREN_EXPRESSION_FUNCtoken: {type: TYPE_BRACE_OPENstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: STATEMENTtoken: trytype: TYPE_DOstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: STATEMENTtoken: {type: TYPE_BRACE_OPENstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: STATEMENTtoken: }type: TYPE_BRACE_CLOSEstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: STATEMENTtoken: catchtype: TYPE_IFstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: CONDITIONtoken: (type: TYPE_PAREN_OPENstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: PAREN_EXPRESSIONtoken: etype: TYPE_LITERALstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: PAREN_EXPRESSION_OPtoken: )type: TYPE_PAREN_CLOSEstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: STATEMENTtoken: {type: TYPE_BRACE_OPENstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: STATEMENTtoken: pushtype: TYPE_LITERALstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: EXPRESSION_OPtoken: =type: TYPE_BIN_OPstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: EXPRESSIONtoken: {type: TYPE_BRACE_OPENstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: PROPERTY_ASSIGNMENTtoken: applytype: TYPE_LITERALstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: PROPERTY_ASSIGNMENTtoken: :type: TYPE_COLONstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: PROPERTY_EXPRESSIONtoken: truetype: TYPE_LITERALstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: PROPERTY_EXPRESSION_OPtoken: ?type: TYPE_HOOKstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: PROPERTY_EXPRESSIONtoken: 0type: TYPE_LITERALstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: PROPERTY_EXPRESSION_OPtoken: :type: TYPE_COLONstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: PROPERTY_EXPRESSION_OPtoken: {type: TYPE_BRACE_OPENstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: PROPERTY_EXPRESSION_OPtoken: }type: TYPE_BRACE_CLOSEstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT>EXPRESSION_OP
state: EXPRESSION_OPtoken: }type: TYPE_BRACE_CLOSEstack: EXPRESSION_OP>PAREN_EXPRESSION_OP>STATEMENT
state: STATEMENTtoken: ;type: TYPE_SEMICOLONstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: STATEMENTtoken: }type: TYPE_BRACE_CLOSEstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: PAREN_EXPRESSION_OPtoken: returntype: TYPE_RETURNstack: EXPRESSION_OP
state: PAREN_EXPRESSION_OPtoken: nametype: TYPE_LITERALstack: EXPRESSION_OP
state: PAREN_EXPRESSION_OPtoken: ===type: TYPE_BIN_OPstack: EXPRESSION_OP
state: PAREN_EXPRESSIONtoken: "input"type: TYPE_LITERALstack: EXPRESSION_OP
state: PAREN_EXPRESSION_OPtoken: ;type: TYPE_SEMICOLONstack: EXPRESSION_OP
state: PAREN_EXPRESSIONtoken: }type: TYPE_BRACE_CLOSEstack: EXPRESSION_OP
state: PAREN_EXPRESSIONtoken: )type: TYPE_PAREN_CLOSEstack: EXPRESSION_OP
state: EXPRESSION_OPtoken: ;type: TYPE_SEMICOLONstack:
Output with maxLineLength 1
…
return #
name   # Bad!
===
"input"
;

EDIT: Added a stack column.

For comparison, here is a simple case where it does work correctly.

input
call(function(){
	return name === "input";
});

State changes

statetokentypestack
state: STATEMENTtoken: calltype: TYPE_LITERALstack:
state: EXPRESSION_OPtoken: (type: TYPE_PAREN_OPENstack: EXPRESSION_OP
state: PAREN_EXPRESSIONtoken: functiontype: TYPE_FUNCstack: EXPRESSION_OP
state: PAREN_EXPRESSION_FUNCtoken: (type: TYPE_PAREN_OPENstack: EXPRESSION_OP
state: PAREN_EXPRESSION_FUNCtoken: )type: TYPE_PAREN_CLOSEstack: EXPRESSION_OP
state: PAREN_EXPRESSION_FUNCtoken: {type: TYPE_BRACE_OPENstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: STATEMENTtoken: returntype: TYPE_RETURNstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: EXPRESSION_NO_NLtoken: nametype: TYPE_LITERALstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: EXPRESSION_OPtoken: ===type: TYPE_BIN_OPstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: EXPRESSIONtoken: "input"type: TYPE_LITERALstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: EXPRESSION_OPtoken: ;type: TYPE_SEMICOLONstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: STATEMENTtoken: }type: TYPE_BRACE_CLOSEstack: EXPRESSION_OP>PAREN_EXPRESSION_OP
state: PAREN_EXPRESSION_OPtoken: )type: TYPE_PAREN_CLOSEstack: EXPRESSION_OP
state: EXPRESSION_OPtoken: ;type: TYPE_SEMICOLONstack:
out
call
(
function
(
)
{
return name # Good!
===
"input"
;
}
)
;

EDIT: Added a stack column.

Change 451899 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Add test case for T201606

https://gerrit.wikimedia.org/r/451899

Change 451900 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Disambiguate token constants from state constants

https://gerrit.wikimedia.org/r/451900

Change 451901 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Document every operator and token type, with spec ref

https://gerrit.wikimedia.org/r/451901

It is necessary to look at the stack, because the state machine corrupt the stack and therefor the necessary state is wrong later on

To trigger the bug in the stack, it needs a propery assigment with a property expression (for example a ternary operation)

The brace inside the property expression does not get a new level in the stack, but the closing brace reduce the level.

p = { a: true ? 0 : { b: "C" } };

p -> propery assigment
{  -> first stack: expression op
a: -> property expression
true -> property expression op
next { -> still inside the property expression op, but it must open a new stack
C -> still inside property expression op
} -> property expression op ends and stack gets reduced

[Another explanation: The inner { b: "C" } must get its own level, but that is not done. Add debug of the stack and it is visible that the inner {} still working with a depth of 1, but it must at depth of 2]

I would say that $push[self::PROPERTY_EXPRESSION_OP][self::TYPE_BRACE_OPEN] = self::PROPERTY_EXPRESSION_OP is missing.
That would say the state machine that a brace inside parenthesis expression can open a new stack.

Maybe also add
$push[self::PAREN_EXPRESSION_OP][self::TYPE_BRACE_OPEN] = self::PAREN_EXPRESSION_OP, but I have not looked at it or have an code examle

Change 452001 had a related patch set uploaded (by Umherirrender; owner: Umherirrender):
[mediawiki/core@master] JavaScriptMinifier: Propose fix for T201606

https://gerrit.wikimedia.org/r/452001

I would say that $push[self::PROPERTY_EXPRESSION_OP][self::TYPE_BRACE_OPEN] = self::PROPERTY_EXPRESSION_OP is missing.
That would say the state machine that a brace inside parenthesis expression can open a new stack.

Maybe also add
$push[self::PAREN_EXPRESSION_OP][self::TYPE_BRACE_OPEN] = self::PAREN_EXPRESSION_OP, but I have not looked at it or have an code examle

@Umherirrender Exactly right -- we tracked this down a couple of hours ago, and @Krinkle is in the process of filing the relevant patches.

Another way of saying this is that there was a mismatch between the push conditions (https://github.com/wikimedia/mediawiki/blob/master/includes/libs/JavaScriptMinifier.php#L343) and the pop conditions (https://github.com/wikimedia/mediawiki/blob/master/includes/libs/JavaScriptMinifier.php#L361) -- one operated on braces, one operated on parens, where they ought to have actually been symmetric.

By contrast, PAREN_EXPRESSION_OP is symmetric: https://github.com/wikimedia/mediawiki/blob/master/includes/libs/JavaScriptMinifier.php#L333 and https://github.com/wikimedia/mediawiki/blob/master/includes/libs/JavaScriptMinifier.php#L359 both match on parens.

Change 451899 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Add test case for T201606

https://gerrit.wikimedia.org/r/451899

Change 451900 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Disambiguate token constants from state constants

https://gerrit.wikimedia.org/r/451900

Change 451901 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Document every operator and token type, with spec ref

https://gerrit.wikimedia.org/r/451901

Change 452011 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Add better line-breaker tests

https://gerrit.wikimedia.org/r/452011

@Umherirrender Thanks for helping out! Ian and I talked privately and also concluded that we need the stack for this. I've amended my comments now to also include stack column, which indeed was needed to understand the bug.

Change 452019 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Fix bad state after '{}' in property value

https://gerrit.wikimedia.org/r/452019

Change 452001 abandoned by Umherirrender:
JavaScriptMinifier: Propose fix for T201606

Reason:
Better to go with I07b809a7ca56e282ecb48b5c89c217b4b8da6856

https://gerrit.wikimedia.org/r/452001

Change 452011 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Add better line-breaker tests

https://gerrit.wikimedia.org/r/452011

Change 452019 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Fix bad state after '{}' in property value

https://gerrit.wikimedia.org/r/452019

Change 452045 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] resourceloader: Increase minification cache version

https://gerrit.wikimedia.org/r/452045

Change 452175 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Move test case for 'x++' to provideLineBreaker()

https://gerrit.wikimedia.org/r/452175

Change 452176 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Add test case for another line-break bug

https://gerrit.wikimedia.org/r/452176

Change 452175 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Move test case for 'x++' to provideLineBreaker()

https://gerrit.wikimedia.org/r/452175

Change 452176 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Add test case for another line-break bug

https://gerrit.wikimedia.org/r/452176

[mediawiki/core@master] JavaScriptMinifier: Add test case for another line-break bug
https://gerrit.wikimedia.org/r/452176

Using P7449 as instrument, this test case produces the following state transitions.

call( {
	key: 1 ? 0 : function () {
		return this;
	}
} );
1) JavaScriptMinifierTest::testLineBreaker …
This test printed output: 
| stack | 
| token: `call` | action = goto EXPRESSION_OP
| stack | 
| token: `(` | action = stack_push EXPRESSION_OP | action = goto PAREN_EXPRESSION
| stack | EXPRESSION_OP
| token: `{` | action = stack_push PAREN_EXPRESSION_OP | action = goto PROPERTY_ASSIGNMENT
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `key`
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `:` | action = goto PROPERTY_EXPRESSION
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `1` | action = goto PROPERTY_EXPRESSION_OP
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `?` | action = goto PROPERTY_EXPRESSION
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `0` | action = goto PROPERTY_EXPRESSION_OP
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `:`
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `function`
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `(` | action = stack_push PROPERTY_EXPRESSION_OP | action = goto PAREN_EXPRESSION
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP>PROPERTY_EXPRESSION_OP
| token: `)` | action = stack_pop PROPERTY_EXPRESSION_OP
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `{` | action = stack_push PROPERTY_EXPRESSION_OP
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP>PROPERTY_EXPRESSION_OP
| token: `return`
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP>PROPERTY_EXPRESSION_OP
| token: `this`
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP>PROPERTY_EXPRESSION_OP
| token: `;`
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP>PROPERTY_EXPRESSION_OP
| token: `}` | action = stack_pop PROPERTY_EXPRESSION_OP
| stack | EXPRESSION_OP>PAREN_EXPRESSION_OP
| token: `}` | action = stack_pop PAREN_EXPRESSION_OP
| stack | EXPRESSION_OP
| token: `)` | action = stack_pop EXPRESSION_OP
| stack | 
| token: `;` | action = goto STATEMENT

It stands out that we never enter the state of EXPRESSION_TERNARY oder EXPRESSION_TERNARY_FUNC, despite there being a ternary with a function expression. Instead, it remains flatly in the PAREN_EXPRESSION_OP state. It does not add the object literal to the stack, nor the function. Thus, return is not valid there, and no line break is prevented.

Change 452431 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] benchmarks: Add benchmark for JavaScriptMinifier

https://gerrit.wikimedia.org/r/452431

Change 452432 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Turn $goto into a generic $model

https://gerrit.wikimedia.org/r/452432

Change 452433 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Merge $push and $pop into $model

https://gerrit.wikimedia.org/r/452433

Change 452434 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[mediawiki/core@master] JavaScriptMinifier: Fix bad state after ternary in object literal

https://gerrit.wikimedia.org/r/452434

Change 452431 merged by jenkins-bot:
[mediawiki/core@master] benchmarks: Add benchmark for JavaScriptMinifier

https://gerrit.wikimedia.org/r/452431

Change 452432 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Turn $goto into a generic $model

https://gerrit.wikimedia.org/r/452432

Change 452433 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Merge $push and $pop into $model

https://gerrit.wikimedia.org/r/452433

Change 452434 merged by jenkins-bot:
[mediawiki/core@master] JavaScriptMinifier: Fix bad state after ternary in object literal

https://gerrit.wikimedia.org/r/452434

Change 452045 merged by jenkins-bot:
[mediawiki/core@master] resourceloader: Increase minification cache version

https://gerrit.wikimedia.org/r/452045

The unreachable code after return statement warning no longer appears for me in Firefox at https://en.wikipedia.beta.wmflabs.org/wiki/Sandbox. Marking as resolved.

Thanks again @Fomafix for noticing this!