Comments are attached to the wrong node with starting function declaration

[mrennie]

Parse the following snippet with the online demo:

/***/function o() {/***/function f() {};};

For the first FunctionDeclaration the comment is attached to its id node (wrong) and for the second the comment is attached to the FunctionDeclaration node (as expected)

[mikesherov]

@mrennie, thanks for contributing! We'll be looking into this issue soon!

[mrennie]

Here is another simple example:

/**/function f() {var a;}

Looks like the attaching code is attaching the comments to the first 'finished' node rather than the node they appear immediately before.

Wouldn't it make more sense to attach comments when you start a node? that way we don;t have to keep a stack of nodes and attaching the comments becomes as simple as assigning arrays to the previous (trailing) and next node we are starting (leading).

[ariya]

cc @ikarienator, who also knows a lot about comment attachment.

[ikarienator]

Which version are you using?

on master, esprima.parse('/**/ function a() {}', {attachComment: true}) gives me:

{
    "range": [
        5,
        20
    ],
    "type": "Program",
    "body": [
        {
            "range": [
                5,
                20
            ],
            "type": "FunctionDeclaration",
            "id": {
                "range": [
                    14,
                    15
                ],
                "type": "Identifier",
                "name": "a"
            },
            "params": [],
            "defaults": [],
            "body": {
                "range": [
                    18,
                    20
                ],
                "type": "BlockStatement",
                "body": []
            },
            "rest": null,
            "generator": false,
            "expression": false,
            "leadingComments": [
                {
                    "type": "Block",
                    "value": "",
                    "range": [
                        0,
                        4
                    ]
                }
            ]
        }
    ],
    "comments": [
        {
            "type": "Block",
            "value": "",
            "range": [
                0,
                4
            ]
        }
    ]
}

[mrennie]

/**/ function a() {function o() {}}

Run on http://esprima.org/demo/parse.html produces:

{
    "range": [
        5,
        35
    ],
    "type": "Program",
    "body": [
        {
            "range": [
                5,
                35
            ],
            "type": "FunctionDeclaration",
            "id": {
                "range": [
                    14,
                    15
                ],
                "type": "Identifier",
                "name": "a",
                "leadingComments": [
                    {
                        "type": "Block",
                        "value": "",
                        "range": [
                            0,
                            4
                        ]
                    }
                ]
            },
            "params": [],
            "defaults": [],
            "body": {
                "range": [
                    18,
                    35
                ],
                "type": "BlockStatement",
                "body": [
                    {
                        "range": [
                            19,
                            34
                        ],
                        "type": "FunctionDeclaration",
                        "id": {
                            "range": [
                                28,
                                29
                            ],
                            "type": "Identifier",
                            "name": "o"
                        },
                        "params": [],
                        "defaults": [],
                        "body": {
                            "range": [
                                32,
                                34
                            ],
                            "type": "BlockStatement",
                            "body": []
                        },
                        "rest": null,
                        "generator": false,
                        "expression": false
                    }
                ]
            },
            "rest": null,
            "generator": false,
            "expression": false
        }
    ],
    "comments": [
        {
            "type": "Block",
            "value": "",
            "range": [
                0,
                4
            ]
        }
    ]
}

[ikarienator]

http://esprima.org/demo/parse.html is probably out dated. It's definitely not using master branch.

[ariya]

@ikarienator It may have something to do with the inner function declaration. Compare these two:

> require('./esprima').parse('/*abc*/ function a() {}', {attachComment: true}).body[0].leadingComments
[ { type: 'Block', value: 'abc', range: [ 0, 7 ] } ]

> require('./esprima').parse('/*abc*/ function a() { function p(){} }', {attachComment: true}).body[0].leadingComments
undefined

The first one is correct.

[mrennie]

For what its worth, I originally found the problem while working on Orion bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=460468

My branch has the Esprima code from almost the tip of master.

[mrennie]

Here is another interesting case from some of our test failures:

switch(a) {
  case 1: switch(b) {
    case 1: foo; 
    //$FALLTHROUGH$
    case 2: foo;
  }
}

The failure case here is that the comment is no longer attached to the SwitchCase, but is now attached to the test of the SwitchCase.

[ikarienator]

@mrennie Can you checkout master and try?

[mrennie]

The problems still exist using the very latest from master.

[ariya]

@mrennie I believe the comment should be attached to the test (i.e. the smallest node following the comment). There could be a defect there because (again) the nested SwitchStatement.

[jifeon]

Hi guys! Any news here?