Incorrect Line Numbers Reported

[l-cornelius-dol]

I am using Meriyah with GraalVM to validate Javascript source in my code editor. I am using the version from the dev branch from commit 96eb15f, dated 2019-08-21.

Given the code:

...
108    handleAddressSelected : function(adr) {
109        var prp                          this.props;
110
111       // Address selection puts the entire address into the target field value so address1 must always be changed.
112       //     e.g.: 6/19 Ashby Circuit, Kamhah ACT, Australia
113
114        if(adr.postcode!=prp.adrPostcode || adr.state!=prp.adrState) {                                                  
115            this.handleAddressChanged(prp.address1);                                                                    
116            }
117        else if(sta.showAddressLine2) {
118            this.handleAddressChanged(adr.street,adr.unit,adr.name,adr.citySuburb);
119            }
120        else {
121            this.handleAddressChanged((adr.unit ? adr.unit+"/" : "")+adr.street,"",adr.name,adr.citySuburb);
122            }
123        },

The line reported from Meriyah for the error is:

ERROR : C:/[...deleted...]/Contact.js:~: [121:45]: Unexpected token: 'this'

Which was immensely difficult to figure out.

After a long time I noticed the missing = at line 109. Then I used the browser to locate the error and it pointed to the this at line 109 also. Obviously this was made much more difficult by the fact that there was no reason to suspect the line number was incorrect. Since I've been using Meriyah for a while, I suspect that this was introduced in the last few weeks.

[l-cornelius-dol]

@KFlash I implemented this fix, and it still reports the incorrect line number (note that the column is correct, being the s of this at line 109). It seems that line parser has been modified because the current token is advanced before the error is detected and thrown.

[l-cornelius-dol]

I've found the problem. My file was saved using CRLF line breaks. When this is done, all line numbers are out, but not by the number of lines up until that point. So under some, but not all, conditions CR is adding to the line count.

[KFlash]

I'm not sure what's going on, but for every newline the line count is increased. That's normal. See here. And for CRLF same if CR was not found first. So there shouldn't be any issues here.
Regarding this. It's now automatically get it's line and column values right before the token is consumed and not passed down as an argument. So this should be correct.
Sure there are no issues in your end?

Your line 109 var prp this.props; isn't valid JS and not sure how you got that code? ASI isn't triggered in that case. You are missing a semicolon after prp.

[l-cornelius-dol]

@KFlash : Line 109 was a typo, and should've been var prp = this.props; but I'd accidentally omitted the equals and got the error in the OP. The problem being that Meriyah reported the wrong line number.

It's definitely not counting lines correctly if the file uses CRLF line-endings; I can switch between CRLF and LF and see that with the former the line number is consistently out, reporting 121 instead of 109 and with the latter it reports 109. I can go back and forth with the only variable being the file's line endings. I've done this with multiple files and multiple classes of errors.

With your fix implemented the same (incorrect) line number was still reported, hence it was not that the line number passed down shouldn't be used. And the problem was not limited to handling this.

With further digging I discovered that the line numbers associated with all parsed elements were incorrect when CRLF line-endings are used. It was a sheer fluke/inspiration that I noticed the CRLF line endings and thought to try changing it, and that only because I normally use LF.

[KFlash]

@lawrence-dol Still not sure what's going on here. I just ran a bunch of tests against Acorn and got the exact same results.
See this test file
https://github.com/meriyah/meriyah/blob/master/test/parser/miscellaneous/crlf.ts

[l-cornelius-dol]

I think the bit manipulation state |= 1 | 4;, which occurs twice, is wrong, since it equates to state = state | (1 | 4); => state = state | 0;, which has no effect. This means that the ensuing scanNewLine(parser); will almost certainly do the wrong thing, at least sometimes.

As an aside the bit manipulation state = (state | 4 | 1) ^ 4; seems convoluted to me (without thinking about it too hard). That switches bit 3 off and bit 1 on. Identical to state = (state | 1) & ~4; which to me is a clearer intent.

Lastly, and I didn't look at this closely, but on principle a CRLF should be consumed as a whole, so when an LF is processed, it should unconditionally clear bit 3 (state &= ~4;) since the next character could be a lone CR, as in CR LF CR CR LF.

[l-cornelius-dol]

@KFlash : I believe the test you referenced is invalid for this case, because it doesn't replicate the conditions required to trip the fault, which will be a specific sequence of CRLF. I will attempt to reproduce them, but I am sure it will be related to the failure to set bits 3 and 1 in the two places I pointed out.

[l-cornelius-dol]

The problem lies with single line comments. The line is out by one for every preceding SLC, whether or not they are on a line by themselves or after some code. Simplest replication:

 1    // Single line comment
 2    // Single line comment
 3    // Single line comment
 4    // Single line comment
 5    // Single line comment
 6    // Single line comment
 7    // Single line comment
 8    // Single line comment
 9    // Single line comment
10    // Single line comment
11
12    function handleAutocomplete() {
13        var prp                          this.props;
14        }

Script: C:/.../test.js
ERROR : C:/.../test.js:~: [23:41]: Unexpected token: 'this'

Alter the number of comments and the reported line changes.

[KFlash]

I'm working on it. Try to see if you see any differences with 1.6.12. I just pushed it to NPM.

[l-cornelius-dol]

1.6.13 seems to have resolved the problem.

[KFlash]

Good to know :)

[l-cornelius-dol]

Thanks for working with me on this. Apologies for the misleading bit about state |= 1 | 4; -- apparently my bit-fu is rusty.

[KFlash]

No problem. Issue was very easy, and the way you investigated and described things was very helpful :)