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
@@ -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
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
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
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
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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,6 @@
import { zeroToFive, dirty } from './iterables';
import { iterate } from './loops';

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