fix(inference): Refactor membership inference

- Tags with `@lends` are now filtered out early on, so they never
  generate documentation. Lends tags are structural hints only.
- Uses built-in type checks instead of babel-types where appropriate.
- More internal documentation
- Now reuses code for parsing lends between different forms

Previously broken syntax, now working:

```js
/** parent */
export default function() {
  /** child */
  this.a = 1;
}
```

Author: tmcw

lib/infer/membership.js

@@ -7,24 +7,31 @@ var n = require('babel-types'),
   isJSDocComment = require('../../lib/is_jsdoc_comment'),
   parse = require('../../lib/parse');
 
+function inferModuleName(comment) {
+  return (comment.kind === 'module' && comment.name) ||
+    pathParse(comment.context.file).name;
+}
+
 /**
  * Given an AST node, try to find a comment in front of it that
  * has a `lends` tag, and if it has that, return the tag, split by
  * .s.
  *
  * @private
- * @param {Object} node AST node
+ * @param {Object} path AST node
  * @returns {string|undefined} lends identifier, if any
  */
-function findLendsIdentifiers(node) {
-  if (!node || !node.leadingComments) {
+function findLendsIdentifiers(path) {
+  if (!path || !path.get('leadingComments')) {
     return;
   }
 
-  for (var i = 0; i < node.leadingComments.length; i++) {
-    var comment = node.leadingComments[i];
-    if (isJSDocComment(comment)) {
-      var lends = parse(comment.value).lends;
+  var leadingComments = path.get('leadingComments');
+
+  for (var i = 0; i < leadingComments.length; i++) {
+    var comment = leadingComments[i];
+    if (isJSDocComment(comment.node)) {
+      var lends = parse(comment.node.value).lends;
       if (lends) {
         return lends.split('.');
       }
@@ -33,13 +40,16 @@ function findLendsIdentifiers(node) {
 }
 
 /**
- * Extract and return the identifiers for expressions of type this.foo
+ * Extract and return the identifiers for expressions of
+ * type this.foo
  *
- * @param {NodePath} path AssignmentExpression, MemberExpression, or Identifier
+ * @param {NodePath} path AssignmentExpression, MemberExpression,
+ * or Identifier
+ * @param {Comment} comment
  * @returns {Array<string>} identifiers
  * @private
  */
-function extractThis(path) {
+function extractThis(path, comment) {
   var identifiers = [];
 
   path.traverse({
@@ -60,14 +70,22 @@ function extractThis(path) {
         identifiers.push(scope.path.parentPath.parentPath.node.id.name, 'prototype');
       }
 
+      // function OldClass() { this.foo = 1 }
       if (n.isFunctionDeclaration(scope.block)) {
-        identifiers.push(scope.block.id.name, 'prototype');
-      } else if (n.isFunctionExpression(scope.block)) {
-        if (n.isVariableDeclarator(scope.path.parentPath)) {
+        // named function like
+        // function OldClass() { ... }
+        if (scope.block.id) {
+          identifiers.push(scope.block.id.name, 'prototype');
+        } else if (n.isExportDefaultDeclaration(path.scope.parentBlock)) {
+          identifiers.push(inferModuleName(comment));
+        }
+      // var Binding = function OldClass() { this.foo = 1 }
+      } else if (n.isFunctionExpression((scope.block))) {
+        if (scope.path.parentPath.isVariableDeclarator()) {
           /** var Bar = function(foo) { this.foo = foo; }; */
           identifiers = identifiers
             .concat(scope.path.parentPath.get('id').node.name).concat('prototype');
-        } else if (n.isAssignmentExpression(scope.path.parentPath)) {
+        } else if (scope.path.parentPath.isAssignmentExpression()) {
           /** this.Bar = function(foo) { this.foo = foo; }; */
           identifiers = identifiers
             .concat(extractIdentifiers(scope.path.parentPath.get('left'))).concat('prototype');
@@ -169,10 +187,6 @@ function normalizeMemberof(comment/*: Comment*/)/*: Comment */ {
 module.exports = function () {
   var currentModule;
 
-  function inferModuleName(comment) {
-    return (comment.kind === 'module' && comment.name) ||
-      pathParse(comment.context.file).name;
-  }
 
   /**
    * Set `memberof` and `instance`/`static` tags on `comment` based on the
@@ -185,13 +199,13 @@ module.exports = function () {
    * @param {Array<string>} identifiers array of identifier names
    * @param {string} explicitScope if derived from an es6 class, whether or
    * not this method had the static keyword
-   * @returns {undefined} mutates `comment`
+   * @returns {Comment} returns mutated `comment`
    * @private
    */
   function inferMembershipFromIdentifiers(comment, identifiers, explicitScope) {
     if (identifiers.length === 1 && identifiers[0] === 'module' && comment.name === 'exports') {
       comment.name = inferModuleName(currentModule || comment);
-      return;
+      return comment;
     }
 
     /*
@@ -215,142 +229,166 @@ module.exports = function () {
         comment.scope = 'static';
       }
     }
+    return comment;
   }
 
-  return function inferMembership(comment/*: Comment */) {
-
+  function shouldSkipInference(comment/*: Comment */)/*: boolean */ {
+    // If someone uses the @name tag, they explicitly ask for inference
+    // to be skipped.
     if (comment.tags.some(tag => tag.title === 'name')) {
-      return comment;
+      return true;
     }
 
-    if (comment.kind === 'module') {
-      currentModule = comment;
+    // Lends tags are go-betweens that let people reassign membership
+    // in bulk: they themselves don't get an inference step
+    if (comment.lends) {
+      return true;
     }
 
-    if (comment.lends) {
+    // If this chunk doesn't have code attached, like if it was the result
+    // of a polyglot parse, don't try to infer anything.
+    if (!comment.context.ast) {
+      return true;
+    }
+
+    return false;
+  }
+
+  return function inferMembership(comment/*: Comment */) {
+
+    // First skip inference if the user indicates it or if it isn't possible.
+    if (shouldSkipInference(comment)) {
       return comment;
     }
 
+    // If someone explicitly specifies the parent of this chunk, don't
+    // try to infer it, just return what they specified.
     if (comment.memberof) {
       return normalizeMemberof(comment);
     }
 
-    if (!comment.context.ast) {
-      return comment;
+    if (comment.kind === 'module') {
+      currentModule = comment;
     }
 
     var path = comment.context.ast;
-    var identifiers;
-
 
+    // INFERENCE ===============================================================
     // Deal with an oddity of espree: the jsdoc comment is attached to a different
     // node in the two expressions `a.b = c` vs `a.b = function () {}`.
-    if (n.isExpressionStatement(path.node) &&
-        n.isAssignmentExpression(path.node.expression) &&
-        n.isMemberExpression(path.node.expression.left)) {
+    if (path.isExpressionStatement() &&
+      path.get('expression').isAssignmentExpression() &&
+      path.get('expression').get('left').isMemberExpression()) {
       path = path.get('expression').get('left');
     }
 
     // Same as above but for `b: c` vs `b: function () {}`.
-    if (n.isObjectProperty(path.node) &&
-        n.isIdentifier(path.node.key)) {
+    if (path.isObjectProperty() &&
+        path.get('key').isIdentifier()) {
       path = path.get('key');
     }
 
+    // Forms:
+    //
     // Foo.bar = ...;
     // Foo.prototype.bar = ...;
     // Foo.bar.baz = ...;
-    if (n.isMemberExpression(path.node)) {
-      identifiers = [].concat(
-        extractThis(path),
+    //
+    // Lends is not supported in this codepath.
+    if (path.isMemberExpression()) {
+      var memberIdentifiers = [].concat(
+        extractThis(path, comment),
         extractIdentifiers(path)
       );
-      if (identifiers.length >= 2) {
-        inferMembershipFromIdentifiers(comment, identifiers.slice(0, -1));
+      if (memberIdentifiers.length >= 2) {
+        return inferMembershipFromIdentifiers(comment, memberIdentifiers.slice(0, -1));
       }
+      return comment;
     }
 
-    // /** @lends Foo */{ bar: ... }
-    if (n.isIdentifier(path.node) &&
-        n.isObjectProperty(path.parentPath) &&
-        n.isObjectExpression(path.parentPath.parentPath)) {
-      // The @lends comment is sometimes attached to the first property rather than
-      // the object expression itself.
-      identifiers = findLendsIdentifiers(path.parentPath.parentPath.node) ||
-          findLendsIdentifiers(path.parentPath.parentPath.node.properties[0]);
-      if (identifiers) {
-        inferMembershipFromIdentifiers(comment, identifiers);
-      }
-    }
+    // Like straight membership, classes don't need
+    // to support lends.
+    //
+    // class Foo { bar() { } }
+    // var Foo = class { bar() { } }
+    // class Foo { prop: T }
+    // var Foo = class { prop: T }
+    if ((path.isClassMethod() || path.isClassProperty()) &&
+        path.parentPath.isClassBody() &&
+        path.parentPath.parentPath.isClass()) {
 
-    // Foo = { bar: ... };
-    // Foo.prototype = { bar: ... };
-    // Foo.bar = { baz: ... };
-    if (n.isIdentifier(path.node) &&
-        n.isObjectProperty(path.parentPath) &&
-        n.isObjectExpression(path.parentPath.parentPath) &&
-        n.isAssignmentExpression(path.parentPath.parentPath.parentPath)) {
-      identifiers = extractIdentifiers(path.parentPath.parentPath.parentPath.get('left'));
-      if (identifiers.length >= 1) {
-        inferMembershipFromIdentifiers(comment, identifiers);
+      var scope = 'instance';
+      if (path.node.static == true) {
+        scope = 'static';
       }
-    }
 
-    // Shorthand methods on ordinary objects
-    if (n.isObjectMethod(path.node) &&
-      n.isObjectExpression(path.parentPath)) {
-
-      // Foo = { bar() {} };
-      // Foo.prototype = { bar() {} };
-      // Foo.bar = { baz() {} };
-      if (n.isAssignmentExpression(path.parentPath.parentPath)) {
-        identifiers = extractIdentifiers(path.parentPath.parentPath.get('left'));
-        if (identifiers.length >= 1) {
-          inferMembershipFromIdentifiers(comment, identifiers);
-        }
+      if (path.parentPath.parentPath.isExpression()) {
+        return inferMembershipFromIdentifiers(comment,
+          extractIdentifiers(path.parentPath.parentPath.parentPath.get('left')),
+          scope);
       }
 
-      // var Foo = { bar() {} };
-      if (n.isVariableDeclarator(path.parentPath.parentPath)) {
-        identifiers = [path.parentPath.parentPath.get('id').node.name];
-        inferMembershipFromIdentifiers(comment, identifiers);
+      var declarationNode = path.parentPath.parentPath.node;
+      if (!declarationNode.id) {
+        // export default function () {}
+        // export default class {}
+        // Use module name instead.
+        return inferMembershipFromIdentifiers(comment, [pathParse(comment.context.file).name], scope);
       }
+
+      return inferMembershipFromIdentifiers(comment, [declarationNode.id.name], scope);
     }
 
-    // var Foo = { bar: ... }
-    if (n.isIdentifier(path) &&
-        n.isObjectProperty(path.parentPath) &&
-        n.isObjectExpression(path.parentPath.parentPath) &&
-        n.isVariableDeclarator(path.parentPath.parentPath.parentPath)) {
-      identifiers = [path.parentPath.parentPath.parentPath.node.id.name];
-      inferMembershipFromIdentifiers(comment, identifiers);
+    // Whether something is an ObjectMethod (shorthand like foo() {} )
+    // or ObjectProperty (old fashioned like foo: function() {} )
+    // doesn't matter for the membership phase, as long as we end up knowing
+    // that it belongs to an object. So we first establish objectParent,
+    // and then have the logic for the numerous ways an object can be named.
+    var objectParent;
+
+    if (path.isIdentifier() &&
+      path.parentPath.isObjectProperty() &&
+      path.parentPath.parentPath.isObjectExpression()) {
+      objectParent = path.parentPath.parentPath;
+    } else if (path.isObjectMethod() &&
+      path.parentPath.isObjectExpression()) {
+      objectParent = path.parentPath;
     }
 
-    // class Foo { bar() { } }
-    // var Foo = class { bar() { } }
-    // class Foo { prop: T }
-    // var Foo = class { prop: T }
-    if ((n.isClassMethod(path) || n.isClassProperty(path)) &&
-        n.isClassBody(path.parentPath) &&
-        n.isClass(path.parentPath.parentPath)) {
-      if (n.isExpression(path.parentPath.parentPath)) {
-        identifiers = extractIdentifiers(path.parentPath.parentPath.parentPath.get('left'));
-      } else {
-        var declarationNode = path.parentPath.parentPath.node;
-        if (!declarationNode.id) {
-          // export default function () {}
-          // export default class {}
-          // Use module name instead.
-          identifiers = [pathParse(comment.context.file).name];
-        } else {
-          identifiers = [declarationNode.id.name];
-        }
-      }
-      var scope = 'instance';
-      if (path.node.static == true) {
-        scope = 'static';
+    // Confirm that the thing being documented is a property of an object.
+    if (objectParent) {
+
+      // The @lends comment is sometimes attached to the first property rather than
+      // the object expression itself.
+      var lendsIdentifiers = findLendsIdentifiers(objectParent) ||
+          findLendsIdentifiers(objectParent.get('properties')[0]);
+
+      if (lendsIdentifiers) {
+
+        return inferMembershipFromIdentifiers(comment, lendsIdentifiers);
+
+      } else if (objectParent.parentPath.isAssignmentExpression()) {
+
+        // Foo = { ... };
+        // Foo.prototype = { ... };
+        // Foo.bar = { ... };
+        return inferMembershipFromIdentifiers(comment,
+          extractIdentifiers(objectParent.parentPath.get('left')));
+
+      } else if (objectParent.parentPath.isVariableDeclarator()) {
+
+        // var Foo = { ... };
+        return inferMembershipFromIdentifiers(comment,
+          [objectParent.parentPath.get('id').node.name]);
+
+      } else if (objectParent.parentPath.isExportDefaultDeclaration()) {
+
+        // export default { ... };
+        return inferMembershipFromIdentifiers(comment,
+          [inferModuleName(currentModule || comment)]);
+
       }
-      inferMembershipFromIdentifiers(comment, identifiers, scope);
+
     }
 
     // var function Foo() {

lib/is_jsdoc_comment.js

@@ -11,7 +11,7 @@
  * comments.
  *
  * @name isJSDocComment
- * @param {Object} comment an ast node of the comment
+ * @param {Object} comment an ast path of the comment
  * @return {boolean} whether it is valid
  */
 module.exports = function isJSDocComment(comment/*: {

lib/module_filters.js

@@ -12,7 +12,6 @@ var internalModuleRegexp = process.platform === 'win32' ?
 
 /**
  * Module filters
- * @private
  */
 module.exports = {
   internalOnly: internalModuleRegexp.test.bind(internalModuleRegexp),

lib/parsers/javascript.js

@@ -45,7 +45,8 @@ function parseJavaScript(data/*: Object*/,
     walkComments.bind(null, 'leadingComments', true),
     walkComments.bind(null, 'innerComments', false),
     walkComments.bind(null, 'trailingComments', false)
-  ], fn => fn(ast, data, addComment)).filter(Boolean);
+  ], fn => fn(ast, data, addComment))
+    .filter(comment => comment && !comment.lends);
 }
 
 function _addComment(visited, data, commentValue, commentLoc, path, nodeLoc, includeContext) {