Add SequenceExpression AST node resolving #1649

Looks at each expression in the sequence and only retains the node if it has an effect.

Note: in sequence expressions with effects always keep the last node, to be consistent when assigning the resulting value to a variable.

src/ast/nodes/ConditionalExpression.js

@@ -52,8 +52,8 @@ export default class ConditionalExpression extends Node {
 				code.remove( this.start, branchToRetain.start );
 				code.remove( branchToRetain.end, this.end );
 				if ( branchToRetain.type === 'SequenceExpression' ) {
-					code.prependRight( branchToRetain.start, '(' );
-					code.appendLeft( branchToRetain.end, ')' );
+					code.prependLeft( branchToRetain.start, '(' );
+					code.appendRight( branchToRetain.end, ')' );
 				}
 				branchToRetain.render( code, es );
 			}

src/ast/nodes/SequenceExpression.js

@@ -0,0 +1,59 @@
+import Node from '../Node.js';
+
+export default class SequenceExpression extends Node {
+	getValue () {
+		return this.expressions[ this.expressions.length - 1 ].getValue();
+	}
+
+	hasEffects ( options ) {
+		return this.expressions.some( expression => expression.hasEffects( options ) );
+	}
+
+	includeInBundle () {
+		if ( this.isFullyIncluded() ) return false;
+		let addedNewNodes = false;
+		if ( this.expressions[ this.expressions.length - 1 ].includeInBundle() ) {
+			addedNewNodes = true;
+		}
+		this.expressions.forEach( node => {
+			if ( node.shouldBeIncluded() ) {
+				if ( node.includeInBundle() ) {
+					addedNewNodes = true;
+				}
+			}
+		} );
+		if ( !this.included || addedNewNodes ) {
+			this.included = true;
+			return true;
+		}
+		return false;
+	}
+
+	render ( code, es ) {
+		if ( !this.module.bundle.treeshake ) {
+			super.render( code, es );
+		}
+
+		else {
+			const last = this.expressions[ this.expressions.length - 1 ];
+			const included = this.expressions.slice( 0, this.expressions.length - 1 ).filter( expression => expression.included );
+
+			if ( included.length === 0 ) {
+				code.remove( this.start, last.start );
+				code.remove( last.end, this.end );
+			}
+
+			else {
+				let previousEnd = this.start;
+				for ( const expression of included ) {
+					code.remove( previousEnd, expression.start );
+					code.appendLeft( expression.end, ', ' );
+					previousEnd = expression.end;
+				}
+
+				code.remove( previousEnd, last.start );
+				code.remove( last.end, this.end );
+			}
+		}
+	}
+}

src/ast/nodes/index.js

@@ -38,6 +38,7 @@ import Property from './Property.js';
 import RestElement from './RestElement.js';
 import ReturnStatement from './ReturnStatement.js';
 import Statement from './shared/Statement.js';
+import SequenceExpression from './SequenceExpression.js';
 import SwitchCase from './SwitchCase.js';
 import SwitchStatement from './SwitchStatement.js';
 import TaggedTemplateExpression from './TaggedTemplateExpression.js';
@@ -94,6 +95,7 @@ export default {
 	Property,
 	RestElement,
 	ReturnStatement,
+	SequenceExpression,
 	SwitchCase,
 	SwitchStatement,
 	TaggedTemplateExpression,

test/form/samples/conditional-put-parens-around-sequence/_expected/amd.js

@@ -1,10 +1,10 @@
 define(function () { 'use strict';
 
-	var a = (1, 2, 3 );
-	var b = (4, 5, 6 );
+	var a = (foo(), 3);
+	var b = (bar(), 6);
 	foo( a, b );
 
 	// verify works with no whitespace
-	bar((1,2),(7,8));
+	bar((foo(), 2),(bar(), 8));
 
 });

test/form/samples/conditional-put-parens-around-sequence/_expected/cjs.js

@@ -1,8 +1,8 @@
 'use strict';
 
-var a = (1, 2, 3 );
-var b = (4, 5, 6 );
+var a = (foo(), 3);
+var b = (bar(), 6);
 foo( a, b );
 
 // verify works with no whitespace
-bar((1,2),(7,8));
+bar((foo(), 2),(bar(), 8));

test/form/samples/conditional-put-parens-around-sequence/_expected/es.js

@@ -1,6 +1,6 @@
-var a = (1, 2, 3 );
-var b = (4, 5, 6 );
+var a = (foo(), 3);
+var b = (bar(), 6);
 foo( a, b );
 
 // verify works with no whitespace
-bar((1,2),(7,8));
+bar((foo(), 2),(bar(), 8));

test/form/samples/conditional-put-parens-around-sequence/_expected/iife.js

@@ -1,11 +1,11 @@
 (function () {
 	'use strict';
 
-	var a = (1, 2, 3 );
-	var b = (4, 5, 6 );
+	var a = (foo(), 3);
+	var b = (bar(), 6);
 	foo( a, b );
 
 	// verify works with no whitespace
-	bar((1,2),(7,8));
+	bar((foo(), 2),(bar(), 8));
 
 }());

test/form/samples/conditional-put-parens-around-sequence/_expected/umd.js

@@ -4,11 +4,11 @@
 	(factory());
 }(this, (function () { 'use strict';
 
-	var a = (1, 2, 3 );
-	var b = (4, 5, 6 );
+	var a = (foo(), 3);
+	var b = (bar(), 6);
 	foo( a, b );
 
 	// verify works with no whitespace
-	bar((1,2),(7,8));
+	bar((foo(), 2),(bar(), 8));
 
 })));

