From cbf6bd6ffb7e428de3611dff81eb68bba9c49613 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Wed, 30 Dec 2015 20:10:12 -0500 Subject: [PATCH 1/2] failing test for #397 --- test/form/side-effect-m/_config.js | 4 ++++ test/form/side-effect-m/_expected/amd.js | 21 +++++++++++++++++++ test/form/side-effect-m/_expected/cjs.js | 19 +++++++++++++++++ test/form/side-effect-m/_expected/es6.js | 17 +++++++++++++++ test/form/side-effect-m/_expected/iife.js | 21 +++++++++++++++++++ test/form/side-effect-m/_expected/umd.js | 25 +++++++++++++++++++++++ test/form/side-effect-m/even.js | 11 ++++++++++ test/form/side-effect-m/main.js | 5 +++++ test/form/side-effect-m/odd.js | 5 +++++ 9 files changed, 128 insertions(+) create mode 100644 test/form/side-effect-m/_config.js create mode 100644 test/form/side-effect-m/_expected/amd.js create mode 100644 test/form/side-effect-m/_expected/cjs.js create mode 100644 test/form/side-effect-m/_expected/es6.js create mode 100644 test/form/side-effect-m/_expected/iife.js create mode 100644 test/form/side-effect-m/_expected/umd.js create mode 100644 test/form/side-effect-m/even.js create mode 100644 test/form/side-effect-m/main.js create mode 100644 test/form/side-effect-m/odd.js diff --git a/test/form/side-effect-m/_config.js b/test/form/side-effect-m/_config.js new file mode 100644 index 0000000..5fc4d8c --- /dev/null +++ b/test/form/side-effect-m/_config.js @@ -0,0 +1,4 @@ +module.exports = { + solo: true, + description: 'detects side-effects with cyclical dependencies' +}; diff --git a/test/form/side-effect-m/_expected/amd.js b/test/form/side-effect-m/_expected/amd.js new file mode 100644 index 0000000..763d466 --- /dev/null +++ b/test/form/side-effect-m/_expected/amd.js @@ -0,0 +1,21 @@ +define(function () { 'use strict'; + + function odd ( n ) { + return n !== 0 && even( n - 1 ); + } + + var counter = 0; + + // This should be in the output + export var foo = odd( 12 ); + + function even ( n ) { + alert( counter++ ) + return n === 0 || odd( n - 1 ); + } + + console.log( even( 5 ) ); + + console.log( counter ); + +}); diff --git a/test/form/side-effect-m/_expected/cjs.js b/test/form/side-effect-m/_expected/cjs.js new file mode 100644 index 0000000..8aeebd5 --- /dev/null +++ b/test/form/side-effect-m/_expected/cjs.js @@ -0,0 +1,19 @@ +'use strict'; + +function odd ( n ) { + return n !== 0 && even( n - 1 ); +} + +var counter = 0; + +// This should be in the output +export var foo = odd( 12 ); + +function even ( n ) { + alert( counter++ ) + return n === 0 || odd( n - 1 ); +} + +console.log( even( 5 ) ); + +console.log( counter ); diff --git a/test/form/side-effect-m/_expected/es6.js b/test/form/side-effect-m/_expected/es6.js new file mode 100644 index 0000000..f1bc219 --- /dev/null +++ b/test/form/side-effect-m/_expected/es6.js @@ -0,0 +1,17 @@ +function odd ( n ) { + return n !== 0 && even( n - 1 ); +} + +var counter = 0; + +// This should be in the output +export var foo = odd( 12 ); + +function even ( n ) { + alert( counter++ ) + return n === 0 || odd( n - 1 ); +} + +console.log( even( 5 ) ); + +console.log( counter ); diff --git a/test/form/side-effect-m/_expected/iife.js b/test/form/side-effect-m/_expected/iife.js new file mode 100644 index 0000000..b507ec9 --- /dev/null +++ b/test/form/side-effect-m/_expected/iife.js @@ -0,0 +1,21 @@ +(function () { 'use strict'; + + function odd ( n ) { + return n !== 0 && even( n - 1 ); + } + + var counter = 0; + + // This should be in the output + export var foo = odd( 12 ); + + function even ( n ) { + alert( counter++ ) + return n === 0 || odd( n - 1 ); + } + + console.log( even( 5 ) ); + + console.log( counter ); + +})(); diff --git a/test/form/side-effect-m/_expected/umd.js b/test/form/side-effect-m/_expected/umd.js new file mode 100644 index 0000000..45f0a07 --- /dev/null +++ b/test/form/side-effect-m/_expected/umd.js @@ -0,0 +1,25 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory() : + typeof define === 'function' && define.amd ? define(factory) : + (factory()); +}(this, function () { 'use strict'; + + function odd ( n ) { + return n !== 0 && even( n - 1 ); + } + + var counter = 0; + + // This should be in the output + export var foo = odd( 12 ); + + function even ( n ) { + alert( counter++ ) + return n === 0 || odd( n - 1 ); + } + + console.log( even( 5 ) ); + + console.log( counter ); + +})); diff --git a/test/form/side-effect-m/even.js b/test/form/side-effect-m/even.js new file mode 100644 index 0000000..5b16c01 --- /dev/null +++ b/test/form/side-effect-m/even.js @@ -0,0 +1,11 @@ +import { odd } from './odd.js' + +export var counter = 0; + +// This should be in the output +export var foo = odd( 12 ); + +export function even ( n ) { + alert( counter++ ) + return n === 0 || odd( n - 1 ); +} diff --git a/test/form/side-effect-m/main.js b/test/form/side-effect-m/main.js new file mode 100644 index 0000000..a7adf02 --- /dev/null +++ b/test/form/side-effect-m/main.js @@ -0,0 +1,5 @@ +import { counter, even } from './even.js'; + +console.log( even( 5 ) ); + +console.log( counter ); diff --git a/test/form/side-effect-m/odd.js b/test/form/side-effect-m/odd.js new file mode 100644 index 0000000..0293a7b --- /dev/null +++ b/test/form/side-effect-m/odd.js @@ -0,0 +1,5 @@ +import { even } from './even.js'; + +export function odd ( n ) { + return n !== 0 && even( n - 1 ); +} From 742cf9255c4d65280f751ca50812098a259ac898 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Wed, 30 Dec 2015 20:56:29 -0500 Subject: [PATCH 2/2] fix side-effect detection in cyclical function calls (#397) --- src/Declaration.js | 8 +++++++- src/Module.js | 2 +- test/form/side-effect-m/_config.js | 1 - test/form/side-effect-m/_expected/amd.js | 2 +- test/form/side-effect-m/_expected/cjs.js | 2 +- test/form/side-effect-m/_expected/es6.js | 2 +- test/form/side-effect-m/_expected/iife.js | 2 +- test/form/side-effect-m/_expected/umd.js | 2 +- 8 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/Declaration.js b/src/Declaration.js index f7076f0..ef10b7f 100644 --- a/src/Declaration.js +++ b/src/Declaration.js @@ -43,14 +43,20 @@ export default class Declaration { run ( strongDependencies ) { if ( this.tested ) return this.hasSideEffects; - this.tested = true; + if ( !this.functionNode ) { this.hasSideEffects = true; // err on the side of caution. TODO handle unambiguous `var x; x = y => z` cases } else { + if ( this.running ) return true; // short-circuit infinite loop + this.running = true; + this.hasSideEffects = run( this.functionNode.body, this.functionNode._scope, this.statement, strongDependencies, false ); + + this.running = false; } + this.tested = true; return this.hasSideEffects; } diff --git a/src/Module.js b/src/Module.js index be6117f..a63687b 100644 --- a/src/Module.js +++ b/src/Module.js @@ -570,7 +570,7 @@ export default class Module { let marked = false; this.statements.forEach( statement => { - marked = marked || statement.run( this.strongDependencies, safe ); + marked = statement.run( this.strongDependencies, safe ) || marked; }); return marked; diff --git a/test/form/side-effect-m/_config.js b/test/form/side-effect-m/_config.js index 5fc4d8c..1c3ca4e 100644 --- a/test/form/side-effect-m/_config.js +++ b/test/form/side-effect-m/_config.js @@ -1,4 +1,3 @@ module.exports = { - solo: true, description: 'detects side-effects with cyclical dependencies' }; diff --git a/test/form/side-effect-m/_expected/amd.js b/test/form/side-effect-m/_expected/amd.js index 763d466..8c7787a 100644 --- a/test/form/side-effect-m/_expected/amd.js +++ b/test/form/side-effect-m/_expected/amd.js @@ -7,7 +7,7 @@ define(function () { 'use strict'; var counter = 0; // This should be in the output - export var foo = odd( 12 ); + var foo = odd( 12 ); function even ( n ) { alert( counter++ ) diff --git a/test/form/side-effect-m/_expected/cjs.js b/test/form/side-effect-m/_expected/cjs.js index 8aeebd5..43eacf4 100644 --- a/test/form/side-effect-m/_expected/cjs.js +++ b/test/form/side-effect-m/_expected/cjs.js @@ -7,7 +7,7 @@ function odd ( n ) { var counter = 0; // This should be in the output -export var foo = odd( 12 ); +var foo = odd( 12 ); function even ( n ) { alert( counter++ ) diff --git a/test/form/side-effect-m/_expected/es6.js b/test/form/side-effect-m/_expected/es6.js index f1bc219..61bbbbf 100644 --- a/test/form/side-effect-m/_expected/es6.js +++ b/test/form/side-effect-m/_expected/es6.js @@ -5,7 +5,7 @@ function odd ( n ) { var counter = 0; // This should be in the output -export var foo = odd( 12 ); +var foo = odd( 12 ); function even ( n ) { alert( counter++ ) diff --git a/test/form/side-effect-m/_expected/iife.js b/test/form/side-effect-m/_expected/iife.js index b507ec9..6c27951 100644 --- a/test/form/side-effect-m/_expected/iife.js +++ b/test/form/side-effect-m/_expected/iife.js @@ -7,7 +7,7 @@ var counter = 0; // This should be in the output - export var foo = odd( 12 ); + var foo = odd( 12 ); function even ( n ) { alert( counter++ ) diff --git a/test/form/side-effect-m/_expected/umd.js b/test/form/side-effect-m/_expected/umd.js index 45f0a07..902026f 100644 --- a/test/form/side-effect-m/_expected/umd.js +++ b/test/form/side-effect-m/_expected/umd.js @@ -11,7 +11,7 @@ var counter = 0; // This should be in the output - export var foo = odd( 12 ); + var foo = odd( 12 ); function even ( n ) { alert( counter++ )