Skip to content
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

Comment ranges at wrong location #104

Closed
neumatho opened this issue Oct 12, 2020 · 5 comments
Closed

Comment ranges at wrong location #104

neumatho opened this issue Oct 12, 2020 · 5 comments

Comments

@neumatho
Copy link

Try to run the following code in e.g. RunKit:

var meriyah = require("meriyah")
var escodegen = require("escodegen")
var esprima = require("esprima")

var code = `try{}//
finally{}

try{}
catch(e){}//
finally{}

{
try{}
catch(e){}//
finally{}
}
`;

var merComments = [];
var merTokens = [];
var merTree = meriyah.parse(code, {
    ranges: true,
    onComment: function(type, value, start, end) {
        type = type === 'SingleLine' ? 'Line' : 'Block';
        merComments.push({type: type, value: value, range: [start, end]});
    },
    onToken: function(type, start, end) {
        merTokens.push({ type: type, range: [start, end]});
    }
});

merTree = escodegen.attachComments(merTree, merComments, merTokens);
var merCode = escodegen.generate(merTree, { comment: true });

var espriTree = esprima.parse(code, { comment: true, range: true, tokens: true });
espriTree = escodegen.attachComments(espriTree, espriTree.comments, espriTree.tokens);
var espriCode = escodegen.generate(espriTree, { comment: true });

[merComments, espriTree.comments, merTree, espriTree, merCode, espriCode]

If you then compare the ranges in the comment array for both Meriyah and Esprima, then you will see the values are not the same (start is 2 under, end is 1 under). If you then look at the generated code, which are generated from the Meriyah and Esprima tree and compare them, then you will see the comments are placed at the wrong places from the Meriyah tree.

Generated code from Esprima tree

try {
}    //
finally {
}
try {
} catch (e) {
}    //
finally {
}
{
    try {
    } catch (e) {
    }    //
    finally {
    }
}

Generated code from Meriyah tree

try {
} finally
    //
    {
    }
try {
} catch (e) {
}    //
finally {
}
{
    try {
    } catch (e) {
    }    //
    finally {
    }
}

-Thomas

@KFlash
Copy link
Contributor

KFlash commented Oct 12, 2020

@3cp Didn't you solve this before so this could work with ESCodegen?

@3cp
Copy link
Member

3cp commented Oct 12, 2020

It was #83. Fixed start of non-comment token.

I will have a look of comment, the range of comment is not constructed by a token.

I think I noticed something wrong with comment before. But didn't pay too much attention because it didn't affect my use cases.

@3cp
Copy link
Member

3cp commented Oct 13, 2020

We set start for comment after // or /*, while other parsers set start before them. There is also difference on the ending \n. I see ESTree has no requirement on comment token/node, I can update our implementation to follow other parsers.

But this is not the only issue here. In our ast tree, the finally block (in ast node, not the onToken) has wrong start, it should start at { after keyword finally, not before it. I am going to fix finally block and catch block.

@KFlash why we have html comment? for jsx? But why comment #2 is counted as a comment token?

  it('should extract html comments in array', () => {
    const arr: any[] = [];
    parseScript('<!--comment #1\n--> comment #2', {
      onComment: arr,
      webcompat: true
    });
    t.deepEqual(arr, [
      {
        type: 'HTMLOpen',
        value: 'comment #1\n'
      },
      {
        type: 'HTMLClose',
        value: ' comment #2'
      }
    ]);
  });

@KFlash
Copy link
Contributor

KFlash commented Oct 13, 2020

HTML comment is part of the AnnexB extension. Same as other parsers. HTML comment skips either <!-- or --> and then parsed as a single line comment.

@3cp
Copy link
Member

3cp commented Oct 14, 2020

That's a weird spec, it's different from comment in html format. Since the spec says

SingleLineHTMLCloseComment::
LineTerminatorSequenceHTMLCloseComment

I will make the start of HTMClose comment before "\n-->"

// For Single, start before "//",
// For HTMLOpen, start before "<!--",
// For HTMLClose, start before "\n-->"

@3cp 3cp closed this as completed in fe00a67 Oct 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants