Skip to content

Commit

Permalink
Preserve empty for...of loops (#3132)
Browse files Browse the repository at this point in the history
* Add test based on #3126

* Placeholder fix

* comment out options

* remove solo

* get rid of comments

* leap of faith on export let;

* "Fix" two for...of tests

* fix for-of-scpes "fix"

* fix effect-in-for-of-loop test

* fix for effect-in-for-of-loop-in-functions
  • Loading branch information
imatlopez authored and lukastaegert committed Sep 26, 2019
1 parent bc5b98d commit a92d09d
Show file tree
Hide file tree
Showing 12 changed files with 54 additions and 54 deletions.
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);

0 comments on commit a92d09d

Please sign in to comment.