Skip to content

Commit

Permalink
fix(inference): Refactor membership inference
Browse files Browse the repository at this point in the history 
- 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;
}
```
  • Loading branch information
@tmcw
tmcw authored Feb 24, 2017
1 parent 802dc4c commit 84c9215
Show file tree
Hide file tree
Showing 16 changed files with 1,655 additions and 133 deletions.
258 changes: 148 additions & 110 deletions lib/infer/membership.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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('.');
}
Expand All @@ -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({
Expand All @@ -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');
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

/*
Expand All @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion lib/is_jsdoc_comment.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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/*: {
Expand Down
1 change: 0 additions & 1 deletion lib/module_filters.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ var internalModuleRegexp = process.platform === 'win32' ?

/**
* Module filters
* @private
*/
module.exports = {
internalOnly: internalModuleRegexp.test.bind(internalModuleRegexp),
Expand Down
3 changes: 2 additions & 1 deletion lib/parsers/javascript.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Loading

0 comments on commit 84c9215

Please sign in to comment.