more reliable renaming

src/Bundle/index.js

@@ -5,6 +5,7 @@ import { hasOwnProp } from '../utils/object';
 import { sequence } from '../utils/promise';
 import Module from '../Module/index';
 import finalisers from '../finalisers/index';
+import replaceIdentifiers from '../utils/replaceIdentifiers';
 
 export default class Bundle {
 	constructor ( options ) {
@@ -78,6 +79,9 @@ export default class Bundle {
 		let magicString = new MagicString.Bundle();
 
 		this.body.forEach( statement => {
+			const module = statement._module;
+
+			replaceIdentifiers( statement, statement._source, module.nameReplacements );
 			magicString.addSource( statement._source );
 		});
 

src/Module/index.js

@@ -5,7 +5,6 @@ import MagicString from 'magic-string';
 import analyse from '../ast/analyse';
 import { hasOwnProp } from '../utils/object';
 import { sequence } from '../utils/promise';
-import replaceIdentifiers from '../utils/replaceIdentifiers';
 
 const emptyArrayPromise = Promise.resolve([]);
 
@@ -21,7 +20,7 @@ export default class Module {
 			sourceType: 'module'
 		});
 
-		analyse( this.ast, this.code );
+		analyse( this.ast, this.code, this );
 
 		this.nameReplacements = {};
 
@@ -130,10 +129,12 @@ export default class Module {
 							throw new Error( `Module ${module.path} does not export ${importDeclaration.name} (imported by ${this.path})` );
 						}
 
-						// we 'suggest' that the bundle use our local name for this import
-						// throughout the bundle. If that causes a conflict, we'll end up
-						// with something slightly different
-						module.nameReplacements[ exportDeclaration.localName ] = importDeclaration.localName;
+						const globalName = module.nameReplacements[ exportDeclaration.localName ];
+						if ( globalName ) {
+							this.rename( importDeclaration.localName, globalName );
+						} else {
+							module.rename( exportDeclaration.localName, importDeclaration.localName );
+						}
 
 						return module.define( exportDeclaration.localName );
 					});
@@ -143,17 +144,13 @@ export default class Module {
 			else if ( name === 'default' && this.exports.default.isDeclaration ) {
 				// We have something like `export default foo` - so we just start again,
 				// searching for `foo` instead of default. First, sync up names
-				this.nameReplacements.default = this.exports.default.name;
+				this.rename( 'default', this.exports.default.name );
 				promise = this.define( this.exports.default.name );
 			}
 
 			else {
 				let statement;
 
-				if ( !name ) {
-					console.log( new Error( 'no name' ).stack );
-				}
-
 				if ( name === 'default' ) {
 					// We have an expression, e.g. `export default 42`. We have
 					// to assign that expression to a variable
@@ -177,9 +174,6 @@ export default class Module {
 				if ( statement && !statement._imported ) {
 					const nodes = [];
 
-					// replace identifiers, as necessary
-					replaceIdentifiers( statement, statement._source, this.nameReplacements );
-
 					const include = statement => {
 						if ( statement._imported ) return emptyArrayPromise;
 
@@ -214,7 +208,11 @@ export default class Module {
 		return this.definitionPromises[ name ];
 	}
 
-	replaceName ( name, replacement ) {
+	rename ( name, replacement ) {
+		if ( hasOwnProp.call( this.nameReplacements, name ) ) {
+			throw new Error( 'Cannot rename an identifier twice' );
+		}
+
 		this.nameReplacements[ name ] = replacement;
 	}
 }

src/ast/analyse.js

@@ -9,7 +9,7 @@ function isStatement ( node, parent ) {
 	       node.type === 'FunctionDeclaration'; // TODO or any of the other various statement-ish things it could be
 }
 
-export default function analyse ( ast, magicString ) {
+export default function analyse ( ast, magicString, module ) {
 	let scope = new Scope();
 	let topLevelStatements = [];
 	let currentTopLevelStatement;
@@ -43,6 +43,9 @@ export default function analyse ( ast, magicString ) {
 		statement._dependsOn = {};
 		statement._imported = false;
 
+		// link back to the module
+		statement._module = module;
+
 		// store the actual code, for easy regeneration
 		statement._source = magicString.snip( previous, statement.end );
 		previous = statement.end;

test/samples/consistent-renaming/_config.js

@@ -0,0 +1,3 @@
+module.exports = {
+	description: 'renames identifiers consistently'
+};

test/samples/consistent-renaming/main.js

@@ -0,0 +1,3 @@
+import { one as oneRenamed } from './one';
+import { two } from './two';
+import { three as threeRenamed } from './three';

test/samples/consistent-renaming/one.js

@@ -0,0 +1 @@
+export var one = 1;

test/samples/consistent-renaming/three.js

@@ -0,0 +1 @@
+export var three = 3;

test/samples/consistent-renaming/two.js

@@ -0,0 +1,4 @@
+import { one as _one } from './one';
+import { three as _three } from './three';
+
+export var two = _three - _one;