diff --git a/src/Statement.js b/src/Statement.js index d4117e1..b68d216 100644 --- a/src/Statement.js +++ b/src/Statement.js @@ -8,6 +8,14 @@ function isIife ( node, parent ) { return parent && parent.type === 'CallExpression' && node === parent.callee; } +function isFunctionDeclaration ( node, parent ) { + // `function foo () {}` + if ( node.type === 'FunctionDeclaration' ) return true; + + // `var foo = function () {}` - same thing for present purposes + if ( node.type === 'FunctionExpression' && parent.type === 'VariableDeclarator' ) return true; +} + export default class Statement { constructor ( node, module, start, end ) { this.node = node; @@ -117,22 +125,34 @@ export default class Statement { // calledImmediately(); // // ...but it's better than nothing - let depth = 0; + let readDepth = 0; + + // This allows us to track whether a modifying statement (i.e. assignment + // /update expressions) need to be captured + let writeDepth = 0; if ( !this.isImportDeclaration ) { walk( this.node, { enter: ( node, parent ) => { if ( node._scope ) { - if ( !scope.isBlockScope && !isIife( node, parent ) ) depth += 1; + if ( !scope.isBlockScope ) { + if ( !isIife( node, parent ) ) readDepth += 1; + if ( isFunctionDeclaration( node, parent ) ) writeDepth += 1; + } + scope = node._scope; } - this.checkForReads( scope, node, parent, !depth ); - this.checkForWrites( scope, node ); + this.checkForReads( scope, node, parent, !readDepth ); + this.checkForWrites( scope, node, writeDepth ); }, leave: ( node, parent ) => { if ( node._scope ) { - if ( !scope.isBlockScope && !isIife( node, parent ) ) depth -= 1; + if ( !scope.isBlockScope ) { + if ( !isIife( node, parent ) ) readDepth -= 1; + if ( isFunctionDeclaration( node, parent ) ) writeDepth -= 1; + } + scope = scope.parent; } } @@ -171,7 +191,7 @@ export default class Statement { } } - checkForWrites ( scope, node ) { + checkForWrites ( scope, node, writeDepth ) { const addNode = ( node, isAssignment ) => { let depth = 0; // determine whether we're illegally modifying a binding or namespace @@ -209,7 +229,13 @@ export default class Statement { } } - if ( node.type === 'Identifier' ) { + // we only care about writes that happen a) at the top level, + // or b) inside a function that could be immediately invoked. + // Writes inside named functions are only relevant if the + // function is called, in which case we don't need to do + // anything (but we still need to call checkForWrites to + // catch illegal reassignments to imported bindings) + if ( writeDepth === 0 && node.type === 'Identifier' ) { this.modifies[ node.name ] = true; } }; diff --git a/test/form/exclude-unnecessary-modifications/_config.js b/test/form/exclude-unnecessary-modifications/_config.js new file mode 100644 index 0000000..2b3de2b --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'statements that modify definitions within unused functions are excluded' +}; diff --git a/test/form/exclude-unnecessary-modifications/_expected/amd.js b/test/form/exclude-unnecessary-modifications/_expected/amd.js new file mode 100644 index 0000000..84befa4 --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/amd.js @@ -0,0 +1,28 @@ +define(function () { 'use strict'; + + var foo = {}; + + mutate1( foo ); + + // should be included + [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; + }); + + [ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; + }); + + function mutate1 () { + foo.mutated = 1; + } + + ({ + mutate2: function () { + foo.mutated = 2; + } + }).mutate2(); + + console.log( foo ); + +}); diff --git a/test/form/exclude-unnecessary-modifications/_expected/cjs.js b/test/form/exclude-unnecessary-modifications/_expected/cjs.js new file mode 100644 index 0000000..596c910 --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/cjs.js @@ -0,0 +1,26 @@ +'use strict'; + +var foo = {}; + +mutate1( foo ); + +// should be included +[ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; +}); + +[ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; +}); + +function mutate1 () { + foo.mutated = 1; +} + +({ + mutate2: function () { + foo.mutated = 2; + } +}).mutate2(); + +console.log( foo ); diff --git a/test/form/exclude-unnecessary-modifications/_expected/es6.js b/test/form/exclude-unnecessary-modifications/_expected/es6.js new file mode 100644 index 0000000..a88d21e --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/es6.js @@ -0,0 +1,24 @@ +var foo = {}; + +mutate1( foo ); + +// should be included +[ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; +}); + +[ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; +}); + +function mutate1 () { + foo.mutated = 1; +} + +({ + mutate2: function () { + foo.mutated = 2; + } +}).mutate2(); + +console.log( foo ); diff --git a/test/form/exclude-unnecessary-modifications/_expected/iife.js b/test/form/exclude-unnecessary-modifications/_expected/iife.js new file mode 100644 index 0000000..0fd4c40 --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/iife.js @@ -0,0 +1,28 @@ +(function () { 'use strict'; + + var foo = {}; + + mutate1( foo ); + + // should be included + [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; + }); + + [ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; + }); + + function mutate1 () { + foo.mutated = 1; + } + + ({ + mutate2: function () { + foo.mutated = 2; + } + }).mutate2(); + + console.log( foo ); + +})(); diff --git a/test/form/exclude-unnecessary-modifications/_expected/umd.js b/test/form/exclude-unnecessary-modifications/_expected/umd.js new file mode 100644 index 0000000..1c2bb78 --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/_expected/umd.js @@ -0,0 +1,32 @@ +(function (global, factory) { + typeof exports === 'object' && typeof module !== 'undefined' ? factory() : + typeof define === 'function' && define.amd ? define(factory) : + factory(); +}(this, function () { 'use strict'; + + var foo = {}; + + mutate1( foo ); + + // should be included + [ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; + }); + + [ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; + }); + + function mutate1 () { + foo.mutated = 1; + } + + ({ + mutate2: function () { + foo.mutated = 2; + } + }).mutate2(); + + console.log( foo ); + +})); diff --git a/test/form/exclude-unnecessary-modifications/foo.js b/test/form/exclude-unnecessary-modifications/foo.js new file mode 100644 index 0000000..2280631 --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/foo.js @@ -0,0 +1,37 @@ +var foo = {}; + +mutate1( foo ); + +// should be included +[ 'a', 'b', 'c' ].forEach( function ( letter, i ) { + foo[ letter ] = i; +}); + +[ 'd', 'e', 'f' ].forEach( ( letter, i ) => { + foo[ letter ] = i; +}); + +function mutate1 () { + foo.mutated = 1; +} + +({ + mutate2: function () { + foo.mutated = 2; + } +}).mutate2(); + +// should be excluded +var mutate2 = function () { + foo.mutated = 2; +} + +function unused1 () { + foo.wat = 'nope'; +} + +function unused2 () { + mutate1( foo ); +} + +export default foo; diff --git a/test/form/exclude-unnecessary-modifications/main.js b/test/form/exclude-unnecessary-modifications/main.js new file mode 100644 index 0000000..03d6edf --- /dev/null +++ b/test/form/exclude-unnecessary-modifications/main.js @@ -0,0 +1,2 @@ +import foo from './foo'; +console.log( foo );