Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Preserve empty for...of loops #3132

Merged
merged 13 commits into from
Sep 26, 2019
12 changes: 3 additions & 9 deletions src/ast/nodes/ForOfStatement.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import MagicString from 'magic-string';
import { NO_SEMICOLON, RenderOptions } from '../../utils/renderHelpers';
import { ExecutionPathOptions } from '../ExecutionPathOptions';
import BlockScope from '../scopes/BlockScope';
import Scope from '../scopes/Scope';
import { EMPTY_PATH } from '../values';
Expand All @@ -27,14 +26,9 @@ export default class ForOfStatement extends StatementBase {
this.scope = new BlockScope(parentScope);
}

hasEffects(options: ExecutionPathOptions): boolean {
return (
(this.left &&
(this.left.hasEffects(options) ||
this.left.hasEffectsWhenAssignedAtPath(EMPTY_PATH, options))) ||
(this.right && this.right.hasEffects(options)) ||
this.body.hasEffects(options.setIgnoreBreakStatements())
);
hasEffects(): boolean {
// Placeholder until proper Symbol.Iterator support
return true;
}

include(includeChildrenRecursively: IncludeChildren) {
Expand Down
17 changes: 0 additions & 17 deletions test/form/samples/effect-in-for-of-loop-in-functions/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,6 @@ function a () {

a();

function b () {
for ( const item of items.children ) {
// do nothing
}
}

b();

function c () {
let item;
for ( item of items.children ) {
Expand All @@ -25,15 +17,6 @@ function c () {

c();

function d () {
let item;
for ( item of items.children ) {
// do nothing
}
}

d();

assert.deepEqual( items, [
{ foo: 'a', bar: 'c' },
{ foo: 'a', bar: 'c' },
Expand Down
14 changes: 0 additions & 14 deletions test/form/samples/effect-in-for-of-loop/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,16 @@ for ( const a of items ) {
a.foo = 'a';
}

for ( const b of items ) {
// do nothing
}

let c;
for ( c of items ) {
c.bar = 'c';
}

let d;
for ( d of items ) {
// do nothing
}

for ( e of items ) {
e.baz = 'e';
}
var e;

for ( f of items ) {
// do nothing
}
var f;

assert.deepEqual( items, [
{ foo: 'a', bar: 'c', baz: 'e' },
{ foo: 'a', bar: 'c', baz: 'e' },
Expand Down
3 changes: 0 additions & 3 deletions test/form/samples/empty-for-of-statement/_config.js

This file was deleted.

2 changes: 0 additions & 2 deletions test/form/samples/empty-for-of-statement/_expected.js

This file was deleted.

5 changes: 0 additions & 5 deletions test/form/samples/empty-for-of-statement/main.js

This file was deleted.

3 changes: 3 additions & 0 deletions test/form/samples/for-of-scopes/_expected.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ var associated = () => {};
for ( var associated of [ effect1 ] ) {}
associated();

var effect2 = () => console.log( 'effect' );
for ( let shadowed of [ effect2 ] ) {}

var effect3 = () => console.log( 'effect' ); // Must not be removed!
for ( const foo of [ effect3 ] ) {
foo(); // Must not be removed!
Expand Down
4 changes: 0 additions & 4 deletions test/form/samples/side-effects-break-statements/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,6 @@ for ( const val in { x: 1, y: 2 } ) {
break;
}

for ( const val of { x: 1, y: 2 } ) {
break;
}

for ( const val of { x: 1, y: 2 } ) {
console.log( 'effect' );
break;
Expand Down
3 changes: 3 additions & 0 deletions test/function/samples/preserve-for-of-iterable/_config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
module.exports = {
description: 'preserves for...of loops'
};
20 changes: 20 additions & 0 deletions test/function/samples/preserve-for-of-iterable/iterables.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
export let dirty;

export const zeroToFive = {
[Symbol.iterator]() {
return {
current: 0,
last: 5,
next() {
const ret = this.current < this.last
? { done: false, value: this.current++ }
: { done: true };

// assert later
dirty = this.current;

return ret;
}
};
}
};
19 changes: 19 additions & 0 deletions test/function/samples/preserve-for-of-iterable/loops.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
export const awaitable = async (iterable) => {
for await (const value of iterable) {
}
}

// This is equivalent to the above 'awaitable' function.
export const equivalent = async (iterable) => {
const iterator = iterable[Symbol.asyncIterator]()
let { done } = await iterator.next()
while (!done) {
({ done } = await iterator.next())
}

}

export const iterate = iterable => {
for (const value of iterable) {
}
}
6 changes: 6 additions & 0 deletions test/function/samples/preserve-for-of-iterable/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { zeroToFive, dirty } from './iterables';
import { iterate } from './loops';

assert.equal(dirty, undefined);
iterate(zeroToFive);
assert.equal(dirty, 5);