Consider docblocks above decorators (fixes #18)

I wasn't able to create failing test cases at first. The following
example passed just fine:

/**
 * Test
 */
@decorator
class Foo {}

The comment was attached to the class declaration. However, the example in the
issue clearly showed that it doesn't work in practice.

After spending some trying to hunt down where/how the comment was
attached to the class declaration in my example, but not in the issue's
example, I remembered that recast attaches the (block?) comment to
the first statement in the body if it is the first node in the AST.

After adding something above the docblock (in this case an `import`
statement), the issue reproduced as expected.

Author: fkling

src/handlers/__tests__/componentDocblockHandler-test.js

@@ -35,6 +35,8 @@ describe('componentDocblockHandler', () => {
   function test(definitionSrc, parse) { // eslint-disable-line no-shadow
     it('finds docblocks for component definitions', () => {
       var definition = parse(`
+        import something from 'somewhere';
+
         /**
          * Component description
          */
@@ -47,6 +49,8 @@ describe('componentDocblockHandler', () => {
 
     it('ignores other types of comments', () => {
       var definition = parse(`
+        import something from 'somewhere';
+
         /*
          * This is not a docblock',
          */
@@ -67,6 +71,8 @@ describe('componentDocblockHandler', () => {
 
     it('only considers the docblock directly above the definition', () => {
       var definition = parse(`
+        import something from 'somewhere';
+
         /**
          * This is the wrong docblock
          */
@@ -79,6 +85,47 @@ describe('componentDocblockHandler', () => {
     });
   }
 
+  /**
+   * Decorates can only be assigned to class and therefore only make sense for
+   * class declarations and export declarations.
+   */
+  function testDecorators(definitionSrc, parse) { // eslint-disable-line no-shadow
+    describe('decorators', () => {
+      it('uses the docblock above the decorator if it\'s the only one', () => {
+        var definition = parse(`
+          import something from 'somewhere';
+          /**
+           * Component description
+           */
+          @Decorator1
+          @Decorator2
+          ${definitionSrc}
+        `);
+
+        componentDocblockHandler(documentation, definition);
+        expect(documentation.description).toBe('Component description');
+      });
+
+      it('uses the component docblock if present', () => {
+        var definition = parse(`
+          import something from 'somewhere';
+          /**
+           * Decorator description
+           */
+          @Decorator1
+          @Decorator2
+          /**
+           * Component description
+           */
+          ${definitionSrc}
+        `);
+
+        componentDocblockHandler(documentation, definition);
+        expect(documentation.description).toBe('Component description');
+      });
+    });
+  }
+
   describe('React.createClass', () => {
     test(
       'var Component = React.createClass({})',
@@ -91,6 +138,10 @@ describe('componentDocblockHandler', () => {
       'class Component {}',
       src => lastStatement(src)
     );
+    testDecorators(
+      'class Component {}',
+      src => lastStatement(src)
+    );
   });
 
   describe('ClassExpression', () => {
@@ -114,13 +165,21 @@ describe('componentDocblockHandler', () => {
         'export default class Component {}',
         src => lastStatement(src).get('declaration')
       );
+      testDecorators(
+        'export default class Component {}',
+        src => lastStatement(src).get('declaration')
+      );
     });
 
     describe('Default class expression export', () => {
       test(
         'export default class {}',
         src => lastStatement(src).get('declaration')
       );
+      testDecorators(
+        'export default class {}',
+        src => lastStatement(src).get('declaration')
+      );
     });
 
   });
@@ -141,6 +200,10 @@ describe('componentDocblockHandler', () => {
         'export class Component {}',
         src => lastStatement(src).get('declaration')
       );
+      testDecorators(
+        'export class Component {}',
+        src => lastStatement(src).get('declaration')
+      );
     });
 
   });

src/handlers/componentDocblockHandler.js

@@ -17,6 +17,12 @@ import {getDocblock} from '../utils/docblock';
 
 var {types: {namedTypes: types}} = recast;
 
+function isClassDefinition(nodePath) {
+  var node = nodePath.node;
+  return types.ClassDeclaration.check(node) ||
+    types.ClassExpression.check(node);
+}
+
 /**
  * Finds the nearest block comment before the component definition.
  */
@@ -26,30 +32,38 @@ export default function componentDocblockHandler(
 ) {
   var description = null;
   // Find parent statement (e.g. var Component = React.createClass(<path>);)
-  while (path && !types.Statement.check(path.node)) {
-    path = path.parent;
+  var searchPath = path;
+  while (searchPath && !types.Statement.check(searchPath.node)) {
+    searchPath = searchPath.parent;
   }
-  if (path) {
+  if (searchPath) {
     // Class declarations are statements but can be part of default
     // export declarations
-    if (types.ClassDeclaration.check(path.node) &&
-        types.ExportDefaultDeclaration.check(path.parentPath.node)) {
-      path = path.parentPath;
+    if (types.ClassDeclaration.check(searchPath.node) &&
+        types.ExportDefaultDeclaration.check(searchPath.parentPath.node)) {
+      searchPath = searchPath.parentPath;
     }
     // If the parent is an export statement, we have to traverse one more up
-    if (types.ExportNamedDeclaration.check(path.parentPath.node)) {
-      path = path.parentPath;
+    if (types.ExportNamedDeclaration.check(searchPath.parentPath.node)) {
+      searchPath = searchPath.parentPath;
+    }
+    description = getDocblock(searchPath);
+  }
+  if (description == null && isClassDefinition(path)) {
+    // If we have a class declaration or expression, then the comment might be
+    // attached to the first decorator instead.
+    if (path.node.decorators && path.node.decorators.length > 0) {
+      description = getDocblock(path.get('decorators', 0));
     }
-    description = getDocblock(path);
   }
   if (description == null) {
     // If this is the first statement in the module body, the comment is attached
     // to the program node
-    var programPath = path;
+    var programPath = searchPath;
     while (programPath && !types.Program.check(programPath.node)) {
       programPath = programPath.parent;
     }
-    if (programPath.get('body', 0) === path) {
+    if (programPath.get('body', 0) === searchPath) {
       description = getDocblock(programPath);
     }
   }

src/utils/docblock.js

@@ -30,15 +30,22 @@ let DOCBLOCK_HEADER = /^\*\s/;
  * exists.
  */
 export function getDocblock(path: NodePath): ?string {
+  var comments = [];
   if (path.node.comments) {
-    var comments = path.node.comments.filter(function(comment) {
-      return comment.leading &&
+    comments = path.node.comments.filter(
+      comment => comment.leading &&
         comment.type === 'CommentBlock' &&
-        DOCBLOCK_HEADER.test(comment.value);
-    });
-    if (comments.length > 0) {
-      return parseDocblock(comments[comments.length - 1].value);
-    }
+        DOCBLOCK_HEADER.test(comment.value)
+    );
+  } else if (path.node.leadingComments) {
+    comments = path.node.leadingComments.filter(
+      comment => comment.type === 'CommentBlock' &&
+        DOCBLOCK_HEADER.test(comment.value)
+    );
+  }
+
+  if (comments.length > 0) {
+    return parseDocblock(comments[comments.length - 1].value);
   }
   return null;
 }