csnover / js-doc-parse

An experimental library for parsing JavaScript files and extracting inline documentation.
31 stars 7 forks source link

Incorrect contamination of API doc by properties of superclasses added by unrelated classes #52

Closed AdrianVasiliu closed 12 years ago

AdrianVasiliu commented 12 years ago

We have a number of occurrences of classes (for instance in dijit and dojox) where a module adds properties to some class with code such as:

lang.extend(dijit._WidgetBase, {
   alt: "",
   ...            
});

Now, the effect on the API doc is that the doc of completely unrelated classes wrongly shows that they would have these properties, if and only if these unrelated classes happen to be parsed by js-doc-parse after the class which extends its baseclass.

An example of this situation: the API doc of dojox/treemap/TreeMap shows the properties added to dijit/_WidgetBase by the completely unrelated dojox/mobile/Accordion, dojox/mobile/ListItem, dojox/mobile/i18n, dijit/layout/StackContainer, dijit/layout/BorderContainer etc. These properties do NOT appear on the doc of other subclasses of _WidgetBase that are lexicographically located before the classes that add these properties.

There are two issues here: 1/ It does not seem expected that the API doc contains these added properties depending on the parsing order (which is the alphabetical one). 2/ Is there a recommended/standard way to tell the parser to ignore this call of dojo/_base/lang.extend for the purpose of the API doc? For now the only way we found is to use a single line construct such as:

/* ===== // ===== */ lang.extend(WidgetBase, { alt: "", ... });

(since we can't embed slash-star comments inside slash-star comments). But this solution is fragile and may give huge lines of code... So there should be a better way.

csnover commented 12 years ago

I’m not sure I understand. You shouldn’t extend WidgetBase if you don’t want it to show up in the docs. I do need to fix the output of docs to read from mixins directly, however.

AdrianVasiliu commented 12 years ago

Well, the fact is that there are dozens of modules in dijit and a couple in dojox.mobile which do extend WidgetBase to add properties or methods. To take one example among many, dijit._BidiSupport adds bidi-related APis to WidgetBase, documented as follows: "Including this module will extend _WidgetBase with BIDI related methods." And currently the doc of widgets which do NOT include this module unexpectedly shows the bidi-related APIs (example: dojox/treemap/TreeMap) Another example are the properties added by dijit/layout/*Container.

AdrianVasiliu commented 12 years ago

Concerning a way to tell the parser to skip a given piece of code, maybe it could be, for instance (just brainstorming):

/* ----- */
... some code that we want to be skipped by js-doc-parse...
 (this code can be single-line or multiline, and may contain /*...*/ comments itself)
/* +++++ */
kitsonk commented 12 years ago

This is causing me a bit of grief in the inlining of the API in the rstWiki. In my mind, there is a difference between "inherited" and "extended". If you look at dijit/_WidgetBase it is a mess, because of the number of things that tag onto it. For example dojox/layout/TableContainer adds "colspan". The logic for inherited should only be true if the it is in the declaration dependency chain, not an extension.

cjolif commented 12 years ago

@csnover I can understand your argument about not extending it if we don't want it to appear (even thought this is not always feasible). That's one thing. But the other thing is that there seem to be a bug in the sense that depending on the parsing order the property might appear or not on the class it is added to. I think we need at least to do something on that one.

wkeese commented 12 years ago

The way to "tell the parser to skip a given piece of code" varies by the piece of code, but in this case an easy way is:

lang.mixin(cls, /*===== {} || =====*/   {
      ... real props ...
});

The main problem documented in this ticket has been solved by modifying the dojo code in this way.

As for the second issue, I split it off from here, into #69.