Patch: Remove limitation to one function parameter in onFunctionCall

[GoogleCodeExporter]

Plugins using onFunctionCall are very useful, yet they can only get the
first function parameter. The patch attached removes this limitations and
sets properties functionCall.arg1, functionCall.arg2, ... - as many as
necessary.

It has to save and restore TokenStream cursor "manually", this is somewhat
ugly. I didn't see a better way to do it however.

Original issue reported on code.google.com by wladi...@palant.de on 8 Jul 2008 at 5:11

Attachments:

• remove_parameter_limit.patch

[GoogleCodeExporter]

I also found that the parameter parsing is wrong in most cases. onParamList is 
meant
for a list of names, it won't work properly for more complicated parameters 
(strings,
objects etc). I ended up with the following initialization for the functionCall
parameter:

                var cursor = this.ts.cursor;
                var tokens = this.ts.balance("LEFT_PAREN");

                // Remove parentheses from the token list
                tokens.shift();
                tokens.pop();

                var argIndex = 1;
                var bracketLevel = 0;
                functionCall["arg" + argIndex] = "";
                for (var i = 0; i < tokens.length; i++) {
                    var token = tokens[i];
                    if (bracketLevel == 0 && token.is("COMMA")) {
                        ++argIndex;
                        functionCall["arg" + argIndex] = "";
                        continue;
                    }

                    if (token.is("LEFT_CURLY"))
                        bracketLevel++;
                    else if (token.is("RIGHT_CURLY"))
                        bracketLevel--;

                    if (!token.is("JSDOC"))
                        functionCall["arg" + argIndex] += token.data;
                }
                this.ts.cursor = cursor;

Original comment by wladi...@palant.de on 7 Oct 2008 at 2:27

[GoogleCodeExporter]

I see what you are trying to do, and I agree that the limitation of a single 
argument is arbitrary, but the 
onfunctioncall hook was implemented solely to handle the dojo.define() syntax, 
and in that case we only need 
the first argument. Is there an actual use case where more arguments are needed?

see: trunk/jsdoc-toolkit/app/plugins/functionCall.js

And the main reason the onfunctioncall hook is so limited is because JavaScript 
allows so much flexibility in 
what can be passed into a function call. Practically speaking, almost any code 
is syntactically permitted:

doSomething(
    doSomethingElse(
        function(x, y) {
            // this is a callback, it will be fired by doSomethingElse
            var a = y || 1,
                   b = false;
            this.z = x/a;
            return b;
        },
        "window"
    )
    + (0XFFFFF || /abc/.test("123"))
);

Trying to abuse onParamList to handle something like that will not surprisingly 
fail, but onParamList is not 
intended to parse *arguments* (the values being passed into a function call), 
it's intended to parse 
*parameters* (the names of the arguments listed in the function definition). 
Like so:

function doSomething (myParameter) {
}

Which is a much simpler task. A complete solution to an onArgumentList function 
would be so complex (and 
therefore slow) it would make the entire application unmaintainable and likely 
unusable. The work involved in 
doing this correctly is disproportionate to the actual benefit, in my opinion.

This is particularly true when it is considered that the ability to handle 
things like dojo.declare in merely a 
convenience and is never completely necessary -- there is always another 
(albeit more verbose) way to 
document a class using the @name tag and explicitly listing each @param. 

Thanks you for this but I'm gonna opt for the simpler long-term route of 
keeping the onfunctioncall hook as 
it is.

Original comment by micmath on 19 Jan 2009 at 6:11

• Changed state: WontFix

[GoogleCodeExporter]

Right now, onFunctionCall is used to process calls to two functions in our 
codebase.
One is |extend(SubClass, SuperClass)| which is how we generally mark 
subclasses. The
other generates simple classes on the fly:

createItemSubType("NewClass", {
    classParam1 : "foo",
    classParam2 : "bar"
}, SuperClass);

Both are cases that we want to be documented automatically. The code in comment 
1
works fine with both (it should be pretty generic as well). So I hope you 
reconsider.

Original comment by wladi...@palant.de on 21 Jan 2009 at 12:28

[GoogleCodeExporter]

Since it's backwards-compatible, and you accept the limitations of what 
arguments it can handle, I don't see why 
not.

Committed revision 770.

Thanks for your contribution.

Original comment by micmath on 21 Jan 2009 at 10:46

• Changed state: Fixed
• Added labels: Priority-Low, Type-Enhancement
• Removed labels: Priority-Medium, Type-Defect