flow-jsdoc icon indicating copy to clipboard operation
flow-jsdoc copied to clipboard

Esprima can't parse codes which already have flow type annotations

Open fand opened this issue 8 years ago • 4 comments

Problem

Esprima can't parse codes which already have flow type annotations.

When we have foo.js like this:

// @flow

/**
 * @param {string} str
 */
function foo(str): string {
  return str;
}

... then flow-jsdoc dies, because esprima.parse() can't parse flow type annotation and throws a parse error.

$ flow-jsdoc -f foo.js

/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:552
	        throw this.unexpectedTokenError(token, message);
	        ^
Error: Line 6: Unexpected token :
    at ErrorHandler.constructError (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:3396:22)
    at ErrorHandler.createError (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:3414:27)
    at JSXParser.Parser.unexpectedTokenError (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:542:39)
    at JSXParser.Parser.throwUnexpectedToken (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:552:21)
    at JSXParser.Parser.expect (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:716:19)
    at JSXParser.Parser.parseFunctionSourceElements (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:2572:15)
    at JSXParser.Parser.parseFunctionDeclaration (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:2713:26)
    at JSXParser.Parser.parseStatementListItem (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:1809:39)
    at JSXParser.Parser.parseProgram (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:3061:29)
    at parse (/Users/fand/.nodenv/versions/6.9.5/lib/node_modules/flow-jsdoc/node_modules/esprima/dist/esprima.js:117:24)

It would be great if flow-jsdoc could parse such codes. Replacing Esprima with Babylon will solve the problem.

Also, It would be nice if flow-jsdoc could handle the confliction between JSDoc types and existing flow type annotations 😺

fand avatar Mar 06 '17 05:03 fand

Please state the problem, not a proposed solution in your issues, thanks!

kegsay avatar Mar 07 '17 11:03 kegsay

I've thought about this and not really ever intended this tool to be used on projects which already have actual flowtype annotations because:

  • Why aren't you just using flowtype annotations throughout?
  • What do you do if you get conflicting information between JSDoc and flowtype annotations?

This feels too feature creepy for my liking. Leaving it open on the off-chance this is a highly requested feature, which would force my hand the other way.

kegsay avatar Mar 07 '17 11:03 kegsay

Why aren't you just using flowtype annotations throughout?

I encontered this propblem while working on a project, which already has JSDoc annotations and going to use flowtype. We've already added some flowtype annotations before we adopt flow-jsdoc, so we had to remove flowtype annotations.

Handwritten flowtype annotations are always correct while we check types in CI. But JSDoc type annotations are not always correct because we don't use Closure Compiler. (Most developers don't use Closure Compiler, AFAIK)

So It would be great if flow-jsdoc treats existing flowtype annotations well.

What do you do if you get conflicting information between JSDoc and flowtype annotations?

Well, this might be a hard problem. There are 2 possible solution:

  • Overwrite existing flowtype annotations with JSDoc annotations (or vice versa) by default
  • Let users to specify the priorities for JSDoc and flowtype annotations

Maybe we can start by just replacing the parser, and ignore the confliction.

fand avatar Mar 08 '17 01:03 fand

we can start by just replacing the parser, and ignore the confliction

If we do this then in about a month I'm going to get the following issues submitted to this issue tracker:

  • flow-jsdoc is replacing flow type annotations with JSDoc types
  • flow-jsdoc is not replacing flow type annotations with JSDoc types

It's not clear what the correct behaviour should be, so really the conflict needs to be sorted at the same time as the parser. I'm open to the idea of swapping this project to use Babylon in order to fix this issue, given your use case feels completely valid.

kegsay avatar Mar 08 '17 09:03 kegsay