Skip to content

Parse Google Closure type annotations correctly #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kpozin opened this issue Jul 11, 2012 · 11 comments
Closed

Parse Google Closure type annotations correctly #152

kpozin opened this issue Jul 11, 2012 · 11 comments

Comments

@kpozin
Copy link
Contributor

kpozin commented Jul 11, 2012

The grammar for Google Closure type annotations (page 8 of this doc) is a little bit too complex to be parsed with regular expressions (as in jsdoc/tag/type:parseTypes).

A real parser should be written for these annotations in JavaScript, or the Java version can be extracted from the Closure Compiler (source, JavaDoc).

Probably the biggest problem with the current implementation is that it very incorrectly splits union types. For example, Array.<(Object|number)> is split into Array.<(Object and number)>. In the absence of a complete parser, it's probably safer to keep this as a single type rather than to try to split it at the pipe character.

@hegemonic
Copy link
Contributor

I'm hoping to tackle this sometime in the very near future. Developers at my company are extremely keen to start using Closure Compiler (CC) types, but we can't make the switch until JSDoc is able to parse them.

My initial idea is to do something along these lines, which would also take care of #118:

  • Refactor the type parser into separate components, one for JSDoc-style annotations and another for CC-style annotations.
  • Each component would infer as much information as it could from the presence of characters that represent a type annotation. Components will not infer anything from the absence of those characters. For example, here's how the components would deal with optional types:
    • @param {string} foo: Both components return optional: null.
    • @param {string} [foo]: The JSDoc component returns optional: true, and the CC component returns optional: null.
    • @param {string=} foo: The JSDoc component returns optional: null, and the CC component returns optional: true.
    • @param {string=} [foo]: Both components return optional: true.
  • Each component would look at the raw doclet, then return a cleaned-up doclet that removes the type annotations. In the example above, the JSDoc component would change [foo] to foo, and the CC component would change {string=} to {string}.
  • The type parser would merge the results of the components, then add default values to fill in the gaps (for example, if both components reported optional: null, the type parser would set optional: false). If the components' results conflict, the type parser would throw an error. (Or, if the --lenient option is enabled, it could log a warning and set the types based on some predetermined order of precedence.)

That idea raises the following issues, though (and probably many others):

  • What's the canonical representation of something like @param {{bar: string, baz: number}} foo? Should we make the parse tree look as though the user had used JSDoc-style foo, foo.bar, and foo.baz @param tags?
  • Also, how would I document the bar and baz properties in that case?
  • Same deal for stuff like @param { function(string, number): string } foo. How would I document the function parameters and return value? (This points to a long-standing difficulty with documenting callbacks in JSDoc.)

@micmath, any thoughts?

@micmath
Copy link
Contributor

micmath commented Jul 11, 2012

I'm not going to suggest anyone should kill themselves to make JSDoc type declarations 100% compatible with Closure Compiler's type declarations, because it's just not that important to me personally. But I'm more than happy if someone wants to contribute that work, and I think it would be a great benefit to those that want to use both. I definitely see the problem with the "|" confusion.

For what it's worth, Google have published some docs on their type parsing algorithm that might be useful.

As to the question of converting CC types to JSDoc documentation, I'd say that it isn't necessary. The philosophy of JSDoc has always been to encourage code authors to write meaningful documentation, including textual descriptions and examples, neither of which are possible in a type declaration. In essence a CC-style type declaration competes as a poorer alternative to a JSDoc comment, when it comes to documentation.

Consider:

/**
    @function process
    @param {{bar: string, baz: number}} foo You cannot document foo much can you?
*/

Versus:

/**
    A valid argument to the process function.
    @typedef processArg
    @type object
    @property {string} bar The general description of the bar.
    @property {number} baz The age of the baz.
 */

/** @param {processArg} foo */
function process(foo) {
}

As promoters of documentation I'd say that the second example is better, because it allows and encourages more complete and more useful communication about how to use the API.

Having said that, I would reiterate that, in my opinion, having JSDoc properly parse those CC type definitions would be very beneficial, I just don't think it makes sense to try to generate too much documentation from it, even if you could.

@hegemonic
Copy link
Contributor

Thanks for the insight and the link, @micmath.

I think the solution is twofold:

  1. Parse the CC types correctly, and glean whatever information we can from them.
  2. Provide a way to document CC-typed parameters that can happily coexist with the CC types.

Here's a possibly half-baked idea that involves a new @typeref tag:

/**
    A valid argument to the process function.
    @typedef processArg
    @type object
    @property {string} bar The general description of the bar.
    @property {number} baz The age of the baz.
 */

/** @param {{bar: string, baz: number}} foo {@typeref processArg} */
function process(foo) {
}

Ugly as hell, but it would allow me to do my job as a tech writer, and the developers could still use CC types and enable CC's type-related optimizations.

