Skip to content

Commit

Permalink
Merge pull request #1517 from lukastaegert/abolish-scope-parameters
Browse files Browse the repository at this point in the history 
Refactor scope handling
  • Loading branch information
@Rich-Harris
Rich-Harris committed Aug 11, 2017
2 parents bfeea43 + 3cfb4d4 commit 2395452
Show file tree
Hide file tree
Showing 39 changed files with 492 additions and 570 deletions.
144 changes: 72 additions & 72 deletions src/Bundle.js
View file

Large diffs are not rendered by default.

82 changes: 39 additions & 43 deletions src/Module.js
View file
Expand Up @@ -17,22 +17,22 @@ import ModuleScope from './ast/scopes/ModuleScope.js';

function tryParse ( module, acornOptions ) {
try {
return parse( module.code, assign({
return parse( module.code, assign( {
ecmaVersion: 8,
sourceType: 'module',
onComment: ( block, text, start, end ) => module.comments.push({ block, text, start, end }),
onComment: ( block, text, start, end ) => module.comments.push( { block, text, start, end } ),
preserveParens: false
}, acornOptions ));
}, acornOptions ) );
} catch ( err ) {
module.error({
module.error( {
code: 'PARSE_ERROR',
message: err.message.replace( / \(\d+:\d+\)$/, '' )
}, err.pos );
}
}

export default class Module {
constructor ({ id, code, originalCode, originalSourceMap, ast, sourceMapChain, resolvedIds, resolvedExternalIds, bundle }) {
constructor ( { id, code, originalCode, originalSourceMap, ast, sourceMapChain, resolvedIds, resolvedExternalIds, bundle } ) {
this.code = code;
this.id = id;
this.bundle = bundle;
Expand Down Expand Up @@ -79,17 +79,17 @@ export default class Module {
this.magicString = new MagicString( code, {
filename: this.excludeFromSourcemap ? null : id, // don't include plugin helpers in sourcemap
indentExclusionRanges: []
});
} );

// remove existing sourceMappingURL comments
this.comments = this.comments.filter(comment => {
this.comments = this.comments.filter( comment => {
//only one line comment can contain source maps
const isSourceMapComment = !comment.block && SOURCEMAPPING_URL_RE.test(comment.text);
if (isSourceMapComment) {
this.magicString.remove(comment.start, comment.end );
const isSourceMapComment = !comment.block && SOURCEMAPPING_URL_RE.test( comment.text );
if ( isSourceMapComment ) {
this.magicString.remove( comment.start, comment.end );
}
return !isSourceMapComment;
});
} );

this.declarations = blank();
this.type = 'Module'; // TODO only necessary so that Scope knows this should be treated as a function scope... messy
Expand Down Expand Up @@ -122,7 +122,7 @@ export default class Module {
const name = specifier.exported.name;

if ( this.exports[ name ] || this.reexports[ name ] ) {
this.error({
this.error( {
code: 'DUPLICATE_EXPORT',
message: `A module cannot have multiple exports with the same name ('${name}')`
}, specifier.start );
Expand All @@ -134,7 +134,7 @@ export default class Module {
localName: specifier.local.name,
module: null // filled in later
};
});
} );
}
}

Expand All @@ -145,7 +145,7 @@ export default class Module {
const identifier = ( node.declaration.id && node.declaration.id.name ) || node.declaration.name;

if ( this.exports.default ) {
this.error({
this.error( {
code: 'DUPLICATE_EXPORT',
message: `A module can only have one default export`
}, node.start );
Expand All @@ -171,8 +171,8 @@ export default class Module {
declaration.declarations.forEach( decl => {
extractNames( decl.id ).forEach( localName => {
this.exports[ localName ] = { localName };
});
});
} );
} );
} else {
// export function foo () {}
const localName = declaration.id.name;
Expand All @@ -181,7 +181,7 @@ export default class Module {
}

// export { foo, bar, baz }
else if ( node.specifiers.length ) {
else {
node.specifiers.forEach( specifier => {
const localName = specifier.local.name;
const exportedName = specifier.exported.name;
Expand All @@ -207,7 +207,7 @@ export default class Module {
const localName = specifier.local.name;

if ( this.imports[ localName ] ) {
this.error({
this.error( {
code: 'DUPLICATE_IMPORT',
message: `Duplicated import '${localName}'`
}, specifier.start );
Expand All @@ -218,7 +218,7 @@ export default class Module {

const name = isDefault ? 'default' : isNamespace ? '*' : specifier.imported.name;
this.imports[ localName ] = { source, specifier, name, module: null };
});
} );
}

analyse () {
Expand Down Expand Up @@ -253,13 +253,13 @@ export default class Module {

const id = this.resolvedIds[ specifier.source ] || this.resolvedExternalIds[ specifier.source ];
specifier.module = this.bundle.moduleById.get( id );
});
});
} );
} );

this.exportAllModules = this.exportAllSources.map( source => {
const id = this.resolvedIds[ source ] || this.resolvedExternalIds[ source ];
return this.bundle.moduleById.get( id );
});
} );

