attach scopes to BlockStatement nodes, not Function nodes. fixes #91

src/Statement.js

@@ -49,29 +49,23 @@ export default class Statement {
 				let newScope;
 
 				switch ( node.type ) {
-					case 'FunctionExpression':
 					case 'FunctionDeclaration':
-					case 'ArrowFunctionExpression':
-						if ( node.type === 'FunctionDeclaration' ) {
 						scope.addDeclaration( node.id.name, node, false );
-						}
 
+					case 'BlockStatement':
+						if ( parent && /Function/.test( parent.type ) ) {
 							newScope = new Scope({
 								parent: scope,
-							params: node.params, // TODO rest params?
-							block: false
+								block: false,
+								params: parent.params
 							});
 
 							// named function expressions - the name is considered
 							// part of the function's scope
-						if ( node.type === 'FunctionExpression' && node.id ) {
-							newScope.addDeclaration( node.id.name, node, false );
+							if ( parent.type === 'FunctionExpression' && parent.id ) {
+								newScope.addDeclaration( parent.id.name, parent, false );
 							}
-
-						break;
-
-					case 'BlockStatement':
-						if ( !/Function/.test( parent.type ) ) {
+						} else {
 							newScope = new Scope({
 								parent: scope,
 								block: true
@@ -136,27 +130,19 @@ export default class Statement {
 		if ( !this.isImportDeclaration ) {
 			walk( this.node, {
 				enter: ( node, parent ) => {
-					if ( node._scope ) {
-						if ( !scope.isBlockScope ) {
-							if ( !isIife( node, parent ) ) readDepth += 1;
 					if ( isFunctionDeclaration( node, parent ) ) writeDepth += 1;
-						}
+					if ( /Function/.test( node.type ) && !isIife( node, parent ) ) readDepth += 1;
 
-						scope = node._scope;
-					}
+					if ( node._scope ) scope = node._scope;
 
 					this.checkForReads( scope, node, parent, !readDepth );
 					this.checkForWrites( scope, node, writeDepth );
 				},
 				leave: ( node, parent ) => {
-					if ( node._scope ) {
-						if ( !scope.isBlockScope ) {
-							if ( !isIife( node, parent ) ) readDepth -= 1;
 					if ( isFunctionDeclaration( node, parent ) ) writeDepth -= 1;
-						}
+					if ( /Function/.test( node.type ) && !isIife( node, parent ) ) readDepth -= 1;
 
-						scope = scope.parent;
-					}
+					if ( node._scope ) scope = scope.parent;
 				}
 			});
 		}
@@ -364,11 +350,6 @@ export default class Statement {
 					let newNames = blank();
 					let hasReplacements;
 
-					// special case = function foo ( foo ) {...}
-					if ( node.id && names[ node.id.name ] && scope.declarations[ node.id.name ] ) {
-						magicString.overwrite( node.id.start, node.id.end, names[ node.id.name ] );
-					}
-
 					keys( names ).forEach( name => {
 						if ( !scope.declarations[ name ] ) {
 							newNames[ name ] = names[ name ];
@@ -409,6 +390,8 @@ export default class Statement {
 				if ( parent.type === 'MemberExpression' && !parent.computed && node !== parent.object ) return;
 				if ( parent.type === 'Property' && node !== parent.value ) return;
 				if ( parent.type === 'MethodDefinition' && node === parent.key ) return;
+				if ( parent.type === 'FunctionExpression' ) return;
+				if ( /Function/.test( parent.type ) && ~parent.params.indexOf( node ) ) return;
 				// TODO others...?
 
 				// all other identifiers should be overwritten

test/function/functions-renamed-correctly/_config.js

@@ -1,5 +1,3 @@
 module.exports = {
-	description: 'renames function expression IDs correctly',
-	show: true,
-	solo: true
+	description: 'renames function expression IDs correctly'
 };

test/function/functions-renamed-correctly/after.js

@@ -1,5 +1,5 @@
 function x () {
-	console.log( 'after' );
+	return 'after';
 }
 
 export { x as after };

test/function/functions-renamed-correctly/before.js

@@ -1,5 +1,5 @@
 function x () {
-	console.log( 'before' );
+	return 'before';
 }
 
 export { x as before };

test/function/functions-renamed-correctly/main.js

@@ -2,6 +2,6 @@ import { before } from './before';
 import { x } from './factorial';
 import { after } from './after';
 
-before();
+before(); // before and after ensure x is renamed
 assert.equal( x( 5 ), 120 );
 after();