Inline spread elements where possible (bublejs#179)

My use case that leads me to desire this is code conditional on
compile-time constants, e.g. `{ …, ...FEATURE ? { … } : { … } }` (with
Rollup’s tree-shaking resolving the FEATURE ternary before it gets to
Bublé). I first implemented what amounts to this change in Terser
(terser/terser#224), but then realised that I
was running Bublé before Terser, and so Terser never actually *got* the
spreads to inline. The next idea was running Terser, then Bublé, then
Terser again… yuck. Then I decided that this optimisation really
belonged in Bublé anyway: generating less code is good!

Implementation notes:

The ObjectExpression implementation was straightforward and I
implemented it in situ.

Then the ArrayExpression, CallExpression and NewExpression one: at first
I performed the spread inlining in spread()’s while loop, and that
seemed to work well, but I got scared of how ArrayExpression and
CallExpression both handled their “single element” cases specially, and
that they did not appear to be optional inasmuch as removing the
special-casing yielded exceptions in the test suite; so I extracted that
into a separate function. That ended up pleasing me more as well, with
clearly cut semantics and a smaller diff inside the *Expression files,
as they no longer needed to change their single-element special cases to
*not* activate on a SpreadElement containing an ArrayElement.

src/program/types/ArrayExpression.js

@@ -1,5 +1,5 @@
 import Node from '../Node.js';
-import spread, { isArguments } from '../../utils/spread.js';
+import spread, { isArguments, inlineSpreads } from '../../utils/spread.js';
 
 export default class ArrayExpression extends Node {
 	initialise(transforms) {
@@ -26,6 +26,7 @@ export default class ArrayExpression extends Node {
 		super.transpile(code, transforms);
 
 		if (transforms.spreadRest) {
+			inlineSpreads(code, this, this.elements);
 			// erase trailing comma after last array element if not an array hole
 			if (this.elements.length) {
 				const lastElement = this.elements[this.elements.length - 1];

src/program/types/CallExpression.js

@@ -1,5 +1,5 @@
 import Node from '../Node.js';
-import spread, { isArguments } from '../../utils/spread.js';
+import spread, { isArguments, inlineSpreads } from '../../utils/spread.js';
 import removeTrailingComma from '../../utils/removeTrailingComma.js';
 
 export default class CallExpression extends Node {
@@ -20,6 +20,11 @@ export default class CallExpression extends Node {
 	}
 
 	transpile(code, transforms) {
+		if (transforms.spreadRest && this.arguments.length) {
+			inlineSpreads(code, this, this.arguments);
+			// this.arguments.length may have changed, must retest.
+		}
+
 		if (transforms.spreadRest && this.arguments.length) {
 			let hasSpreadElements = false;
 			let context;

src/program/types/NewExpression.js

@@ -1,5 +1,5 @@
 import Node from '../Node.js';
-import spread, { isArguments } from '../../utils/spread.js';
+import spread, { isArguments, inlineSpreads } from '../../utils/spread.js';
 import removeTrailingComma from '../../utils/removeTrailingComma.js';
 
 export default class NewExpression extends Node {
@@ -23,6 +23,11 @@ export default class NewExpression extends Node {
 	transpile(code, transforms) {
 		super.transpile(code, transforms);
 
+		if (transforms.spreadRest && this.arguments.length) {
+			inlineSpreads(code, this, this.arguments);
+			// this.arguments.length may have changed, must retest.
+		}
+
 		if (transforms.spreadRest && this.arguments.length) {
 			const firstArgument = this.arguments[0];
 			const isNew = true;

src/program/types/ObjectExpression.js

@@ -14,8 +14,35 @@ export default class ObjectExpression extends Node {
 		for (let i = 0; i < this.properties.length; ++i) {
 			const prop = this.properties[i];
 			if (prop.type === 'SpreadElement') {
-				spreadPropertyCount += 1;
-				if (firstSpreadProperty === null) firstSpreadProperty = i;
+				// First see if we can inline the spread, to save needing objectAssign.
+				const argument = prop.argument;
+				if (
+					argument.type === 'ObjectExpression' || (
+						argument.type === 'Literal' &&
+						typeof argument.value !== 'string'
+					)
+				) {
+					if (argument.type === 'ObjectExpression' && argument.properties.length > 0) {
+						// Strip the `...{` and the `}` with a possible trailing comma before it,
+						// leaving just the possible trailing comma after it.
+						code.remove(prop.start, argument.properties[0].start);
+						code.remove(argument.properties[argument.properties.length - 1].end, prop.end);
+						this.properties.splice(i, 1, ...argument.properties);
+						i--;
+					} else {
+						// An empty object, boolean, null, undefined, number or regexp (but NOT
+						// string) will spread to nothing, so just remove the element altogether,
+						// including a possible trailing comma.
+						code.remove(prop.start, i === this.properties.length - 1
+							? prop.end
+							: this.properties[i + 1].start);
+						this.properties.splice(i, 1);
+						i--;
+					}
+				} else {
+					spreadPropertyCount += 1;
+					if (firstSpreadProperty === null) firstSpreadProperty = i;
+				}
 			} else if (prop.computed && transforms.computedProperty) {
 				computedPropertyCount += 1;
 				if (firstComputedProperty === null) firstComputedProperty = i;

src/utils/spread.js

@@ -2,6 +2,59 @@ export function isArguments(node) {
 	return node.type === 'Identifier' && node.name === 'arguments';
 }
 
+export function inlineSpreads(
+	code,
+	node,
+	elements
+) {
+	let i = elements.length;
+
+	while (i--) {
+		const element = elements[i];
+		if (!element || element.type !== 'SpreadElement') {
+			continue;
+		}
+		const argument = element.argument;
+		if (argument.type !== 'ArrayExpression') {
+			continue;
+		}
+		const subelements = argument.elements;
+		if (subelements.some(subelement => subelement === null)) {
+			// Not even going to try inlining spread arrays with holes.
+			// It's a lot of work (got to be VERY careful in comma counting for
+			// ArrayExpression, and turn blanks into undefined for
+			// CallExpression and NewExpression), and probably literally no one
+			// would ever benefit from it.
+			continue;
+		}
+		// We can inline it: drop the `...[` and `]` and sort out any commas.
+		const isLast = i === elements.length - 1;
+		if (subelements.length === 0) {
+			code.remove(
+				isLast && i !== 0
+					? elements[i - 1].end  // Take the previous comma too
+					: element.start,
+				isLast
+					? node.end - 1  // Must remove trailing comma; element.end wouldn’t
+					: elements[i + 1].start);
+		} else {
+			// Strip the `...[` and the `]` with a possible trailing comma before it,
+			// leaving just the possible trailing comma after it.
+			code.remove(element.start, subelements[0].start);
+			code.remove(
+				// Strip a possible trailing comma after the last element
+				subelements[subelements.length - 1].end,
+				// And also a possible trailing comma after the spread
+				isLast
+					? node.end - 1
+					: element.end
+			);
+		}
+		elements.splice(i, 1, ...subelements);
+		i += subelements.length;
+	}
+}
+
 export default function spread(
 	code,
 	elements,

test/samples/computed-properties.js

@@ -238,15 +238,15 @@ module.exports = [
 			let a = {
 				[foo] (x, y) {
 					return {
-						...{abc: '123'}
+						...c
 					};
 				},
 			};
 		`,
 		output: `
 			var a = {};
 			a[foo] = function (x, y) {
-					return Object.assign({}, {abc: '123'});
+					return Object.assign({}, c);
 				};
 		`
 	},

test/samples/object-rest-spread.js

@@ -277,6 +277,61 @@ for( var a = c.a, rest = objectWithoutProperties( c, ["a"] ), b = rest;; ) {}`
 	input: `for( var {...b} = c;; ) {}`,
 	output: `function objectWithoutProperties (obj, exclude) { var target = {}; for (var k in obj) if (Object.prototype.hasOwnProperty.call(obj, k) && exclude.indexOf(k) === -1) target[k] = obj[k]; return target; }
 for( var rest = objectWithoutProperties( c, [] ), b = rest;; ) {}`
-	}
+	},
+
+	{
+		description: 'inlines object spread with one object',
+		input: `var obj = {...{a: 1}};`,
+		output: `var obj = {a: 1};`
+	},
+
+	{
+		description: 'inlines object spread with two objects',
+		input: `var obj = {...{a: 1}, ...{b: 2}};`,
+		output: `var obj = {a: 1, b: 2};`
+	},
 
+	{
+		description: 'inlines object spread with regular keys in between',
+		input: `var obj = { ...{a: 1}, b: 2, c: 3 };`,
+		output: `var obj = { a: 1, b: 2, c: 3 };`
+	},
+
+	{
+		description: 'inlines object spread mixed',
+		input: `var obj = { ...{a: 1}, b: 2, ...{c: 3}, e};`,
+		output: `var obj = { a: 1, b: 2, c: 3, e: e};`
+	},
+
+	{
+		description: 'inlines object spread very mixed',
+		options: {
+			objectAssign: 'Object.assign'
+		},
+		input: `var obj = { ...{a: 1}, b: 2, ...c, e};`,
+		output: `var obj = Object.assign({}, {a: 1, b: 2}, c, {e: e});`
+	},
+
+	{
+		description: 'inlines object spread without extraneous trailing commas',
+		options: {
+			objectAssign: 'Object.assign'
+		},
+		input: `
+			var obj = { ...{a: 1,}, b: 2, ...{c: 3,}, ...d, e, ...{f: 6,},};
+			obj = { a: 1, b: 2, };
+			obj = { a: 1, ...{b: 2} };
+			obj = { a: 1, ...{b: 2,} };
+			obj = { a: 1, ...{b: 2}, };
+			obj = { a: 1, ...{b: 2,}, };
+		`,
+		output: `
+			var obj = Object.assign({}, {a: 1, b: 2, c: 3}, d, {e: e, f: 6});
+			obj = { a: 1, b: 2, };
+			obj = { a: 1, b: 2 };
+			obj = { a: 1, b: 2 };
+			obj = { a: 1, b: 2, };
+			obj = { a: 1, b: 2, };
+		`
+	}
 ];