this.sources.forEach( source => {
const id = this.resolvedIds[ source ];
Expand All @@ -268,12 +268,12 @@ export default class Module {
const module = this.bundle.moduleById.get( id );
this.dependencies.push( module );
}
});
} );
}

bindReferences () {
for ( const node of this.ast.body ) {
node.bind( this.scope );
node.bind();
}

// if ( this.declarations.default ) {
Expand All @@ -288,7 +288,7 @@ export default class Module {
if ( pos !== undefined ) {
props.pos = pos;

const { line, column } = locate( this.code, pos, { offsetLine: 1 }); // TODO trace sourcemaps
const { line, column } = locate( this.code, pos, { offsetLine: 1 } ); // TODO trace sourcemaps

props.loc = { file: this.id, line, column };
props.frame = getCodeFrame( this.code, line, column );
Expand All @@ -302,20 +302,16 @@ export default class Module {
return null;
}

findScope () {
return this.scope;
}

getExports () {
const exports = blank();

keys( this.exports ).forEach( name => {
exports[ name ] = true;
});
} );

keys( this.reexports ).forEach( name => {
exports[ name ] = true;
});
} );

this.exportAllModules.forEach( module => {
if ( module.isExternal ) {
Expand All @@ -325,18 +321,18 @@ export default class Module {

module.getExports().forEach( name => {
if ( name !== 'default' ) exports[ name ] = true;
});
});
} );
} );

return keys( exports );
}

