#889 In Javascript methods are not recognized.

v1.23
closed-fixed
Filetypes (68)
5
2012-09-21
2012-09-20
Oleg Eterevsky
No

Version of Geany: 0.21
OS: Ubuntu Precise

In JavaScript one of the common styles (http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml#Method_definitions) recommends using the following code to define the class methods:

function MyClass() {
}

MyClass.staticMethod = function() {
};

MyClass.prototype.instanceMethod = function() {
};

When editing such code in Geany, these methods appear in class browser, but not in the classes where they belong, but in the section Functions.

Discussion

  • Lex Trotman
    Lex Trotman
    2012-09-21

    The problem is that Javascript does not have syntactic definition of classes, everything is (potentially) a class. It is only programming convention that uses some functions as classes. Since the Javascript parser can't guess what was the programmer's intention, it appears to use some heuristics to decide that a function should also be considered a class.

    Your example gives a good example, the use of the prototype on MyClass is what makes the parser guess it might be intended to be treated as a class. As you show, that can occur after other functions that might be considered to be methods. Since it can't go back and re-parse based on this new information, it can't change staticMethod to a method below the class. Although instanceMethod could be attached to the class that would mean that "methods" are split into two places, so I think its better that none of them appear below the classes.

     
  • Oleg Eterevsky
    Oleg Eterevsky
    2012-09-21

    Thanks for the quick reply!

    I often have to work with JS files that are several thousands lines long and for them the class browser is not as useful as it could be. The static functions, static methods, instance methods and nested functions (which are common for asynchronous code) are all listed in one flat list. I understand that you might have problems adding static (non-prototype) methods to a class, but it's probably ok, since static methods are usually just basic functions, bound to the class only lexically, for the readability purpose. In my opinion, it would really help if you move to the class at least the prototype methods.

    That said, it's obviously up to you to decide, which way is best.

    [By the way, it would also help to put the nested functions under the function where they are defined, but it is a separate issue.]

     
    • labels: 1084466 --> Filetypes
    • milestone: --> v1.23
    • assigned_to: nobody --> colombanw
    • status: open --> closed-fixed
     
  • Displaying as a tree looks reasonable to me. Of course this won't be perfect since the parser has plenty of bugs (patches most welcome), but it might indeed help and some not-too-complex files actually get correctly parsed.

    I enabled scoping in Git (actually it was easy), so in your example you'll see "instanceMethod" as part of the "MyClass" class. However the parser recognizes the other two as functions so they will show as such. This will work with any nesting already recognized by the parser, including functions nested in other functions (no, that's not another subject :)).

    If this isn't enough to address your issue, please tell us. However it's unlikely I would really fix the JavaScript parser myself, because I hardly know JavaScript and touching this parser gives me headaches (already trying to fix another bug in it, still no luck without breaking an unrelated thing…). BTW, this parser comes from http://ctags.sourceforge.net/, and although this project is almost dead you may be extremely lucky at poking somebody there.

     
  • BTW, @elextr: if this actually makes things messier, we can revert it anyway. At least we'd have tried.

     
  • Mike D
    Mike D
    2012-09-23

    Perhaps a special string could be placed in a comment, and the next defined symbol is treated as a class definition? It would seem there is a need for some new syntax/hinting to explicitly define what is a class.

    Heuristics are nice, but as stated they are only guesses. Maybe the heuristics should be configurable to turn on/off, for example when 'prototype' member is used in non-class context (although this is likely rare)

     
  • Oleg Eterevsky
    Oleg Eterevsky
    2012-09-23

    @mrd_ There is such a syntax. It is called JSDoc (http://code.google.com/p/jsdoc-toolkit/).

    @colombanw Thanks a lot! I haven't yet looked at how it works now, will do it tomorrow and leave a comment here.

     
  • Lex Trotman
    Lex Trotman
    2012-09-24

    @mrd_ that is a feature request, please don't hijack bug threads, suggest you want to discuss it first using the ML, the trackers are a bit clunky for discussion purposes and not many monitor them so you won't get input from many JS users.

    Having the parser rely on comments for guidance will only work with those javascript codes written with the comments, heuristics will work on all codes. Depends on what percent use the comments.

     
  • Oleg Eterevsky
    Oleg Eterevsky
    2012-09-24

    I've looked at the ClassBrowser after your fixes. It became much better, the usual methods are recognized and listed under the appropriate class.

    There remain some more specific issues. First of all, the nested functions are still listed flatly in functions section. For example, for

    function MyClass() {
    }

    MyClass.prototype.method2 = function() {
    function nestedFunction1() {
    };
    };

    in the left panel I see

    - Classes
    -- MyClass
    --- method2

    - Functions
    -- MyClass.function.nestedFunction1

    It would be better if this function were under method2, i.e.:

    - Classes
    -- MyClass
    --- method2
    ---- nestedFunction1

    Secondly, sometimes in JavaScript a class is defined in the namespace of another class, for example:

    function MyClass2() {
    };

    MyClass2.SubClass = function() {
    }

    MyClass2.SubClass.prototype.method4 = function() {
    };

    Such classes are correctly recognized as classes, not functions, but their methods are not recognized. So for this example I get:

    - Classes
    -- MyClass2
    -- MyClass2.SubClass

    - Functions
    -- MyClass2.SubClass.method4

    while I would expect to see

    - Classes
    -- MyClass2
    -- MyClass2.SubClass
    --- MyClass2.SubClass.method4

    I have a small js file with these examples and some more, but I do not seem to be able to attach it to this bug.

    Also, if you wish, I can extract these issues and file two separate bugs.

     
  • > There remain some more specific issues. First of all, the nested
    > functions are still listed flatly in functions section. For example, for
    > [...]

    No, functions inside functions are generally parsed correctly, your example shows a separate bug of the JavaScript parser: it adds a "function" in the scope of the nested function for no good reason (MyClass.*function*.nestedFunction1). If the parse had properly generated "MyClass.method2.nestedFunction1" it'd have been correctly nested. So yes it's a bug, but not the same.

    > Secondly, sometimes in JavaScript a class is defined in the namespace
    > of another class, for example:
    > [...]

    This one should be fixed; the parser didn't generate proper scope for some tags but simply put the scope in the name, which prevented proper "tree-zation" of them.

    > Also, if you wish, I can extract these issues and file two separate bugs.

    Yes please. Though, I'd rather not see any more bug report against the JavaScript parser (or with appropriate patches) :-'

     
  • Oleg Eterevsky
    Oleg Eterevsky
    2012-09-24

    Ok, subclasses work fine, thanks.

    Functions inside functions also are recognized, but functions inside methods are not. I guess the bug is in makeJsTag in tagmanager/ctags/js.c. I'll try to fix it and send you the patch.