remove, and warn about, unused imports from external modules (#595)

src/Bundle.js

@@ -175,6 +175,21 @@ export default class Bundle {
 
 				timeStart( 'phase 4' );
 
+				// while we're here, check for unused external imports
+				this.externalModules.forEach( module => {
+					const unused = Object.keys( module.declarations )
+						.filter( name => name !== '*' )
+						.filter( name => !module.declarations[ name ].activated );
+
+					if ( unused.length === 0 ) return;
+
+					const names = unused.length === 1 ?
+						`'${unused[0]}' is` :
+						`${unused.slice( 0, -1 ).map( name => `'${name}'` ).join( ', ' )} and '${unused.pop()}' are`;
+
+					this.onwarn( `${names} imported from external module '${module.id}' but never used` );
+				});
+
 				this.orderedModules = this.sort();
 				this.deconflict();
 
@@ -329,6 +344,16 @@ export default class Bundle {
 							this.externalModules.push( module );
 							this.moduleById.set( externalId, module );
 						}
+
+						const externalModule = this.moduleById.get( externalId );
+
+						// add external declarations so we can detect which are never used
+						Object.keys( module.imports ).forEach( name => {
+							const importDeclaration = module.imports[ name ];
+							if ( importDeclaration.source !== source ) return;
+
+							externalModule.traceExport( importDeclaration.name );
+						});
 					} else {
 						if ( resolvedId === module.id ) {
 							throw new Error( `A module cannot import itself (${resolvedId})` );

src/Declaration.js

@@ -99,13 +99,13 @@ export class ExternalDeclaration {
 		this.safeName = null;
 		this.isExternal = true;
 
-		this.activated = true;
+		this.activated = false;
 
 		this.isNamespace = name === '*';
 	}
 
 	activate () {
-		// noop
+		this.activated = true;
 	}
 
 	addReference ( reference ) {

src/finalisers/cjs.js

@@ -28,7 +28,12 @@ export default function cjs ( bundle, magicString, { exportMode, intro, outro },
 
 				return `${varOrConst} ${module.name} = _interopDefault(require('${module.path}'));`;
 			} else {
-				return `${varOrConst} ${module.name} = require('${module.path}');`;
+				const activated = Object.keys( module.declarations )
+					.filter( name => module.declarations[ name ].activated );
+
+				return activated.length ?
+					`${varOrConst} ${module.name} = require('${module.path}');` :
+					`require('${module.path}');`;
 			}
 		})
 		.join( '\n' );

src/finalisers/es.js

@@ -11,6 +11,7 @@ export default function es ( bundle, magicString, { intro, outro } ) {
 			const specifiersList = [specifiers];
 			const importedNames = keys( module.declarations )
 				.filter( name => name !== '*' && name !== 'default' )
+				.filter( name => module.declarations[ name ].activated )
 				.map( name => {
 					const declaration = module.declarations[ name ];
 

test/form/prefer-const/_expected/amd.js

@@ -1,4 +1,4 @@
-define(['external', 'other', 'another'], function (external, other, another) { 'use strict';
+define(['other'], function (other) { 'use strict';
 
 	const a = 1;
 	const b = 2;
@@ -10,6 +10,7 @@ define(['external', 'other', 'another'], function (external, other, another) { '
 	});
 
 	console.log( Object.keys( namespace ) );
+	console.log( other.name );
 
 	const main = 42;
 

test/form/prefer-const/_expected/cjs.js

@@ -1,8 +1,6 @@
 'use strict';
 
-const external = require('external');
 const other = require('other');
-const another = require('another');
 
 const a = 1;
 const b = 2;
@@ -14,6 +12,7 @@ const namespace = Object.freeze({
 });
 
 console.log( Object.keys( namespace ) );
+console.log( other.name );
 
 const main = 42;
 

test/form/prefer-const/_expected/es.js

@@ -1,6 +1,4 @@
-import 'external';
+import { name } from 'other';
-import 'other';
-import 'another';
 
 const a = 1;
 const b = 2;
@@ -12,6 +10,7 @@ const namespace = Object.freeze({
 });
 
 console.log( Object.keys( namespace ) );
+console.log( name );
 
 const main = 42;
 

test/form/prefer-const/_expected/iife.js

@@ -1,4 +1,4 @@
-const myBundle = (function (external,other,another) {
+const myBundle = (function (other) {
 	'use strict';
 
 	const a = 1;
@@ -11,9 +11,10 @@ const myBundle = (function (external,other,another) {
 	});
 
 	console.log( Object.keys( namespace ) );
+	console.log( other.name );
 
 	const main = 42;
 
 	return main;
 
-}(external,other,another));
+}(other));

test/form/prefer-const/_expected/umd.js

@@ -1,8 +1,8 @@
 (function (global, factory) {
-	typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory(require('external'), require('other'), require('another')) :
+	typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory(require('other')) :
-	typeof define === 'function' && define.amd ? define(['external', 'other', 'another'], factory) :
+	typeof define === 'function' && define.amd ? define(['other'], factory) :
-	(global.myBundle = factory(global.external,global.other,global.another));
+	(global.myBundle = factory(global.other));
-}(this, (function (external,other,another) { 'use strict';
+}(this, (function (other) { 'use strict';
 
 	const a = 1;
 	const b = 2;
@@ -14,6 +14,7 @@
 	});
 
 	console.log( Object.keys( namespace ) );
+	console.log( other.name );
 
 	const main = 42;
 

test/form/prefer-const/main.js

@@ -1,9 +1,7 @@
-import external from 'external';
+import { name } from 'other';
-import a from 'other';
-import { b } from 'other';
-import { another } from 'another';
 import * as namespace from './namespace.js';
 
 console.log( Object.keys( namespace ) );
+console.log( name );
 
 export default 42;

test/form/unused-import/_config.js

@@ -0,0 +1,5 @@
+const assert = require( 'assert' );
+
+module.exports = {
+	description: 'excludes unused imports ([#595])'
+};

test/form/unused-import/_expected/amd.js

@@ -0,0 +1,5 @@
+define(['external'], function (external) { 'use strict';
+
+
+
+});

test/form/unused-import/_expected/cjs.js

@@ -0,0 +1,3 @@
+'use strict';
+
+require('external');

test/form/unused-import/_expected/es.js

@@ -0,0 +1 @@
+import 'external';

test/form/unused-import/_expected/iife.js

@@ -0,0 +1,6 @@
+(function (external) {
+	'use strict';
+
+
+
+}(external));

test/form/unused-import/_expected/umd.js

@@ -0,0 +1,9 @@
+(function (global, factory) {
+	typeof exports === 'object' && typeof module !== 'undefined' ? factory(require('external')) :
+	typeof define === 'function' && define.amd ? define(['external'], factory) :
+	(factory(global.external));
+}(this, (function (external) { 'use strict';
+
+
+
+})));

test/form/unused-import/main.js

@@ -0,0 +1,5 @@
+import { unused } from 'external';
+
+function alsoUnused () {
+	unused();
+}

test/function/class-methods-not-renamed/foo.js

@@ -2,7 +2,7 @@ import { bar } from 'path'; // path, so the test doesn't fail for unrelated reas
 
 export default class Foo {
 	bar () {
-		if ( false ) return bar();
+		if ( Math.random() < 0 ) return bar();
 		return true;
 	}
 }

test/function/preserves-function-expression-names/_config.js

@@ -7,5 +7,6 @@ module.exports = {
 	},
 	exports ( exports ) {
 		assert.equal( exports.x.name, 'basename' );
+		assert.equal( exports.y, 'somefile.txt' );
 	}
 };

test/function/preserves-function-expression-names/main.js

@@ -1,5 +1,6 @@
 import { basename } from 'path';
 
 var x = function basename () {};
+var y = basename( 'path/to/somefile.txt' );
 
-export { x };
+export { x, y };

test/function/unused-import/_config.js

@@ -0,0 +1,12 @@
+const assert = require( 'assert' );
+
+module.exports = {
+	description: 'warns on unused imports ([#595])',
+	warnings: warnings => {
+		assert.deepEqual( warnings, [
+			`Treating 'external' as external dependency`,
+			`'unused', 'notused' and 'neverused' are imported from external module 'external' but never used`,
+			`Generated an empty bundle`
+		]);
+	}
+};

test/function/unused-import/main.js

@@ -0,0 +1,5 @@
+import { unused, notused, neverused as willnotuse } from 'external';
+
+function alsoUnused () {
+	unused();
+}