namespace () {
if ( !this.declarations['*'] ) {
this.declarations['*'] = new SyntheticNamespaceDeclaration( this );
if ( !this.declarations[ '*' ] ) {
this.declarations[ '*' ] = new SyntheticNamespaceDeclaration( this );
}

return this.declarations['*'];
return this.declarations[ '*' ];
}

render ( es, legacy ) {
Expand All @@ -355,8 +351,8 @@ export default class Module {

run () {
for ( const node of this.ast.body ) {
if ( node.hasEffects( this.scope ) ) {
node.run( this.scope );
if ( node.hasEffects() ) {
node.run();
}
}
}
Expand Down Expand Up @@ -392,7 +388,7 @@ export default class Module {
const declaration = otherModule.traceExport( importDeclaration.name );

if ( !declaration ) {
this.error({
this.error( {
code: 'MISSING_EXPORT',
message: `'${importDeclaration.name}' is not exported by ${relativeId( otherModule.id )}`,
url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module`
Expand All @@ -407,7 +403,7 @@ export default class Module {

traceExport ( name ) {
// export * from 'external'
if ( name[0] === '*' ) {
if ( name[ 0 ] === '*' ) {
const module = this.bundle.moduleById.get( name.slice( 1 ) );
return module.traceExport( '*' );
}
Expand All @@ -418,7 +414,7 @@ export default class Module {
const declaration = reexportDeclaration.module.traceExport( reexportDeclaration.localName );

if ( !declaration ) {
this.error({
this.error( {
code: 'MISSING_EXPORT',
message: `'${reexportDeclaration.localName}' is not exported by ${relativeId( reexportDeclaration.module.id )}`,
url: `https://github.com/rollup/rollup/wiki/Troubleshooting#name-is-not-exported-by-module`
Expand All @@ -439,7 +435,7 @@ export default class Module {
if ( name === 'default' ) return;

for ( let i = 0; i < this.exportAllModules.length; i += 1 ) {
const module = this.exportAllModules[i];
const module = this.exportAllModules[ i ];
const declaration = module.traceExport( name );

if ( declaration ) return declaration;
Expand All @@ -450,7 +446,7 @@ export default class Module {
if ( pos !== undefined ) {
warning.pos = pos;

const { line, column } = locate( this.code, pos, { offsetLine: 1 }); // TODO trace sourcemaps
const { line, column } = locate( this.code, pos, { offsetLine: 1 } ); // TODO trace sourcemaps

warning.loc = { file: this.id, line, column };
warning.frame = getCodeFrame( this.code, line, column );
Expand Down
44 changes: 25 additions & 19 deletions src/ast/Node.js
View file
Expand Up @@ -2,8 +2,8 @@ import { locate } from 'locate-character';
import { UNKNOWN } from './values.js';

export default class Node {
bind ( scope ) {
this.eachChild( child => child.bind( this.scope || scope ) );
bind () {
this.eachChild( child => child.bind() );
}

eachChild ( callback ) {
Expand All @@ -28,11 +28,6 @@ export default class Node {
return selector.test( this.type ) ? this : this.parent.findParent( selector );
}

// TODO abolish findScope. if a node needs to store scope, store it
findScope ( functionScope ) {
return this.parent.findScope( functionScope );
}

gatherPossibleValues ( values ) {
//this.eachChild( child => child.gatherPossibleValues( values ) );
values.add( UNKNOWN );
Expand All @@ -42,28 +37,41 @@ export default class Node {
return UNKNOWN;
}

hasEffects ( scope ) {
if ( this.scope ) scope = this.scope;

hasEffects () {
for ( const key of this.keys ) {
const value = this[ key ];

if ( value ) {
if ( 'length' in value ) {
for ( const child of value ) {
if ( child && child.hasEffects( scope ) ) {
if ( child && child.hasEffects() ) {
return true;
}
}
} else if ( value && value.hasEffects( scope ) ) {
} else if ( value.hasEffects() ) {
return true;
}
}
}
}

initialise ( scope ) {
this.eachChild( child => child.initialise( this.scope || scope ) );
initialise ( parentScope ) {
this.initialiseScope( parentScope );
this.initialiseNode( parentScope );
this.initialiseChildren( parentScope );
}

// Override if e.g. some children need to be initialised with the parent scope
initialiseChildren () {
this.eachChild( child => child.initialise( this.scope ) );
}

// Override to perform special initialisation steps after the scope is initialised
initialiseNode () {}

// Overwrite to create a new scope
initialiseScope ( parentScope ) {
this.scope = parentScope;
}

insertSemicolon ( code ) {
Expand All @@ -74,7 +82,7 @@ export default class Node {

locate () {
// useful for debugging
const location = locate( this.module.code, this.start, { offsetLine: 1 });
const location = locate( this.module.code, this.start, { offsetLine: 1 } );
location.file = this.module.id;
location.toString = () => JSON.stringify( location );

Expand All @@ -85,13 +93,11 @@ export default class Node {
this.eachChild( child => child.render( code, es ) );
}

run ( scope ) {
run () {
if ( this.ran ) return;
this.ran = true;

this.eachChild( child => {
child.run( this.scope || scope );
});
this.eachChild( child => child.run() );
}

toString () {
Expand Down

0 comments on commit 2395452

Please sign in to comment.