From a92d09df294fa42cd7310759c0ef4d4b2e7f3c52 Mon Sep 17 00:00:00 2001 From: Matias Lopez Date: Thu, 26 Sep 2019 16:37:37 +0100 Subject: [PATCH] Preserve empty for...of loops (#3132) * 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 --- src/ast/nodes/ForOfStatement.ts | 12 +++-------- .../main.js | 17 ---------------- .../samples/effect-in-for-of-loop/main.js | 14 ------------- .../samples/empty-for-of-statement/_config.js | 3 --- .../empty-for-of-statement/_expected.js | 2 -- .../samples/empty-for-of-statement/main.js | 5 ----- test/form/samples/for-of-scopes/_expected.js | 3 +++ .../side-effects-break-statements/main.js | 4 ---- .../preserve-for-of-iterable/_config.js | 3 +++ .../preserve-for-of-iterable/iterables.js | 20 +++++++++++++++++++ .../samples/preserve-for-of-iterable/loops.js | 19 ++++++++++++++++++ .../samples/preserve-for-of-iterable/main.js | 6 ++++++ 12 files changed, 54 insertions(+), 54 deletions(-) delete mode 100644 test/form/samples/empty-for-of-statement/_config.js delete mode 100644 test/form/samples/empty-for-of-statement/_expected.js delete mode 100644 test/form/samples/empty-for-of-statement/main.js create mode 100644 test/function/samples/preserve-for-of-iterable/_config.js create mode 100644 test/function/samples/preserve-for-of-iterable/iterables.js create mode 100644 test/function/samples/preserve-for-of-iterable/loops.js create mode 100644 test/function/samples/preserve-for-of-iterable/main.js diff --git a/src/ast/nodes/ForOfStatement.ts b/src/ast/nodes/ForOfStatement.ts index 587e3c7c537..23874656dc7 100644 --- a/src/ast/nodes/ForOfStatement.ts +++ b/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'; @@ -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) { diff --git a/test/form/samples/effect-in-for-of-loop-in-functions/main.js b/test/form/samples/effect-in-for-of-loop-in-functions/main.js index 2db042360d8..5b7bb91e0c7 100644 --- a/test/form/samples/effect-in-for-of-loop-in-functions/main.js +++ b/test/form/samples/effect-in-for-of-loop-in-functions/main.js @@ -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 ) { @@ -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' }, diff --git a/test/form/samples/effect-in-for-of-loop/main.js b/test/form/samples/effect-in-for-of-loop/main.js index 851682f5491..eccc0b7a9b3 100644 --- a/test/form/samples/effect-in-for-of-loop/main.js +++ b/test/form/samples/effect-in-for-of-loop/main.js @@ -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' }, diff --git a/test/form/samples/empty-for-of-statement/_config.js b/test/form/samples/empty-for-of-statement/_config.js deleted file mode 100644 index 4a7bd91179f..00000000000 --- a/test/form/samples/empty-for-of-statement/_config.js +++ /dev/null @@ -1,3 +0,0 @@ -module.exports = { - description: 'removes an empty for-of statement' -}; diff --git a/test/form/samples/empty-for-of-statement/_expected.js b/test/form/samples/empty-for-of-statement/_expected.js deleted file mode 100644 index 472e544d88c..00000000000 --- a/test/form/samples/empty-for-of-statement/_expected.js +++ /dev/null @@ -1,2 +0,0 @@ -console.log( 1 ); -console.log( 2 ); diff --git a/test/form/samples/empty-for-of-statement/main.js b/test/form/samples/empty-for-of-statement/main.js deleted file mode 100644 index 90cdbd9806c..00000000000 --- a/test/form/samples/empty-for-of-statement/main.js +++ /dev/null @@ -1,5 +0,0 @@ -console.log( 1 ); -for ( const i of globalThis.unknown ) { - // do nothing -} -console.log( 2 ); diff --git a/test/form/samples/for-of-scopes/_expected.js b/test/form/samples/for-of-scopes/_expected.js index 7a3d255e13a..1c887974ff3 100644 --- a/test/form/samples/for-of-scopes/_expected.js +++ b/test/form/samples/for-of-scopes/_expected.js @@ -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! diff --git a/test/form/samples/side-effects-break-statements/main.js b/test/form/samples/side-effects-break-statements/main.js index 528bc9b34c7..fadaa8aacf4 100644 --- a/test/form/samples/side-effects-break-statements/main.js +++ b/test/form/samples/side-effects-break-statements/main.js @@ -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; diff --git a/test/function/samples/preserve-for-of-iterable/_config.js b/test/function/samples/preserve-for-of-iterable/_config.js new file mode 100644 index 00000000000..d2105b0c09a --- /dev/null +++ b/test/function/samples/preserve-for-of-iterable/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'preserves for...of loops' +}; diff --git a/test/function/samples/preserve-for-of-iterable/iterables.js b/test/function/samples/preserve-for-of-iterable/iterables.js new file mode 100644 index 00000000000..a2afb6cd272 --- /dev/null +++ b/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; + } + }; + } +}; diff --git a/test/function/samples/preserve-for-of-iterable/loops.js b/test/function/samples/preserve-for-of-iterable/loops.js new file mode 100644 index 00000000000..58031387846 --- /dev/null +++ b/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) { + } +} diff --git a/test/function/samples/preserve-for-of-iterable/main.js b/test/function/samples/preserve-for-of-iterable/main.js new file mode 100644 index 00000000000..8ed88329764 --- /dev/null +++ b/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);