Build jsdoc static analysis tool

[jasonLaster]

This issue was started in #1756 by @ChetHarrison

We should build a tool that helps automate some of the jsdoc work involved with documentation:

1. generate yaml for new classes / utility functions
2. lint existing api for missing functions, parameters, properties, events
3. add function bodies to api

The work is being done in this repo: https://github.com/ChetHarrison/jsdoccer

[ChetHarrison]

@jasonLaster good idea on the new ticket. I am a little confused by the last post. Why would we capture the code in the JSON and not in the YAML? The good news is you can already configure the tool to do that. So here is an example. If we wanted to document all functions, we would add the following config to the syntaxWhitelist in the .jsdoccerrc config file:

    "syntaxWhitelist": {
        "FunctionDeclaration": {
            "attributes": ["id", "params"],
            "code": ["body"]
        }
    }

this tells the tool to grab the function name id, all the parameter names params and the code from the function body and return the following JSON for the iterateEvents function from bindEntityEvents.js:

{ type: 'FunctionDeclaration',
    id: [ 'iterateEvents' ],
    params: 
     [ 'target',
       'entity',
       'bindings',
       'functionCallback',
       'stringCallback' ],
    body: '{\n    if (!entity || !bindings) {\n        return;\n    }\n    if (!_.isFunction(bindings) && !_.isObject(bindings)) {\n        throw new Marionette.Error({\n            message: \'Bindings must be an object or function.\',\n            url: \'marionette.functions.html#marionettebindentityevents\'\n        });\n    }\n    if (_.isFunction(bindings)) {\n        bindings = bindings.call(target);\n    }\n    _.each(bindings, function (methods, evt) {\n        if (_.isFunction(methods)) {\n            functionCallback(target, entity, evt, methods);\n        } else {\n            stringCallback(target, entity, evt, methods);\n        }\n    });\n}' }

i then set up a YAML template called function-definition.tpl that is a slugified version of the AST type FunctionDefinition it looks like this

<%- id %>
  description: | <% params.forEach(function(param) {%>
    @param {type} <%= param %> - <param description> <%}); %>

  examples:
    -
      name: Function Body
      example: |
        ```js
        <%= body %>

So if you didn't want the code in the example you would just leave out the <%= body %> tag. When you run the JSON through this template you get:

iterateEvents description: | @param {type} target - @param {type} entity - @param {type} bindings - @param {type} functionCallback - @param {type} stringCallback -

examples:

  name: Function Body
  example: |
    ```js
    {
if (!entity || !bindings) {
    return;
}
if (!_.isFunction(bindings) && !_.isObject(bindings)) {
    throw new Marionette.Error({
        message: 'Bindings must be an object or function.',
        url: 'marionette.functions.html#marionettebindentityevents'
    });
}
if (_.isFunction(bindings)) {
    bindings = bindings.call(target);
}
_.each(bindings, function (methods, evt) {
    if (_.isFunction(methods)) {
        functionCallback(target, entity, evt, methods);
    } else {
        stringCallback(target, entity, evt, methods);
    }
});

}

[ChetHarrison]

At this point you could augment the YAML with parameter descriptions and examples that can't realistically be interpolated from the AST. I may be able to type the params. I found a bug last night so I really should write some tests and coverage and find out where my blind spots are but at this point is pretty configurable.

The Parser API is a bit of a jungle when you first look at it but after looking at the ASTs Esprima produces side by side with the code, and knowing you can produce executable code from that AST you start to see the patterns. It is actually quite amazing how something seemingly so specific can be decomposed into something so general!

[ChetHarrison]

@jasonLaster Got some bugs. I'll let you know when it is working again.

[jasonLaster]

@ChetHarrison I think adding the source/body to the yaml will significantly increase the bloat of the yaml.

The point of pulling the documentation out of the source is to separate the two so that they're more readable. If we're adding the source - i'd argue we could write all our code yaml first :P

The big difference between the json and the yaml is that the json will be the built outproduct of jsdoc. In the future I'd like to also pull the unit tests for functions into the json as well.

