From 365f45c7f3dd0854c94abeaab184469063e48103 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oskar=20Segersv=C3=A4rd?= Date: Thu, 24 Sep 2015 19:03:04 +0200 Subject: [PATCH] Made Scope.reference stricter. Helps to catch undefined/missing exports. --- src/Module.js | 13 +++++++++++++ src/Scope.js | 7 ++++++- .../import-of-unexported-fails/_config.js | 10 ++++++++++ .../function/import-of-unexported-fails/empty.js | 0 test/function/import-of-unexported-fails/main.js | 3 +++ test/testScope.js | 16 +++++----------- 6 files changed, 37 insertions(+), 12 deletions(-) create mode 100644 test/function/import-of-unexported-fails/_config.js create mode 100644 test/function/import-of-unexported-fails/empty.js create mode 100644 test/function/import-of-unexported-fails/main.js diff --git a/src/Module.js b/src/Module.js index 2123103..8f0c212 100644 --- a/src/Module.js +++ b/src/Module.js @@ -50,6 +50,18 @@ class Id { } } +class LateBoundIdPlaceholder { + constructor ( module, name ) { + this.module = module; + this.name = name; + this.placeholder = true; + } + + mark () { + throw new Error(`The imported name "${this.name}" is never exported by "${this.module.id}".`); + } +} + export default class Module { constructor ({ id, source, ast, bundle }) { this.source = source; @@ -103,6 +115,7 @@ export default class Module { } // throw new Error( `The name "${name}" is never exported (from ${this.id})!` ); + this.exports.define( name, new LateBoundIdPlaceholder( this, name ) ); return reference.call( this.exports, name ); }; diff --git a/src/Scope.js b/src/Scope.js index 8185085..e93a8bb 100644 --- a/src/Scope.js +++ b/src/Scope.js @@ -81,6 +81,7 @@ export default class Scope { }); this.ids.filter( isntReference ).forEach( id => { + // TODO: can this be removed? if ( typeof id === 'string' ) { throw new Error( `Required name "${id}" is undefined!` ); } @@ -153,7 +154,11 @@ export default class Scope { // Get a reference to the identifier `name` in this scope. reference ( name ) { - return new Reference( this, this.index( name ) ); + if ( !( name in this.names ) ) { + throw new Error( `Cannot reference undefined identifier "${name}"` ); + } + + return new Reference( this, this.names[ name ] ); } // Return the used names of the scope. diff --git a/test/function/import-of-unexported-fails/_config.js b/test/function/import-of-unexported-fails/_config.js new file mode 100644 index 0000000..75e1586 --- /dev/null +++ b/test/function/import-of-unexported-fails/_config.js @@ -0,0 +1,10 @@ +var assert = require( 'assert' ); + +module.exports = { + description: 'marking an imported, but unexported, identifier should throw', + + error: function ( err ) { + assert.equal( err.message.slice( 0, 50 ), 'The imported name "default" is never exported by "' ); + assert.equal( err.message.slice( -10 ), 'empty.js".' ); + } +}; diff --git a/test/function/import-of-unexported-fails/empty.js b/test/function/import-of-unexported-fails/empty.js new file mode 100644 index 0000000..e69de29 diff --git a/test/function/import-of-unexported-fails/main.js b/test/function/import-of-unexported-fails/main.js new file mode 100644 index 0000000..37cf334 --- /dev/null +++ b/test/function/import-of-unexported-fails/main.js @@ -0,0 +1,3 @@ +import a from './empty.js'; + +a(); diff --git a/test/testScope.js b/test/testScope.js index 2658774..67e443d 100644 --- a/test/testScope.js +++ b/test/testScope.js @@ -93,7 +93,7 @@ describe( 'Scope', function () { }); }); - it( 'dedupes-external-imports', function () { + it( 'cannot reference undefined names', function () { var real = new Scope(); var external = real.virtual(), @@ -104,17 +104,11 @@ describe( 'Scope', function () { locals.bind( 'Comp', external.reference( 'Component' ) ); - exports.bind( 'default', locals.reference( 'Foo' ) ); - - try { - real.deconflict(); - assert.ok( false, 'Scope.deconflict should throw with "Foo" undefined' ); - } catch ( ignore ) { - // as expected - } + assert.throws( function () { + exports.bind( 'default', locals.reference( 'Foo' ) ); + }, 'Cannot reference undefined identifier "Foo"' ); locals.define( 'Foo' ); - - real.deconflict(); + exports.bind( 'default', locals.reference( 'Foo' ) ); }); });