From d249c7eb34a8cbe20fff21108ac3db73af2301c9 Mon Sep 17 00:00:00 2001 From: Rich-Harris Date: Tue, 27 Dec 2016 16:37:01 -0500 Subject: [PATCH] preserve var declarations in dead branches --- package.json | 1 - src/ast/nodes/IfStatement.js | 43 +++++++++++++++++++ .../_config.js | 3 ++ .../main.js | 5 +++ 4 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/function/preserves-var-declarations-in-dead-branches/_config.js create mode 100644 test/function/preserves-var-declarations-in-dead-branches/main.js diff --git a/package.json b/package.json index 71d71ba..1b90828 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,6 @@ "console-group": "^0.3.1", "eslint": "^2.13.1", "eslint-plugin-import": "^2.2.0", - "estree-walker": "^0.2.1", "is-reference": "^1.0.0", "istanbul": "^0.4.3", "magic-string": "^0.15.2", diff --git a/src/ast/nodes/IfStatement.js b/src/ast/nodes/IfStatement.js index fe83a78..87c8792 100644 --- a/src/ast/nodes/IfStatement.js +++ b/src/ast/nodes/IfStatement.js @@ -1,4 +1,5 @@ import Statement from './shared/Statement.js'; +import extractNames from '../utils/extractNames.js'; import { UNKNOWN } from '../values.js'; // Statement types which may contain if-statements as direct children. @@ -11,9 +12,34 @@ const statementsWithIfStatements = new Set([ 'WhileStatement' ]); +function handleVarDeclarations ( node, scope ) { + const hoistedVars = []; + + function visit ( node ) { + if ( node.type === 'VariableDeclaration' && node.kind === 'var' ) { + node.initialise( scope ); + + node.declarations.forEach( declarator => { + extractNames( declarator.id ).forEach( name => { + if ( !~hoistedVars.indexOf( name ) ) hoistedVars.push( name ); + }); + }); + } + + else if ( !/Function/.test( node.type ) ) { + node.eachChild( visit ); + } + } + + visit( node ); + + return hoistedVars; +} + // TODO DRY this out export default class IfStatement extends Statement { initialise ( scope ) { + this.scope = scope; this.testValue = this.test.getValue(); if ( this.module.bundle.treeshake ) { @@ -23,11 +49,15 @@ export default class IfStatement extends Statement { else if ( this.testValue ) { this.consequent.initialise( scope ); + + if ( this.alternate ) this.hoistedVars = handleVarDeclarations( this.alternate, scope ); this.alternate = null; } else { if ( this.alternate ) this.alternate.initialise( scope ); + + this.hoistedVars = handleVarDeclarations( this.consequent, scope ); this.consequent = null; } } @@ -49,6 +79,19 @@ export default class IfStatement extends Statement { // TODO if no block-scoped declarations, remove enclosing // curlies and dedent block (if there is a block) + if ( this.hoistedVars ) { + const names = this.hoistedVars + .map( name => { + const declaration = this.scope.findDeclaration( name ); + return declaration.activated ? declaration.getName() : null; + }) + .filter( Boolean ); + + if ( names.length > 0 ) { + code.insertLeft( this.start, `var ${names.join( ', ' )};\n\n` ); + } + } + if ( this.testValue ) { code.remove( this.start, this.consequent.start ); code.remove( this.consequent.end, this.end ); diff --git a/test/function/preserves-var-declarations-in-dead-branches/_config.js b/test/function/preserves-var-declarations-in-dead-branches/_config.js new file mode 100644 index 0000000..e0f687e --- /dev/null +++ b/test/function/preserves-var-declarations-in-dead-branches/_config.js @@ -0,0 +1,3 @@ +module.exports = { + description: 'preserves var declarations in dead branches (#977)' +}; diff --git a/test/function/preserves-var-declarations-in-dead-branches/main.js b/test/function/preserves-var-declarations-in-dead-branches/main.js new file mode 100644 index 0000000..523b8cc --- /dev/null +++ b/test/function/preserves-var-declarations-in-dead-branches/main.js @@ -0,0 +1,5 @@ +if ( false ) { + var foo; +} + +assert.equal( foo, undefined );