diff --git a/src/ast/nodes/CallExpression.js b/src/ast/nodes/CallExpression.js index 720e866..d275100 100644 --- a/src/ast/nodes/CallExpression.js +++ b/src/ast/nodes/CallExpression.js @@ -27,7 +27,7 @@ export default class CallExpression extends Node { } hasEffects ( scope ) { - return callHasEffects( scope, this.callee ); + return callHasEffects( scope, this.callee, false ); } initialise ( scope ) { diff --git a/src/ast/nodes/NewExpression.js b/src/ast/nodes/NewExpression.js index 8bfbb33..d8fc3b1 100644 --- a/src/ast/nodes/NewExpression.js +++ b/src/ast/nodes/NewExpression.js @@ -3,6 +3,6 @@ import callHasEffects from './shared/callHasEffects.js'; export default class NewExpression extends Node { hasEffects ( scope ) { - return callHasEffects( scope, this.callee ); + return callHasEffects( scope, this.callee, true ); } } diff --git a/src/ast/nodes/shared/callHasEffects.js b/src/ast/nodes/shared/callHasEffects.js index 6df76d1..9660b8a 100644 --- a/src/ast/nodes/shared/callHasEffects.js +++ b/src/ast/nodes/shared/callHasEffects.js @@ -5,7 +5,43 @@ import { UNKNOWN } from '../../values.js'; const currentlyCalling = new Set(); -function fnHasEffects ( fn ) { +function isES5Function ( node ) { + return node.type === 'FunctionExpression' || node.type === 'FunctionDeclaration'; +} + +function hasEffectsNew ( node, scope ) { + let inner = node; + + if ( inner.type === 'ExpressionStatement' ) { + inner = inner.expression; + + if ( inner.type === 'AssignmentExpression' ) { + if ( inner.right.hasEffects( scope ) ) { + return true; + + } else { + inner = inner.left; + + if ( inner.type === 'MemberExpression' ) { + if ( inner.computed && inner.property.hasEffects( scope ) ) { + return true; + + } else { + inner = inner.object; + + if ( inner.type === 'ThisExpression' ) { + return false; + } + } + } + } + } + } + + return node.hasEffects( scope ); +} + +function fnHasEffects ( fn, isNew ) { if ( currentlyCalling.has( fn ) ) return false; // prevent infinite loops... TODO there must be a better way currentlyCalling.add( fn ); @@ -14,7 +50,7 @@ function fnHasEffects ( fn ) { const body = fn.body.type === 'BlockStatement' ? fn.body.body : [ fn.body ]; for ( const node of body ) { - if ( node.hasEffects( scope ) ) { + if ( isNew ? hasEffectsNew( node, scope ) : node.hasEffects( scope ) ) { currentlyCalling.delete( fn ); return true; } @@ -24,14 +60,14 @@ function fnHasEffects ( fn ) { return false; } -export default function callHasEffects ( scope, callee ) { +export default function callHasEffects ( scope, callee, isNew ) { const values = new Set([ callee ]); for ( const node of values ) { if ( node === UNKNOWN ) return true; // err on side of caution if ( /Function/.test( node.type ) ) { - if ( fnHasEffects( node ) ) return true; + if ( fnHasEffects( node, isNew && isES5Function( node ) ) ) return true; } else if ( isReference( node ) ) { diff --git a/test/form/side-effect-es5-classes/_config.js b/test/form/side-effect-es5-classes/_config.js new file mode 100644 index 0000000..380a44e --- /dev/null +++ b/test/form/side-effect-es5-classes/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'omits ES5 classes which are pure (e.g. they only assign to `this`)' +}; diff --git a/test/form/side-effect-es5-classes/_expected/amd.js b/test/form/side-effect-es5-classes/_expected/amd.js new file mode 100644 index 0000000..b996fb7 --- /dev/null +++ b/test/form/side-effect-es5-classes/_expected/amd.js @@ -0,0 +1,34 @@ +define(function () { 'use strict'; + + function Bar ( x ) { + console.log( 'side effect' ); + this.value = x; + } + + function Baz ( x ) { + this[ console.log( 'side effect' ) ] = x; + } + + function Qux ( x ) { + this.value = console.log( 'side effect' ); + } + + function Corge ( x ) { + this.value = x; + } + + var Arrow = ( x ) => { + undefined.value = x; + }; + + console.log( 'before' ); + + var bar = new Bar(5); + var baz = new Baz(5); + var qux = new Qux(5); + var corge = Corge(5); + var arrow = new Arrow(5); + + console.log( 'after' ); + +}); diff --git a/test/form/side-effect-es5-classes/_expected/cjs.js b/test/form/side-effect-es5-classes/_expected/cjs.js new file mode 100644 index 0000000..987153a --- /dev/null +++ b/test/form/side-effect-es5-classes/_expected/cjs.js @@ -0,0 +1,32 @@ +'use strict'; + +function Bar ( x ) { + console.log( 'side effect' ); + this.value = x; +} + +function Baz ( x ) { + this[ console.log( 'side effect' ) ] = x; +} + +function Qux ( x ) { + this.value = console.log( 'side effect' ); +} + +function Corge ( x ) { + this.value = x; +} + +var Arrow = ( x ) => { + undefined.value = x; +}; + +console.log( 'before' ); + +var bar = new Bar(5); +var baz = new Baz(5); +var qux = new Qux(5); +var corge = Corge(5); +var arrow = new Arrow(5); + +console.log( 'after' ); diff --git a/test/form/side-effect-es5-classes/_expected/es.js b/test/form/side-effect-es5-classes/_expected/es.js new file mode 100644 index 0000000..529afb8 --- /dev/null +++ b/test/form/side-effect-es5-classes/_expected/es.js @@ -0,0 +1,30 @@ +function Bar ( x ) { + console.log( 'side effect' ); + this.value = x; +} + +function Baz ( x ) { + this[ console.log( 'side effect' ) ] = x; +} + +function Qux ( x ) { + this.value = console.log( 'side effect' ); +} + +function Corge ( x ) { + this.value = x; +} + +var Arrow = ( x ) => { + undefined.value = x; +}; + +console.log( 'before' ); + +var bar = new Bar(5); +var baz = new Baz(5); +var qux = new Qux(5); +var corge = Corge(5); +var arrow = new Arrow(5); + +console.log( 'after' ); diff --git a/test/form/side-effect-es5-classes/_expected/iife.js b/test/form/side-effect-es5-classes/_expected/iife.js new file mode 100644 index 0000000..daff1c0 --- /dev/null +++ b/test/form/side-effect-es5-classes/_expected/iife.js @@ -0,0 +1,35 @@ +(function () { + 'use strict'; + + function Bar ( x ) { + console.log( 'side effect' ); + this.value = x; + } + + function Baz ( x ) { + this[ console.log( 'side effect' ) ] = x; + } + + function Qux ( x ) { + this.value = console.log( 'side effect' ); + } + + function Corge ( x ) { + this.value = x; + } + + var Arrow = ( x ) => { + undefined.value = x; + }; + + console.log( 'before' ); + + var bar = new Bar(5); + var baz = new Baz(5); + var qux = new Qux(5); + var corge = Corge(5); + var arrow = new Arrow(5); + + console.log( 'after' ); + +}()); diff --git a/test/form/side-effect-es5-classes/_expected/umd.js b/test/form/side-effect-es5-classes/_expected/umd.js new file mode 100644 index 0000000..36c8dc5 --- /dev/null +++ b/test/form/side-effect-es5-classes/_expected/umd.js @@ -0,0 +1,38 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory() : + typeof define === 'function' && define.amd ? define(factory) : + (factory()); +}(this, (function () { 'use strict'; + + function Bar ( x ) { + console.log( 'side effect' ); + this.value = x; + } + + function Baz ( x ) { + this[ console.log( 'side effect' ) ] = x; + } + + function Qux ( x ) { + this.value = console.log( 'side effect' ); + } + + function Corge ( x ) { + this.value = x; + } + + var Arrow = ( x ) => { + undefined.value = x; + }; + + console.log( 'before' ); + + var bar = new Bar(5); + var baz = new Baz(5); + var qux = new Qux(5); + var corge = Corge(5); + var arrow = new Arrow(5); + + console.log( 'after' ); + +}))); diff --git a/test/form/side-effect-es5-classes/arrow.js b/test/form/side-effect-es5-classes/arrow.js new file mode 100644 index 0000000..24128a5 --- /dev/null +++ b/test/form/side-effect-es5-classes/arrow.js @@ -0,0 +1,3 @@ +export var Arrow = ( x ) => { + this.value = x; +}; diff --git a/test/form/side-effect-es5-classes/bar.js b/test/form/side-effect-es5-classes/bar.js new file mode 100644 index 0000000..021ce4d --- /dev/null +++ b/test/form/side-effect-es5-classes/bar.js @@ -0,0 +1,4 @@ +export function Bar ( x ) { + console.log( 'side effect' ); + this.value = x; +} diff --git a/test/form/side-effect-es5-classes/baz.js b/test/form/side-effect-es5-classes/baz.js new file mode 100644 index 0000000..2c34610 --- /dev/null +++ b/test/form/side-effect-es5-classes/baz.js @@ -0,0 +1,3 @@ +export function Baz ( x ) { + this[ console.log( 'side effect' ) ] = x; +} diff --git a/test/form/side-effect-es5-classes/corge.js b/test/form/side-effect-es5-classes/corge.js new file mode 100644 index 0000000..1e94c31 --- /dev/null +++ b/test/form/side-effect-es5-classes/corge.js @@ -0,0 +1,3 @@ +export function Corge ( x ) { + this.value = x; +} diff --git a/test/form/side-effect-es5-classes/foo.js b/test/form/side-effect-es5-classes/foo.js new file mode 100644 index 0000000..b5e4788 --- /dev/null +++ b/test/form/side-effect-es5-classes/foo.js @@ -0,0 +1,4 @@ +export function Foo ( x ) { + this.value = x; + this["string property"] = 20; +} diff --git a/test/form/side-effect-es5-classes/main.js b/test/form/side-effect-es5-classes/main.js new file mode 100644 index 0000000..9dc4f0e --- /dev/null +++ b/test/form/side-effect-es5-classes/main.js @@ -0,0 +1,17 @@ +import { Foo } from './foo'; +import { Bar } from './bar'; +import { Baz } from './baz'; +import { Qux } from './qux'; +import { Corge } from './corge'; +import { Arrow } from './arrow'; + +console.log( 'before' ); + +var foo = new Foo(5); +var bar = new Bar(5); +var baz = new Baz(5); +var qux = new Qux(5); +var corge = Corge(5); +var arrow = new Arrow(5); + +console.log( 'after' ); diff --git a/test/form/side-effect-es5-classes/qux.js b/test/form/side-effect-es5-classes/qux.js new file mode 100644 index 0000000..52ce884 --- /dev/null +++ b/test/form/side-effect-es5-classes/qux.js @@ -0,0 +1,3 @@ +export function Qux ( x ) { + this.value = console.log( 'side effect' ); +} diff --git a/test/form/unmodified-default-exports/_expected/amd.js b/test/form/unmodified-default-exports/_expected/amd.js index 2ddbff5..87bc874 100644 --- a/test/form/unmodified-default-exports/_expected/amd.js +++ b/test/form/unmodified-default-exports/_expected/amd.js @@ -1,6 +1,7 @@ define(function () { 'use strict'; var Foo = function () { + console.log( 'side effect' ); this.isFoo = true; }; diff --git a/test/form/unmodified-default-exports/_expected/cjs.js b/test/form/unmodified-default-exports/_expected/cjs.js index 0ab59dc..5a2b3a5 100644 --- a/test/form/unmodified-default-exports/_expected/cjs.js +++ b/test/form/unmodified-default-exports/_expected/cjs.js @@ -1,6 +1,7 @@ 'use strict'; var Foo = function () { + console.log( 'side effect' ); this.isFoo = true; }; diff --git a/test/form/unmodified-default-exports/_expected/es.js b/test/form/unmodified-default-exports/_expected/es.js index a298bb9..d1fa1b3 100644 --- a/test/form/unmodified-default-exports/_expected/es.js +++ b/test/form/unmodified-default-exports/_expected/es.js @@ -1,4 +1,5 @@ var Foo = function () { + console.log( 'side effect' ); this.isFoo = true; }; diff --git a/test/form/unmodified-default-exports/_expected/iife.js b/test/form/unmodified-default-exports/_expected/iife.js index c07ff1a..ccf59c6 100644 --- a/test/form/unmodified-default-exports/_expected/iife.js +++ b/test/form/unmodified-default-exports/_expected/iife.js @@ -2,6 +2,7 @@ 'use strict'; var Foo = function () { + console.log( 'side effect' ); this.isFoo = true; }; diff --git a/test/form/unmodified-default-exports/_expected/umd.js b/test/form/unmodified-default-exports/_expected/umd.js index f74ed8e..e16d656 100644 --- a/test/form/unmodified-default-exports/_expected/umd.js +++ b/test/form/unmodified-default-exports/_expected/umd.js @@ -5,6 +5,7 @@ }(this, (function () { 'use strict'; var Foo = function () { + console.log( 'side effect' ); this.isFoo = true; }; @@ -16,4 +17,4 @@ var foo = new Foo(); -}))); \ No newline at end of file +}))); diff --git a/test/form/unmodified-default-exports/foo.js b/test/form/unmodified-default-exports/foo.js index 08ca0b2..f58be3b 100644 --- a/test/form/unmodified-default-exports/foo.js +++ b/test/form/unmodified-default-exports/foo.js @@ -1,4 +1,5 @@ var Foo = function () { + console.log( 'side effect' ); this.isFoo = true; }; diff --git a/test/function/es5-class-called-without-new/_config.js b/test/function/es5-class-called-without-new/_config.js new file mode 100644 index 0000000..7be35b6 --- /dev/null +++ b/test/function/es5-class-called-without-new/_config.js @@ -0,0 +1,2 @@ +module.exports = { +}; diff --git a/test/function/es5-class-called-without-new/foo.js b/test/function/es5-class-called-without-new/foo.js new file mode 100644 index 0000000..dd0a117 --- /dev/null +++ b/test/function/es5-class-called-without-new/foo.js @@ -0,0 +1,7 @@ +export function Foo ( x ) { + this.value = x; +} + +export function Bar ( x ) { + this.value = x; +} diff --git a/test/function/es5-class-called-without-new/main.js b/test/function/es5-class-called-without-new/main.js new file mode 100644 index 0000000..419b804 --- /dev/null +++ b/test/function/es5-class-called-without-new/main.js @@ -0,0 +1,7 @@ +import { Foo, Bar } from './foo'; + +assert.equal( new Foo(5).value, 5 ); + +assert.throws( function () { + Bar(5); +}, /^TypeError: Cannot set property 'value' of undefined$/ );