jquery / esprima

ECMAScript parsing infrastructure for multipurpose analysis
http://esprima.org
BSD 2-Clause "Simplified" License
7.05k stars 787 forks source link

Negative column with multi-line string literal #2032

Closed lahma closed 4 years ago

lahma commented 4 years ago

When parsing a multi-line string literal that has windows line feeds (\r\n) a negative column number is produced.

Steps to reproduce

var tree = esprima.parseScript("var str = 'test \\\r\nd'+'test \\\r\ntest'+'f';", { loc: true });
tree["body"][0].declarations[0].init.left.loc
JSON.stringify(tree["body"][0].declarations[0].init.left.loc)

Expected output

{"start":{"line":1,"column":10},"end":{"line":3,"column":5}}

Actual output

{"start":{"line":1,"column":-9},"end":{"line":3,"column":5}}

Relevant references

I see there's been such problem before too: https://github.com/jquery/esprima/issues/1844

lahma commented 4 years ago

I think the problem is here:

https://github.com/jquery/esprima/blob/d277380c4cf7a2ff3ba3499b6d82b126c9888d4a/src/parser.ts#L1514-L1521

This code path doesn't check for the popped marker and use its lineStart as lastLineStart for startNode call like the code below it does:

https://github.com/jquery/esprima/blob/d277380c4cf7a2ff3ba3499b6d82b126c9888d4a/src/parser.ts#L1536-L1544

I see that tests are explicitly checked not to contain windows line feeds so I'm unsure how to create a PR for this.

lahma commented 4 years ago

Created PR #2033

lahma commented 4 years ago

ping @ariya 🙂

KFlash commented 4 years ago

There already exist issues and PR"s created years back that he haven't responded too yet. Expect him to be a slow replier.

lahma commented 4 years ago

And it's all fine, I understand that when it's free work one can choose his priorities. I just have some wishful thinking to verify the approach to get that on .NET version's side where we actually throw an error when we encounter invalid index values.