Skip to content

Commit 82ee830

Browse files
tmcwtmcw
committed
refactor(nest): Better nesting implementation
This nesting implementation uses a proper recursive tree algorithm Fixes #554
1 parent d8ec5da commit 82ee830

File tree

3 files changed

+96
-152
lines changed

3 files changed

+96
-152
lines changed

‎declarations/comment.js

+15-14
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ declare type CommentTag = {
6565
default?: any,
6666
lineNumber?: number,
6767
type?: DoctrineType,
68-
properties?: Array<CommentTag>
68+
children?: Array<CommentTag>
6969
};
7070

7171
declare type CommentMembers = {
@@ -88,18 +88,19 @@ declare type Remark = {
8888

8989
declare type Access = 'private' | 'public' | 'protected';
9090
declare type Scope = 'instance' | 'static' | 'inner' | 'global';
91-
declare type Kind = 'class' |
92-
'constant' |
93-
'event' |
94-
'external' |
95-
'file' |
96-
'function' |
97-
'member' |
98-
'mixin' |
99-
'module' |
100-
'namespace' |
101-
'typedef' |
102-
'interface';
91+
declare type Kind =
92+
| 'class'
93+
| 'constant'
94+
| 'event'
95+
| 'external'
96+
| 'file'
97+
| 'function'
98+
| 'member'
99+
| 'mixin'
100+
| 'module'
101+
| 'namespace'
102+
| 'typedef'
103+
| 'interface';
103104

104105
declare type Comment = {
105106
errors: Array<CommentError>,
@@ -152,4 +153,4 @@ declare type ReducedComment = {
152153
name: string,
153154
kind: ?Kind,
154155
scope?: ?Scope
155-
}
156+
};

‎lib/nest.js

+56-45
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,70 @@
11
/* @flow */
22
'use strict';
33

4+
var _ = require('lodash');
5+
6+
const PATH_SPLIT = /(?:\[])?\./g;
7+
48
/**
59
* Nest nestable tags, like param and property, into nested
610
* arrays that are suitable for output.
11+
*Okay, so we're building a tree of comments, with the tagName
12+
* being the indexer
13+
*
14+
* foo.abe
15+
* foo.bar.baz
16+
* foo.bar.a
17+
* foo.bar[].bax
18+
*
19+
* foo -> .abe
20+
* \-> .bar -> .baz
21+
* \-> .a
22+
* \-> [].baz
723
*
824
* @private
925
* @param {Object} comment a comment with tags
1026
* @param {string} tagTitle the tag to nest
1127
* @param {string} target the tag to nest into
1228
* @returns {Object} nested comment
1329
*/
14-
function nestTag(
15-
comment /*: Comment */,
16-
tagTitle /*: string */,
17-
target /*: string */
18-
) {
19-
if (!comment[target]) {
20-
return comment;
21-
}
30+
var nestTag = (tags /*: Array<CommentTag> */) =>
31+
tags
32+
.sort(
33+
(a, b) =>
34+
a.name.split(PATH_SPLIT).length - b.name.split(PATH_SPLIT).length
35+
)
36+
.reduce(
37+
(memo, tag) => {
38+
function insertTag(node, parts) {
39+
// The 'done' case: we're at parts.length === 1,
40+
// this is where the node should be placed. We reliably
41+
// get to this case because the recursive method
42+
// is always passed parts.slice(1)
43+
if (parts.length === 1) {
44+
_.assign(node, {
45+
children: (node.children || []).concat(tag)
46+
});
47+
} else {
48+
// The recursive case: try to find the child that owns
49+
// this tag.
50+
let child = node.children &&
51+
node.children.find(
52+
property => parts[0] === _.last(property.name.split(PATH_SPLIT))
53+
);
2254

23-
var result = [], index = {};
55+
if (!child) {
56+
throw new Error(`Parent of ${tag.name} not found `);
57+
}
2458

25-
comment[target].forEach(tag => {
26-
var tagName = tag.name;
27-
if (tagName) {
28-
index[tagName] = tag;
29-
var parts = tagName
30-
.split(/(\[\])?\./)
31-
.filter(part => part && part !== '[]');
32-
if (parts.length > 1) {
33-
var parent = index[parts.slice(0, -1).join('.')];
34-
if (parent === undefined) {
35-
comment.errors.push({
36-
message: '@' +
37-
tagTitle +
38-
' ' +
39-
tag.name +
40-
"'s parent " +
41-
parts[0] +
42-
' not found',
43-
commentLineNumber: tag.lineNumber
44-
});
45-
result.push(tag);
46-
return;
59+
insertTag(child, parts.slice(1));
60+
}
4761
}
48-
parent.properties = parent.properties || [];
49-
parent.properties.push(tag);
50-
} else {
51-
result.push(tag);
52-
}
53-
}
54-
});
55-
56-
comment[target] = result;
5762

58-
return comment;
59-
}
63+
insertTag(memo, tag.name.split(PATH_SPLIT));
64+
return memo;
65+
},
66+
{ children: [] }
67+
);
6068

6169
/**
6270
* Nests
@@ -70,8 +78,11 @@ function nestTag(
7078
* @param {Object} comment input comment
7179
* @return {Object} nested comment
7280
*/
73-
function nest(comment /*: Comment*/) {
74-
return nestTag(nestTag(comment, 'param', 'params'), 'property', 'properties');
75-
}
81+
var nest = (comment /*: Comment*/) =>
82+
_.assign(comment, {
83+
params: nestTag(comment.params),
84+
properties: nestTag(comment.properties)
85+
});
7686

7787
module.exports = nest;
88+
module.exports.nestTag = nestTag;

‎test/lib/nest.js

+25-93
Original file line numberDiff line numberDiff line change
@@ -1,101 +1,33 @@
11
'use strict';
22

3-
var test = require('tap').test,
4-
parse = require('../../lib/parsers/javascript'),
5-
nest = require('../../lib/nest');
3+
var test = require('tap').test;
4+
var nestTag = require('../../lib/nest').nestTag;
65

7-
function toComment(fn, filename) {
8-
return parse(
9-
{
10-
file: filename,
11-
source: fn instanceof Function ? '(' + fn.toString() + ')' : fn
12-
},
13-
{}
14-
).map(nest);
15-
}
16-
17-
test('nest params - no params', function(t) {
18-
t.deepEqual(
19-
toComment(function() {
20-
/** @name foo */
21-
})[0].params,
22-
[],
23-
'no params'
24-
);
25-
t.end();
26-
});
27-
28-
test('nest params - no nesting', function(t) {
29-
var result = toComment(function() {
30-
/** @param {Object} foo */
31-
});
32-
t.equal(result[0].params.length, 1);
33-
t.equal(result[0].params[0].name, 'foo');
34-
t.equal(result[0].params[0].properties, undefined);
35-
t.end();
36-
});
6+
// Print a tree of tags in a way that's easy to test.
7+
var printTree = indent =>
8+
node =>
9+
`${new Array(indent + 1).join(' ')}- ${node.name}\n${(node.children || [])
10+
.map(printTree(indent + 1))
11+
.join('\n')}`;
3712

3813
test('nest params - basic', function(t) {
39-
var result = toComment(function() {
40-
/**
41-
* @param {Object} foo
42-
* @param {string} foo.bar
43-
* @param {string} foo.baz
44-
*/
45-
});
46-
t.equal(result[0].params.length, 1);
47-
t.equal(result[0].params[0].name, 'foo');
48-
t.equal(result[0].params[0].properties.length, 2);
49-
t.equal(result[0].params[0].properties[0].name, 'foo.bar');
50-
t.equal(result[0].params[0].properties[1].name, 'foo.baz');
51-
t.end();
52-
});
53-
54-
test('nest properties - basic', function(t) {
55-
var result = toComment(function() {
56-
/**
57-
* @property {Object} foo
58-
* @property {string} foo.bar
59-
* @property {string} foo.baz
60-
*/
61-
});
62-
t.equal(result[0].properties.length, 1);
63-
t.equal(result[0].properties[0].name, 'foo');
64-
t.equal(result[0].properties[0].properties.length, 2);
65-
t.equal(result[0].properties[0].properties[0].name, 'foo.bar');
66-
t.equal(result[0].properties[0].properties[1].name, 'foo.baz');
67-
t.end();
68-
});
69-
70-
test('nest params - array', function(t) {
71-
var result = toComment(function() {
72-
/**
73-
* @param {Object[]} employees - The employees who are responsible for the project.
74-
* @param {string} employees[].name - The name of an employee.
75-
* @param {string} employees[].department - The employee's department.
76-
*/
77-
});
78-
t.equal(result[0].params.length, 1);
79-
t.equal(result[0].params[0].name, 'employees');
80-
t.equal(result[0].params[0].properties.length, 2);
81-
t.equal(result[0].params[0].properties[0].name, 'employees[].name');
82-
t.equal(result[0].params[0].properties[1].name, 'employees[].department');
83-
t.end();
84-
});
85-
86-
test('nest params - missing parent', function(t) {
87-
var result = toComment(function() {
88-
/** @param {string} foo.bar */
89-
});
90-
t.equal(result[0].params.length, 1);
91-
t.deepEqual(
92-
result[0].errors[0],
93-
{
94-
message: "@param foo.bar's parent foo not found",
95-
commentLineNumber: 0
96-
},
97-
'correct error message'
14+
var params = [
15+
'foo',
16+
'foo.bar',
17+
'foo.bar.third',
18+
'foo.third',
19+
'foo.third[].baz'
20+
].map(name => ({ name }));
21+
var result = nestTag(params);
22+
t.equal(
23+
printTree(0)(result.children[0]),
24+
`- foo
25+
- foo.bar
26+
- foo.bar.third
27+
28+
- foo.third
29+
- foo.third[].baz
30+
`
9831
);
99-
t.equal(result[0].params[0].name, 'foo.bar');
10032
t.end();
10133
});

0 commit comments

Comments
 (0)