estraverse icon indicating copy to clipboard operation
estraverse copied to clipboard

estraverse.attachComments attaches root comment to wrong node

Open mrennie opened this issue 11 years ago • 2 comments

Consider the following snippet:

/**
 * 
 */
function f1(o1, o2, o3) {
    return; 
};

If you call estraverse.attachComments on the AST for it, the block comment is attached to the 'Program' node and not the 'FunctionDeclaration' node as you would expect.

I'm assuming this is due to the fact that the 'Program' and 'FunctionDeclaration' start range is the same.

mrennie avatar Dec 17 '13 19:12 mrennie

The behavior you explained is right. But this is the expected behavior of attachComments. attachComments follows the simple rule: attachComments attempts to attach a comment to a node that is closest to the root node. For example,

function a() {
  // comment
  a + b;
}

Line comment // comment will be attached to a + b expression statement, not Identifier a.

This is the consistent behavior and we can easily expect the result if we know that rule. And if we need to extract comment for some nodes, we can search parent nodes that node.range[0] equals to the current.range[1]. Heuristics is very useful, but it sometimes produces the result we cannot expect.

But you're right, providing more useful attaching rule as options is useful for users. So I think providing sticky to the statement rule is nice. A comment is propagated to the root node while parent.range[0 or 1] === comment.range[0 or 1], but if a statement is found, a comment is attached to it. What do you think about this plan?

Constellation avatar Dec 17 '13 20:12 Constellation

I think it will work nicely. I could also work around the problem by checking the ancestor as you mentioned.

For some context. the use-case here is an eslint rule that checks for un-documented function declarations in Orion: https://bugs.eclipse.org/bugs/show_bug.cgi?id=424128

mrennie avatar Dec 17 '13 21:12 mrennie