---

By the way, I really like where this is headed. The configuration is really great. It's smart to start with the configuration because inevitably this tool will grow and always need to be more flexible.

You're right - the AST work really shows how languages are built up and are naturally recursive and composable. Really cool stuff.

[ChetHarrison]

@jasonLaster Thanks. It has been an illuminating processes. I found a way to make it even more configurable so you can have a set of sensible defaults like document my functions but then you can get to the level of detail you my need in your specific project. For example Marionette does a lot of Backbone extensions like:

Marionette.AppRouter = Backbone.Router.extend({...});

Now that is something that happens frequently in Marionette but not that often in JS. So It would be nice to be able to custom template up how you want to handle that scenario.

As far as the code in the yaml I will yank it from the template and were done! It will already grab any block of code to ask for in the config, functions, function bodies, etc., and put it in the target JSON. It's up to you if you want to reference it in the YAML template.

[ChetHarrison]

So the quickest way to id those custom conditions is to look at the saved AST copy what ever piece of the JSON contains target conditions and past that into your config. It do this

var x = JSON.stringify(AST);
if (x.indexOf(targetAstJsonString) > -1) { // template your custom YAML };

That way you don't have to write or even think about what all those conditions need to be but you will now what values will be available to you in the returned portion of the syntax tree. It's not like you really need to filter the stuff you don't need when the YAML template essentially does that anyway.

[jasonLaster]

@ChetHarrison - I really like the project level customization. Have a template for certain scenarios like extend.

[ChetHarrison]

@jasonLaster yeah. I need your brain. I am chasing my tail bit. In the config I could to the indexOf comparison of the the target string to the stringifyed AST JSON but it makes for a nasty super error prone config because the config is JSON and trying properly to escape and indent JSON blocks as the value of a JSON attribute is enough to drive your crazy on the most trivial of cases a la

var config = {
   targetConditions: "\"type\": \"FunctionDeclaration\""
}

when it's way more explicit to use the damn JSON

var config = {
   targetConditions: "ast.type === 'Property' && ast.value.type = 'FunctionExpression'"
}

but then your setting yourself up for an

if (eval(config.targetConditions)) { // found target syntax }

and if using eval isn't hacky enough evaluating config is just irresponsible.

Any thoughts?

[ChetHarrison]

I have a few ideas on how to get around this. A few hours without a keyboard can solve many problems ...

[samccone]

:clap: @ChetHarrison

[jasonLaster]

sounds hairy. Let me know what you come up with. will be on gitter tomorrow

[ChetHarrison]

thanks @samccone I am going to hit this hard today and see if I can wrap it up.

[jasonLaster]

:boom:

[ChetHarrison]

@jasonLaster I am going to prototype a few ideas and see what smells good.

[ChetHarrison]

Ok @jasonLaster @samccone kicking ass today. I made a separate file that exports a config object with a syntax type attr and an associated AST test. So it walks the AST and checks for a match at each node. Then I have a totally predictable block of AST and it gets pushed in order. the AST to YAML phase pops each collected type/ast-block pair looks up it's template and :boom:

Here is a sample config object

module.exports = {
        methods: function(ast) {
            return ast.type === 'Property' &&
                ast.value.type === 'FunctionExpression';
        },

        functions: function(ast) {
            return ast.type === 'FunctionDeclaration';
        }
    };

The beauty is that now I can use the JSON.stringify with indexOf trick as a matcher or regex or whatever you want. I still need to save the YAML to files and convert this to Grunt task. But at least now I'm moving from code to config.

[ChetHarrison]

I gotta run out for a bit but will try wrap up some loose ends tonight

[samccone]

here is a :boom: for me being excited about this

[ChetHarrison]

@samccone @jasonLaster ok this is, as we say in Cali "Totally Bitchen", and in Boston "Wicked Awesome" (see how I just i18n that). We can now parse the AST JSON for syntax targets we want to doc or we can copy and paste JSON.stringifyed text from the saved AST as matchers. Right now it's relatively messy. You need to concat multilines with line breaks but I think a little util function may make that painless. (probably should have been in Underscore).

So here is the current syntax-to-document.js:

module.exports = {
        // General documentation cases.
        methods: function(ast) {

            return ast.type === 'Property' &&
                ast.value.type === 'FunctionExpression';
        },

        functions: function(ast) {

            return ast.type === 'FunctionDeclaration';
        },

        // Custom Backbone.Marionette cases.

        // TODO: Indentation matters so pull indentation 
        // style from config and create tool for multiline
        // strings like PHP """.
        applications: function(ast) {
            var target = 
                '{\n' +
                '  "type": "Program",\n' +
                '  "body": [\n' +
                '    {\n' +
                '      "type": "ExpressionStatement",\n' +
                '      "expression": {\n' +
                '        "type": "AssignmentExpression",\n' +
                '        "operator": "=",\n' +
                '        "left": {\n' +
                '          "type": "MemberExpression",\n' +
                '          "computed": false,\n' +
                '          "object": {\n' +
                '            "type": "Identifier",\n' +
                '            "name": "Marionette"\n' +
                '          },\n' +
                '          "property": {';

            return _parseAstSubString(ast, target);
        }
    };

So in the yaml/mariontette.application.yaml files ...

name: Application

initialize
  description: | 

  examples:
    -
      name:
      example: |

execute
  description: | 

  examples:
    -
      name:
      example: |

request
  description: | 

  examples:
    -
      name:
      example: |

addInitializer
  description: | 
    @param {type} initializer - <param description> 

  examples:
    -
      name:
      example: |

start
  description: | 
    @param {type} options - <param description> 

  examples:
    -
      name:
      example: |

addRegions
  description: | 
    @param {type} regions - <param description> 

  examples:
    -
      name:
      example: |

emptyRegions
  description: | 

  examples:
    -
      name:
      example: |

removeRegion
  description: | 
    @param {type} region - <param description> 

  examples:
    -
      name:
      example: |

getRegion
  description: | 
    @param {type} region - <param description> 

  examples:
    -
      name:
      example: |

getRegions
  description: | 

  examples:
    -
      name:
      example: |

module
  description: | 
    @param {type} moduleNames - <param description> 
    @param {type} moduleDefinition - <param description> 

  examples:
    -
      name:
      example: |

getRegionManager
  description: | 

  examples:
    -
      name:
      example: |

_initializeRegions
  description: | 
    @param {type} options - <param description> 

  examples:
    -
      name:
      example: |

_initRegionManager
  description: | 

  examples:
    -
      name:
      example: |

_initChannel
  description: | 

  examples:
    -
      name:
      example: |

[ChetHarrison]

I still need to Gruntify this.

[ChetHarrison]

the name: Application at the top was pulled from a string matcher.

[ChetHarrison]

Give me examples of what you want to doc an I will see if I can pull it out of the AST metadata.

[samccone]

woooooooo great stuff, @ChetHarrison perhaps run it against the classes on minor and just open a PR with the fixes?

[ChetHarrison]

@samccone yeah that makes sense. I don't want to clobber the classes that already have documentation. Do you just plan to diff your way through those? I am not sure what you mean about "minor". I am going to add some more cases to the config working from the the documented YAML as examples. Probably have something solid in a few hours.

[jasonLaster]

@ChetHarrison appRouter and module are the two classes without docs. callbacks, renderer, domrefresh, and templatecache could all use it as well.

I think the biggest wins for our project will come from the linting ability. You should now be able to run jsdoccer against our classes and compare our yaml write-up to your output and tell us if we're missing a function or a param or if it's out of order.

Secondly, we'll want to be able to run grunt api and get the function body and sources in the json.

so yea three purposes:

1. build for new files w/o yaml
2. lint
3. source for grabbing function bodies when you run grunt api and adding to the json.

---

also when it comes time to add to grunt, we can either create a new task(s) or add it to jsdoc. I'm not really a grunt pro so I'd reach out to @marionettejs/marionette-core for guidance as well.

[ChetHarrison]

@jasonLaster ok. Let's do this. I will ping you and @samccone around 3PM your time. I will push the yaml results to the JSDoccer repo, in the yaml dir. I will run it on all the Marionette classes so you can QA. If it looks good we can form a plan on how best to integrate it.

Besides adding 10 loc to RxJs this is really the first OS project I have contributed to so I am not much of a git hot shot. I just want to make sure I don't overwrite progress!

[jasonLaster]

sounds great @ChetHarrison. Can't wait to get your work in.

[jamesplease]

I can make the Grunt task for this if you'd like @ChetHarrison.

[ChetHarrison]

@jmeas I think you could do that in a fraction of the time it would take me as I have yet to make one. Then I could mince your shallots in a fraction of the time it would take you.

[jamesplease]

ahaha. sounds like a deal. :+1:

[ChetHarrison]

@jasonLaster @jmeas @samccone I pushed the latest to https://github.com/ChetHarrison/jsdoccer

I ran it on all the classes. So far it is grabbing modules, classes, methods (with parameters, with/without returns) functions (with parameters, with/without returns), and events. I need to add extensions and properties. I need to run some errands but please check the results in the yaml dir. I'll be back in 3 hours or so.

Thanks

[jasonLaster]

@ChetHarrison - module and appRouter look good. I'm on the fence over having a blank example in the template by default. When you get back, create a PR with those two class yaml files into minor and we'll get them in.

[ChetHarrison]

@jasonLaster ok. I'm back will do now.

[ChetHarrison]

@jasonLaster Ok you might want to check and make sure my PR hit the minor

[ChetHarrison]

If you would like headings for classes of syntax ie events etc. I could make a master template and plug the results of each type in that.

[ChetHarrison]

@jasonLaster hang on. I am an idiot I should have run the yaml linter before the PR. Bad Chet. I think me events are breaking it.

[ChetHarrison]

hmmm wierd. When I paste my yaml from jsdoccer into my sublime editor with Marionette up and press save it deletes the white space indentations. I should probably check my prettifier settings.

[ChetHarrison]

@jasonLaster @samccone @jmeas Ahh ok so in your .editorconfig you have trim_trailing_whitespace = true not so cool for yaml!

[ChetHarrison]

I'll grab the minor and PR tonight.

[jasonLaster]

Good catch. I think this kicker @thejameskyle s ass before.

[ChetHarrison]

Marionette has a few approaches to each class's syntax. I am getting a much tighter fit on the various flavors. Currently I am generating a flat collection of syntax targets to document but I think it would be more natural to nest targets because that is how code is written. What would you prefer?

Example

functions
    A
    B

events
   C
   D

or

functions
   foo

   events
      C
      D

would you like to have common syntax targets categorized or match the order of the code.

[jasonLaster]

@ChetHarrison - I thnk you're geting at two things: 1) a list of class events and 2) where each event is fired.

For the second, we'll have comments in the code pointing to the events where they are fired. This hasn't been done yet - but here's the issue that talks about it https://github.com/marionettejs/backbone.marionette/issues/1973

[ChetHarrison]

@jasonLaster take a look at https://github.com/ChetHarrison/jsdoccer/blob/master/yaml/application.yaml

I have adopted a "flat" categorized approach to the classes per @jmeas suggestion. So rather than associating an event with the function that triggers it, every class gets an (in order) list of {module, constructor, properties, functions, events}. Not a big deal to change I have it pretty much down to tweaking config files.

[ChetHarrison]

I was looking at your 'api' grunt task. I wanted to past that code into the doccer so I could test the yaml output and make this into a task too. Is it taking yaml and creating json? Kinda funny cause I am taking json and creating yaml.

[ChetHarrison]

@jasonLaster Ha! I guess it is. I just strapped it in. So from the JSON do you plan to generate markdown or do you have another task to produce html?

[ChetHarrison]

cool passed the yaml linter

[ChetHarrison]

results here https://github.com/ChetHarrison/jsdoccer/tree/master/jsdoc

[samccone]

moving to THE JSDOCCER issue #1756