Merge pull request #212 from benmosher/issue-200

deep namespaces: correct caching

gulpfile.js

@@ -87,13 +87,16 @@ gulp.task('tests', function () {
 // used externally by Istanbul, too
 gulp.task('pretest', ['src', 'tests', 'wipe-extras'])
 
+var reporter = 'spec'
+
 gulp.task('test', ['pretest'], function () {
   return gulp.src('tests/lib/**/*.js', { read: false })
-    .pipe(mocha({ reporter: 'spec', grep: process.env.TEST_GREP }))
+    .pipe(mocha({ reporter: reporter, grep: process.env.TEST_GREP }))
   // NODE_PATH=./lib mocha --recursive --reporter dot tests/lib/
 })
 
 gulp.task('watch-test', function () {
+  reporter = 'progress'
   gulp.watch(SRC, ['test'])
   gulp.watch('tests/' + SRC, ['test'])
 })

src/core/getExports.js

@@ -11,17 +11,22 @@ import isIgnored from './ignore'
 const exportCaches = new Map()
 
 export default class ExportMap {
-  constructor(context) {
-    this.context = context
-    this.named = new Map()
-
+  constructor(path) {
+    this.path = path
+    this.namespace = new Map()
+    // todo: restructure to key on path, value is resolver + map of names
+    this.reexports = new Map()
+    this.dependencies = new Map()
     this.errors = []
   }
 
-  get settings() { return this.context && this.context.settings }
+  get hasDefault() { return this.get('default') != null } // stronger than this.has
 
-  get hasDefault() { return this.named.has('default') }
-  get hasNamed() { return this.named.size > (this.hasDefault ? 1 : 0) }
+  get size() {
+    let size = this.namespace.size + this.reexports.size
+    this.dependencies.forEach(dep => size += dep().size)
+    return size
+  }
 
   static get(source, context) {
 
@@ -62,7 +67,7 @@ export default class ExportMap {
     exportMap.mtime = stats.mtime
 
     // ignore empties, optionally
-    if (exportMap.named.size === 0 && isIgnored(path, context)) {
+    if (exportMap.namespace.size === 0 && isIgnored(path, context)) {
       exportMap = null
     }
 
@@ -72,7 +77,7 @@ export default class ExportMap {
   }
 
   static parse(path, context) {
-    var m = new ExportMap(context)
+    var m = new ExportMap(path)
 
     try {
       var ast = parse(path, context)
@@ -97,28 +102,49 @@ export default class ExportMap {
 
     const namespaces = new Map()
 
+    function remotePath(node) {
+      return resolve.relative(node.source.value, path, context.settings)
+    }
+
+    function resolveImport(node) {
+      const rp = remotePath(node)
+      if (rp == null) return null
+      return ExportMap.for(rp, context)
+    }
+
     function getNamespace(identifier) {
       if (!namespaces.has(identifier.name)) return
 
-      let namespace = m.resolveReExport(namespaces.get(identifier.name), path)
-      if (namespace) return { namespace: namespace.named }
+      return function () {
+        return resolveImport(namespaces.get(identifier.name))
+      }
+    }
+
+    function addNamespace(object, identifier) {
+      const nsfn = getNamespace(identifier)
+      if (nsfn) {
+        Object.defineProperty(object, 'namespace', { get: nsfn })
+      }
+
+      return object
     }
 
+
     ast.body.forEach(function (n) {
 
       if (n.type === 'ExportDefaultDeclaration') {
         const exportMeta = captureDoc(n)
         if (n.declaration.type === 'Identifier') {
-          Object.assign(exportMeta, getNamespace(n.declaration))
+          addNamespace(exportMeta, n.declaration)
         }
-        m.named.set('default', exportMeta)
+        m.namespace.set('default', exportMeta)
         return
       }
 
       if (n.type === 'ExportAllDeclaration') {
-        let remoteMap = m.resolveReExport(n, path)
+        let remoteMap = remotePath(n)
         if (remoteMap == null) return
-        remoteMap.named.forEach((value, name) => { m.named.set(name, value) })
+        m.dependencies.set(remoteMap, () => ExportMap.for(remoteMap, context))
         return
       }
 
@@ -138,47 +164,114 @@ export default class ExportMap {
             case 'FunctionDeclaration':
             case 'ClassDeclaration':
             case 'TypeAlias': // flowtype with babel-eslint parser
-              m.named.set(n.declaration.id.name, captureDoc(n))
+              m.namespace.set(n.declaration.id.name, captureDoc(n))
               break
             case 'VariableDeclaration':
               n.declaration.declarations.forEach((d) =>
-                recursivePatternCapture(d.id, id => m.named.set(id.name, captureDoc(d, n))))
+                recursivePatternCapture(d.id, id => m.namespace.set(id.name, captureDoc(d, n))))
               break
           }
         }
 
-        // capture specifiers
-        let remoteMap
-        if (n.source) remoteMap = m.resolveReExport(n, path)
-
         n.specifiers.forEach((s) => {
           const exportMeta = {}
+          let local
 
-          if (s.type === 'ExportDefaultSpecifier') {
-            // don't add it if it is not present in the exported module
-            if (!remoteMap || !remoteMap.hasDefault) return
-          } else if (s.type === 'ExportSpecifier'){
-            Object.assign(exportMeta, getNamespace(s.local))
-          } else if (s.type === 'ExportNamespaceSpecifier') {
-            exportMeta.namespace = remoteMap.named
+          switch (s.type) {
+            case 'ExportDefaultSpecifier':
+              if (!n.source) return
+              local = 'default'
+              break
+            case 'ExportNamespaceSpecifier':
+              m.namespace.set(s.exported.name, Object.defineProperty(exportMeta, 'namespace', {
+                get() { return resolveImport(n) },
+              }))
+              return
+            case 'ExportSpecifier':
+              if (!n.source) {
+                m.namespace.set(s.exported.name, addNamespace(exportMeta, s.local))
+                return
+              }
+              // else falls through
+            default:
+              local = s.local.name
+              break
           }
 
           // todo: JSDoc
-          m.named.set(s.exported.name, exportMeta)
+          m.reexports.set(s.exported.name, { local, getImport: () => resolveImport(n) })
         })
       }
     })
 
     return m
   }
 
-  resolveReExport(node, base) {
-    var remotePath = resolve.relative(node.source.value, base, this.settings)
-    if (remotePath == null) return null
+  /**
+   * Note that this does not check explicitly re-exported names for existence
+   * in the base namespace, but it will expand all `export * from '...'` exports
+   * if not found in the explicit namespace.
+   * @param  {string}  name
+   * @return {Boolean} true if `name` is exported by this module.
+   */
+  has(name) {
+    if (this.namespace.has(name)) return true
+    if (this.reexports.has(name)) return true
+
+    for (let dep of this.dependencies.values()) {
+      let innerMap = dep()
 
-    return ExportMap.for(remotePath, this.context)
+      // todo: report as unresolved?
+      if (!innerMap) continue
+
+      if (innerMap.has(name)) return true
+    }
+
+    return false
   }
 
+  get(name) {
+    if (this.namespace.has(name)) return this.namespace.get(name)
+
+    if (this.reexports.has(name)) {
+      const { local, getImport } = this.reexports.get(name)
+          , imported = getImport()
+      if (imported == null) return undefined
+
+      // safeguard against cycles, only if name matches
+      if (imported.path === this.path && local === name) return undefined
+
+      return imported.get(local)
+    }
+
+    for (let dep of this.dependencies.values()) {
+      let innerMap = dep()
+      // todo: report as unresolved?
+      if (!innerMap) continue
+
+      // safeguard against cycles
+      if (innerMap.path === this.path) continue
+
+      let innerValue = innerMap.get(name)
+      if (innerValue !== undefined) return innerValue
+    }
+
+    return undefined
+  }
+
+  forEach(callback, thisArg) {
+    this.namespace.forEach((v, n) =>
+      callback.call(thisArg, v, n, this))
+
+    this.reexports.forEach(({ getImport, local }, name) =>
+      callback.call(thisArg, getImport().get(local), name, this))
+
+    this.dependencies.forEach(dep => dep().forEach((v, n) =>
+      callback.call(thisArg, v, n, this)))
+  }
+
+  // todo: keys, values, entries?
+
   reportErrors(context, declaration) {
     context.report({
       node: declaration.source,
@@ -251,3 +344,4 @@ function hashObject(object) {
   settingsShasum.update(JSON.stringify(object))
   return settingsShasum.digest('hex')
 }
+``

src/rules/default.js

@@ -19,7 +19,7 @@ module.exports = function (context) {
 
     if (imports.errors.length) {
       imports.reportErrors(context, node)
-    } else if (!imports.hasDefault) {
+    } else if (!imports.get('default')) {
       context.report(defaultSpecifier, 'No default export found in module.')
     }
   }

src/rules/export.js

@@ -1,8 +1,7 @@
 import ExportMap, { recursivePatternCapture } from '../core/getExports'
 
 module.exports = function (context) {
-  const defaults = new Set()
-      , named = new Map()
+  const named = new Map()
 
   function addNamed(name, node) {
     let nodes = named.get(name)
@@ -16,9 +15,7 @@ module.exports = function (context) {
   }
 
   return {
-    'ExportDefaultDeclaration': function (node) {
-      defaults.add(node)
-    },
+    'ExportDefaultDeclaration': (node) => addNamed('default', node),
 
     'ExportSpecifier': function (node) {
       addNamed(node.exported.name, node.exported)
@@ -48,29 +45,23 @@ module.exports = function (context) {
         remoteExports.reportErrors(context, node)
         return
       }
+      let any = false
+      remoteExports.forEach((v, name) => (any = true) && addNamed(name, node))
 
-      if (!remoteExports.hasNamed) {
+      if (!any) {
         context.report(node.source,
           `No named exports found in module '${node.source.value}'.`)
       }
-
-      for (let name of remoteExports.named.keys()) {
-        addNamed(name, node)
-      }
     },
 
     'Program:exit': function () {
-      if (defaults.size > 1) {
-        for (let node of defaults) {
-          context.report(node, 'Multiple default exports.')
-        }
-      }
-
       for (let [name, nodes] of named) {
         if (nodes.size <= 1) continue
 
         for (let node of nodes) {
-          context.report(node, `Multiple exports of name '${name}'.`)
+          if (name === 'default') {
+            context.report(node, 'Multiple default exports.')
+          } else context.report(node, `Multiple exports of name '${name}'.`)
         }
       }
     },

src/rules/named.js

@@ -17,12 +17,10 @@ module.exports = function (context) {
       return
     }
 
-    var names = imports.named
-
     node.specifiers.forEach(function (im) {
       if (im.type !== type) return
 
-      if (!names.has(im[key].name)) {
+      if (!imports.get(im[key].name)) {
         context.report(im[key],
           im[key].name + ' not found in \'' + node.source.value + '\'')
       }

src/rules/namespace.js

@@ -32,15 +32,15 @@ module.exports = function (context) {
         for (let specifier of declaration.specifiers) {
           switch (specifier.type) {
             case 'ImportNamespaceSpecifier':
-              if (!imports.hasNamed) {
+              if (!imports.size) {
                 context.report(specifier,
                   `No exported names found in module '${declaration.source.value}'.`)
               }
-              namespaces.set(specifier.local.name, imports.named)
+              namespaces.set(specifier.local.name, imports)
               break
             case 'ImportDefaultSpecifier':
             case 'ImportSpecifier': {
-              const meta = imports.named.get(
+              const meta = imports.get(
                 // default to 'default' for default http://i.imgur.com/nj6qAWy.jpg
                 specifier.imported ? specifier.imported.name : 'default')
               if (!meta || !meta.namespace) break
@@ -65,7 +65,7 @@ module.exports = function (context) {
         return
       }
 
-      if (!imports.hasNamed) {
+      if (!imports.size) {
         context.report(namespace,
           `No exported names found in module '${declaration.source.value}'.`)
       }
@@ -87,7 +87,7 @@ module.exports = function (context) {
       var namespace = namespaces.get(dereference.object.name)
       var namepath = [dereference.object.name]
       // while property is namespace and parent is member expression, keep validating
-      while (namespace instanceof Map &&
+      while (namespace instanceof Exports &&
              dereference.type === 'MemberExpression') {
 
         if (dereference.computed) {
@@ -122,7 +122,7 @@ module.exports = function (context) {
 
       // DFS traverse child namespaces
       function testKey(pattern, namespace, path = [init.name]) {
-        if (!(namespace instanceof Map)) return
+        if (!(namespace instanceof Exports)) return
 
         if (pattern.type !== 'ObjectPattern') return