esprima icon indicating copy to clipboard operation
esprima copied to clipboard

Dangling comment not attach to any ast node

Open w-y opened this issue 8 years ago • 4 comments

Steps to reproduce

esprima.parse('function foo() {return /*comment*/;}', { comment: true })

Expected output

{
  "type": "Program",
  "body": [
    {
      "type": "FunctionDeclaration",
      "id": {
        "type": "Identifier",
        "name": "foo"
      },
      "params": [],
      "body": {
        "type": "BlockStatement",
        "body": [
          {
            "type": "ReturnStatement",
            "argument": null,
            "danglingComments": ["comment"],  // attach to ReturnStatement ast node 
          }
        ]
      },
      "generator": false,
      "expression": false,
      "async": false
    }
  ],
  "sourceType": "script",
  "comments": [
    {
      "type": "Block",
      "value": "comment"
    }
  ]
}

Actual output

{
  "type": "Program",
  "body": [
    {
      "type": "FunctionDeclaration",
      "id": {
        "type": "Identifier",
        "name": "foo"
      },
      "params": [],
      "body": {
        "type": "BlockStatement",
        "body": [
          {
            "type": "ReturnStatement",
            "argument": null
          }
        ]
      },
      "generator": false,
      "expression": false,
      "async": false
    }
  ],
  "sourceType": "script",
  "comments": [
    {
      "type": "Block",
      "value": "comment"
    }
  ]
}

Here "comment" become a "dangling comment".

Relevant references

ReturnStatement:
    return ;

In this case, it's true that "comment" is out of both the leading and trailing range of ReturnStatement according to the specification but it will be more convenient if this comment be attached to the ast node.

w-y avatar Nov 03 '17 11:11 w-y

Just to confirm, did you actually mean this one?

esprima.parse('function foo() {return /*comment*/;}', { attachComment: true })

Without attachComment (i.e. just comment), every comment won't be attached to the node.

ariya avatar Nov 06 '17 04:11 ariya

@ariya

I tried attachComment, comment and both.

esprima.parse('function foo() {/*good comment*/return /*bad comment*/;}', { attachComment: true, comment: true })

the result is: (version 4.0.0)

{
  "type": "Program",
  "body": [
    {
      "type": "FunctionDeclaration",
      "id": {
        "type": "Identifier",
        "name": "foo"
      },
      "params": [],
      "body": {
        "type": "BlockStatement",
        "body": [
          {
            "type": "ReturnStatement",
            "argument": null,
            "leadingComments": [
              {
                "type": "Block",
                "value": "good comment",
                "range": [
                  16,
                  32
                ]
              }
            ]
          }
        ]
      },
      "generator": false,
      "expression": false,
      "async": false
    }
  ],
  "sourceType": "script",
  "comments": [
    {
      "type": "Block",
      "value": "good comment"
    },
    {
      "type": "Block",
      "value": "bad comment"
    }
  ]
}

good comment is attached while bad comment is not.

w-y avatar Nov 06 '17 06:11 w-y

Hi @w-y, unfortunately this is the expected behavior due to the limitation of the logic in the comment attacher. In your case, bad comment would have been attached to the argument of return. But, since return has no argument (it is null in the AST), then the comment is not attached anywhere. It can not be attached to the return statement either because the comment appears before the semicolon.

It is not likely that this corner case is going to be tackled. In fact, the whole comment attachment feature is not necessarily fully supported. But do not despair! It is very easy to come up and plug in your own attachment logic. Take a look at the source code, in particular src/comment-handler.ts.

ariya avatar Jun 10 '18 22:06 ariya

@ariya How hard would it be to keep a running list of unattached comments and add them to whichever node is finalised next?

michaelficarra avatar Jun 11 '18 05:06 michaelficarra