@micmath
Copy link
Contributor

micmath commented Jul 11, 2012

The @typedef tag is already supported by Closure Compiler so you may be able to combine the two approaches already (I haven't tried it). That approach may be an easier way to go.

@hegemonic
Copy link
Contributor

Interesting. I'll look into that. But it looks like CC doesn't support the @property tag, which I suspect will leave us right back where we started.

@ghost ghost assigned hegemonic Jul 13, 2012
@hegemonic
Copy link
Contributor

I've made some progress on this, but I still have quite a ways to go.

@kpozin, is this issue causing major problems for you? If so, I can land a temporary fix for the Array.<(Object|number)> problem. But I'd rather focus on the long-term fix.

@kpozin
Copy link
Contributor Author

kpozin commented Jul 16, 2012

@hegemonic, this isn't a huge issue -- I can just join the type names array into a single string for now. I think a complete fix would be much more valuable. Thanks for working on this!

@hegemonic
Copy link
Contributor

For anyone who's curious, I've made some progress on this:

  • On my fork, I've refactored tag.js and type.js, in preparation for adding a more robust Closure Compiler parser.
  • Using PEG.js, I've written a grammar for Closure Compiler types. Still way too buggy to commit, but it's getting there.

Next up:

  • Finish the grammar.
  • Hook things up so that we add stub doclets for Closure Compiler types (e.g., if parameter foo is an object with the type {bar: string}, the generated docs should contain a foo.bar parameter with the type {string} and no description). I'll make it possible to disable this behavior.
  • Add lots of tests.

I'll probably file a separate issue to implement the @typeref idea, or something like it.

@hegemonic
Copy link
Contributor

I decided to land the refactoring before it got stale. The grammar is almost there, but I've been focusing on other stuff lately; will pick this up again at some point soonish.

@hegemonic
Copy link
Contributor

We also need this so we can correctly link to other types in the output. See #229.

hegemonic added a commit that referenced this issue Mar 15, 2013
introduces a real parser for Closure Compiler types, and uses the
parser to interpret type expressions in JSDoc tags.

TODO:
- provide a way to override the type expression
- update templateHelper to generate the correct links in type
applications

future enhancement (to be filed as a new issue): create pseudo-tags for
members that are described in the type expression (e.g., if the type
expression for the parameter `foo` is `{bar: string}`, add a tag for
`foo.bar` with no description)
hegemonic added a commit that referenced this issue Mar 18, 2013
…152)

- create `jsdoc/tag/inline` module, a generalized parser for inline tags
- use the new module to look for an inline `{@type}` tag in tag text;
for tags that can have a type, the inline tag overrides the type
expression
- update submodule
@hegemonic
Copy link
Contributor

This feature is now implemented on the master branch. It will be included in JSDoc 3.2.

All Closure Compiler type expressions should now be parsed and linked correctly. In particular, you can now mix type unions and type applications (for example, Array.<(MyClass|YourClass)>).

Also, JSDoc now throws an error and quits if a type expression cannot be parsed. You can turn this error into a warning by enabling the -l/--lenient option.

If a type expression identifies function parameters or object properties (for example, function(MyClass, YourClass) or {keyName: MyClass}), we do not try to turn the parameters or properties into stub docs. The former would have its type displayed as function, and the latter would display Object. For documentation purposes, it's best to avoid this sort of type expression.

If you're using Closure Compiler, though, there's a reasonable way to document these types while still using type expressions that Closure Compiler understands. Simply include an inline {@type <longname>} tag anywhere in the tag description. JSDoc will then pretend that you used <longname> as the type expression.

For example, JSDoc will look at the following code and pretend that the type expression for the options parameter was DonutOptions:

/**
 * Options for decorating a donut.
 *
 * @typedef DonutOptions
 * @type {Object}
 * @property {boolean} needsSprinkles - Whether the donut needs sprinkles.
 * @property {boolean} needsGlaze - Whether the donut needs glaze.
 */

/**
 * Decorate a donut.
 *
 * @param {Donut} donut - The donut to decorate.
 * @param {{needsSprinkles: boolean, needsGlaze: boolean}} options -
 *     Options for decorating the donut. {@type DonutOptions}
 * @return {Donut} The decorated donut.
 */
function decorateDonut(donut, options) {
    // ...
}

If you're not using Closure Compiler to validate types, you can (and probably should) document this the normal JSDoc way:

/**
 * Decorate a donut.
 *
 * @param {Donut} donut - The donut to decorate.
 * @param {Object} options - Options for decorating the donut.
 * @param {boolean} options.needsSprinkles - Whether the donut needs sprinkles.
 * @param {boolean} options.needsGlaze - Whether the donut needs glaze.
 * @return {Donut} The decorated donut.
 */
function decorateDonut(donut, options) {
    // ...
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants