disallow reassignments and updates of imported bindings/namespaces

src/Module.js

@@ -37,30 +37,6 @@ export default class Module {
 	}
 
 	analyse () {
-		analyse( this.ast, this.code, this );
-
-		this.definedNames = this.ast._scope.names.slice();
-
-		this.canonicalNames = {};
-
-		this.definitions = {};
-		this.definitionPromises = {};
-		this.modifications = {};
-
-		this.ast.body.forEach( statement => {
-			Object.keys( statement._defines ).forEach( name => {
-				this.definitions[ name ] = statement;
-			});
-
-			Object.keys( statement._modifies ).forEach( name => {
-				if ( !has( this.modifications, name ) ) {
-					this.modifications[ name ] = [];
-				}
-
-				this.modifications[ name ].push( statement );
-			});
-		});
-
 		// imports and exports, indexed by ID
 		this.imports = {};
 		this.exports = {};
@@ -160,6 +136,32 @@ export default class Module {
 				}
 			}
 		});
+
+
+
+		analyse( this.ast, this.code, this );
+
+		this.definedNames = this.ast._scope.names.slice();
+
+		this.canonicalNames = {};
+
+		this.definitions = {};
+		this.definitionPromises = {};
+		this.modifications = {};
+
+		this.ast.body.forEach( statement => {
+			Object.keys( statement._defines ).forEach( name => {
+				this.definitions[ name ] = statement;
+			});
+
+			Object.keys( statement._modifies ).forEach( name => {
+				if ( !has( this.modifications, name ) ) {
+					this.modifications[ name ] = [];
+				}
+
+				this.modifications[ name ].push( statement );
+			});
+		});
 	}
 
 	getCanonicalName ( name ) {

src/ast/analyse.js

@@ -1,6 +1,8 @@
 import walk from './walk';
 import Scope from './Scope';
 import { getName } from '../utils/map-helpers';
+import { has } from '../utils/object';
+import getLocation from '../utils/getLocation';
 
 export default function analyse ( ast, magicString, module ) {
 	let scope = new Scope();
@@ -135,11 +137,19 @@ export default function analyse ( ast, magicString, module ) {
 		}
 
 		function checkForWrites ( node ) {
-			function addNode ( node ) {
+			function addNode ( node, disallowImportReassignments ) {
 				while ( node.type === 'MemberExpression' ) {
 					node = node.object;
 				}
 
+				// disallow assignments/updates to imported bindings and namespaces
+				if ( disallowImportReassignments && has( module.imports, node.name ) && !scope.contains( node.name ) ) {
+					const err = new Error( `Illegal reassignment to import '${node.name}'` );
+					err.file = module.path;
+					err.loc = getLocation( module.code.toString(), node.start );
+					throw err;
+				}
+
 				if ( node.type !== 'Identifier' ) {
 					return;
 				}
@@ -148,11 +158,15 @@ export default function analyse ( ast, magicString, module ) {
 			}
 
 			if ( node.type === 'AssignmentExpression' ) {
-				addNode( node.left );
+				addNode( node.left, true );
+			}
+
+			else if ( node.type === 'UpdateExpression' ) {
+				addNode( node.argument, true );
 			}
 
 			else if ( node.type === 'CallExpression' ) {
-				node.arguments.forEach( addNode );
+				node.arguments.forEach( arg => addNode( arg, false ) );
 			}
 
 			// TODO UpdateExpressions, method calls?
@@ -178,4 +192,4 @@ export default function analyse ( ast, magicString, module ) {
 	});
 
 	ast._scope = scope;
-}
+}

test/samples/namespace-reassign-import-fails/_config.js

@@ -1,10 +1,12 @@
+var path = require( 'path' );
 var assert = require( 'assert' );
 
 module.exports = {
 	description: 'disallows reassignments to namespace exports',
 	error: function ( err ) {
-		console.log( err );
-		assert.ok( false, 'TODO figure out correct error' );
+		assert.equal( err.file, path.resolve( __dirname, 'main.js' ) );
+		assert.deepEqual( err.loc, { line: 3, column: 0 });
+		assert.ok( /Illegal reassignment/.test( err.message ) );
 	}
 };
 

test/samples/namespace-update-import-fails/_config.js

@@ -1,10 +1,12 @@
+var path = require( 'path' );
 var assert = require( 'assert' );
 
 module.exports = {
 	description: 'disallows updates to namespace exports',
 	error: function ( err ) {
-		console.log( err );
-		assert.ok( false, 'TODO figure out correct error' );
+		assert.equal( err.file, path.resolve( __dirname, 'main.js' ) );
+		assert.deepEqual( err.loc, { line: 3, column: 0 });
+		assert.ok( /Illegal reassignment/.test( err.message ) );
 	}
 };
 

test/samples/reassign-import-fails/_config.js

@@ -1,9 +1,12 @@
+var path = require( 'path' );
 var assert = require( 'assert' );
 
 module.exports = {
 	description: 'disallows assignments to imported bindings',
 	error: function ( err ) {
-		assert.ok( false, 'TODO choose error' );
+		assert.equal( err.file, path.resolve( __dirname, 'main.js' ) );
+		assert.deepEqual( err.loc, { line: 8, column: 0 });
+		assert.ok( /Illegal reassignment/.test( err.message ) );
 	}
 };
 

test/samples/reassign-import-not-at-top-level-fails/_config.js

@@ -1,9 +1,12 @@
+var path = require( 'path' );
 var assert = require( 'assert' );
 
 module.exports = {
 	description: 'disallows assignments to imported bindings not at the top level',
 	error: function ( err ) {
-		assert.ok( false, 'TODO choose error' );
+		assert.equal( err.file, path.resolve( __dirname, 'main.js' ) );
+		assert.deepEqual( err.loc, { line: 7, column: 2 });
+		assert.ok( /Illegal reassignment/.test( err.message ) );
 	}
 };
 

test/samples/update-expression-of-import-fails/_config.js

@@ -1,9 +1,12 @@
+var path = require( 'path' );
 var assert = require( 'assert' );
 
 module.exports = {
 	description: 'disallows updates to imported bindings',
 	error: function ( err ) {
-		assert.ok( false, 'TODO choose error' );
+		assert.equal( err.file, path.resolve( __dirname, 'main.js' ) );
+		assert.deepEqual( err.loc, { line: 3, column: 0 });
+		assert.ok( /Illegal reassignment/.test( err.message ) );
 	}
 };