test/form/samples/conditional-put-parens-around-sequence/main.js

@@ -1,6 +1,6 @@
-var a = true  ? ( 1, 2, 3 ) : ( 4, 5, 6 );
-var b = false ? ( 1, 2, 3 ) : ( 4, 5, 6 );
+var a = true  ? ( 1, foo(), 3 ) : ( 4, bar(), 6 );
+var b = false ? ( 1, foo(), 3 ) : ( 4, bar(), 6 );
 foo( a, b );
 
 // verify works with no whitespace
-bar(true?(1,2):(3,4),false?(5,6):(7,8));
+bar(true?(foo(),2):(bar(),4),false?(foo(),6):(bar(),8));

test/form/samples/sequence-expression/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'only retain expressions with effects in sequence expressions (#1649)'
+};

test/form/samples/sequence-expression/_expected/amd.js

@@ -0,0 +1,14 @@
+define(function () { 'use strict';
+
+	// should remove expressions without side-effect, multiple effects
+	var a = (foo(), foo(), 2);
+	// without white-space, effect at the end
+	var b = (foo());
+
+	// should only keep final expression
+	var d = (2);
+	console.log(d);
+
+	// should infer value
+
+});

test/form/samples/sequence-expression/_expected/cjs.js

@@ -0,0 +1,12 @@
+'use strict';
+
+// should remove expressions without side-effect, multiple effects
+var a = (foo(), foo(), 2);
+// without white-space, effect at the end
+var b = (foo());
+
+// should only keep final expression
+var d = (2);
+console.log(d);
+
+// should infer value

test/form/samples/sequence-expression/_expected/es.js

@@ -0,0 +1,10 @@
+// should remove expressions without side-effect, multiple effects
+var a = (foo(), foo(), 2);
+// without white-space, effect at the end
+var b = (foo());
+
+// should only keep final expression
+var d = (2);
+console.log(d);
+
+// should infer value

test/form/samples/sequence-expression/_expected/iife.js

@@ -0,0 +1,15 @@
+(function () {
+	'use strict';
+
+	// should remove expressions without side-effect, multiple effects
+	var a = (foo(), foo(), 2);
+	// without white-space, effect at the end
+	var b = (foo());
+
+	// should only keep final expression
+	var d = (2);
+	console.log(d);
+
+	// should infer value
+
+}());

test/form/samples/sequence-expression/_expected/umd.js

@@ -0,0 +1,18 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory() :
+	typeof define === 'function' && define.amd ? define(factory) :
+	(factory());
+}(this, (function () { 'use strict';
+
+	// should remove expressions without side-effect, multiple effects
+	var a = (foo(), foo(), 2);
+	// without white-space, effect at the end
+	var b = (foo());
+
+	// should only keep final expression
+	var d = (2);
+	console.log(d);
+
+	// should infer value
+
+})));

test/form/samples/sequence-expression/main.js

@@ -0,0 +1,17 @@
+// should remove expressions without side-effect, multiple effects
+var a = (0, foo(), 1, foo(), 2);
+// without white-space, effect at the end
+var b = (0,1,foo());
+
+// should remove variable without effect
+var c = (1, 2);
+
+// should only keep final expression
+var d = (1, 2);
+console.log(d);
+
+// should infer value
+if ((1, 2) !== 2) {
+	console.log( 'effect' );
+}
+