New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dangling comment not attach to any ast node #1874

Open
w-y opened this Issue Nov 3, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@w-y

w-y commented Nov 3, 2017

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.

@ariya

This comment has been minimized.

Show comment
Hide comment
@ariya

ariya Nov 6, 2017

Contributor

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.

Contributor

ariya commented Nov 6, 2017

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.

@w-y

This comment has been minimized.

Show comment
Hide comment
@w-y

w-y Nov 6, 2017

@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 commented Nov 6, 2017

@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.

@ariya

This comment has been minimized.

Show comment
Hide comment
@ariya

ariya Jun 10, 2018

Contributor

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.

Contributor

ariya commented Jun 10, 2018

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.

@michaelficarra

This comment has been minimized.

Show comment
Hide comment
@michaelficarra

michaelficarra Jun 11, 2018

Contributor

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

Contributor

michaelficarra commented Jun 11, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment