-
Notifications
You must be signed in to change notification settings - Fork 46
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
Incorrect line number reported when file has LF for line separator #46
Comments
@lawrence-dol I'm not online the next days due to RL stuff. Could you try to experiment with it and try to send a PR? It should be an very easy change. |
@KFlash : I'd like to, but I am not set up with the tool chain for developing TS, so pragmatically, I've no realistic path to testing any changes. I could experiment with the generated JS, but that won't be useful as a direct PR, only as a reference as to what to change. |
A reference is okay. Then I can do the changes when I get back online. Maybe late sunday. Depends if there is any plane delays. |
@KFlash : The following changes seem to be working as expected. The problem was the special handling LF in comments. But please note that was expecting to find the tracking of current line and column at a single, high-level location -- so it's very possible that the problem is resolved for comments but there could still be similar problems elsewhere. First I modified
I renamed
Next I changed the main state handling for line breaks to reflect this abstraction:
And now comment handling for line breaks was much simpler:
And that seems to have resolved the problem. |
is this still an issue? |
@KFlash : Based on the current version from unpkg, 1.7.2, yes it is still an issue. None of these changes are present in 1.7.2. (I am running a version of 1.7.2 which I modified as shown above.) |
@KFlash : Hi there. Is there any update on this? |
AH! Sorry. Been busy with some RL stuff so I haven't had time. But just grab v. 1.80 from NPM and it should have been solved now. |
@KFlash : Version 1.8.0 with the does not work when the line endings are CRLF, but it does for LF. The issue is possibly that Note that my changes eliminated the need for two functions and state flag 4 as well as consolidating the parser switch cases for |
I more or less copied your code above. I will double check |
@lawrence-dol Pushed an update. Also see this https://github.com/meriyah/meriyah/blob/master/src/lexer/common.ts#L34 |
@KFlash : Ha ha. I'm not sure if I should be complimented, or worried. I'm using unpkg.com to get the iife. Is that easy to make available? Otherwise, I can tweak my local copy to match that change. |
OK, if I take 1.8.0 from My full test sources now include both SLC and MLC variations:
|
Can't you try v. 1.8.2 ? That version includes an exact replicate of your code. Doesn't iife exist already? |
Sorry, my bad; I refreshed the page, but forgot the unpkg redirects to a version specific URL. I've just pulled 1.8.2, and I get no output at all. |
That said, 1.8.2 does not replicate my code. It still has two consume functions, and uses |
I can probably make the changes to the TS code, now that I've become more familiar to the code base (just can't build and test it). That might be enough for a good-enough patch? |
It's ok. I can build and test it and give you feedback |
OK, I'll look into after work tonight. |
@KFlash : I've made the changes, but I don't have permission create a pull request on the repo. How would you like me to proceed? |
you can fork the repo. refs: https://help.github.com/en/articles/creating-a-pull-request-from-a-fork |
OK, that's done. Hopefully correctly. The pull request shows up on your repo. |
@lawrence-dol Any update on your progress regarding this issue? |
Not yet; I have a customer-down issue at work and a couple other high-priority tasks. After resetting the initial value for line to 0 (changing it to 1 is surely the reason for the failed tests) the line number for my test code with comments is out by 1. My gut says that the extraction of a character from the source (i.e. anything that logically causes the column to advance, and then the line if a break is read) needs to be consolidated to the single I'm not sure how feasible it's going to be for me to make the fix without the ability to take the TS code and produce JS which I can run to test. Currently I am making changes in the generated JS, and then remaking them in the TS -- which works for highly surgical changes, but not for more extensive ones. |
Take your time :) Do it if and when you can. cc / @nchanged |
Take it easy Kenny :) |
@lawrence-dol Have you looked into this issue any further? Btw. Look at this gist. This is my private locale code and not open sourced. Do you see the solution I'm using for location tracking? I may think you should try if that solution works for you and solve all issues, and if it does can you copy & paste it and create a PR? I'm not sure when I will be active again. Note that in that gist there are no column increment / decrement, and I simply divide the colum and line pos from the current index on line break. I think this lines is of interest. Also note that single line comment are only 3 lines of code :) No magic done in the hot loop as in current Meriyah source. |
@KFlash Can you generate the IIFE with this so I can try it and see? Or is there a simple way that I can do so (bearing in mind that I don't have node.js, or any TS build tools installed)? I presume that this passes current tests, so I can check with my known failing sources. |
I can't generate an IIFE with it because it was just an code snippet and not used anywhere. |
@lawrence-dol I'm friend of K.F and just pushed a change. I noticed the line count was wrong. Reported 12 where it should have been 18. This happened because of missing line pos counting in lexer. It's all fixed and I pushed a minor version to NPM. Your example code have been used as a test - located here and the line and column pos is in line with other parsers. |
For the test I provided, it's necessary to have the blank lines after the comments to trip the fault. It's also important to test Also, this update has reverted the column to point to the end of the faulting token, instead of the start. This is corrected by using |
@KFlash : Is it possible that the test failures related to column number are the result of changing the column to the start of the token instead of the end of it? |
@lawrence-dol I tried to reproduce by parsing the following snippet: And it looks as though it parsed nicely, to: {
"body": [
{
"async": false
"body": {
"body": [
{
"declarations": [
{
"id": {
"loc": {
"end": {
"column": 10
"line": 11
}
"start": {
"column": 7
"line": 11
}
}
"name": "prp"
"type": "Identifier"
}
"init": {
"computed": false
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 13
"line": 11
}
}
"object": {
"loc": {
"end": {
"column": 17
"line": 11
}
"start": {
"column": 13
"line": 11
}
}
"type": "ThisExpression"
}
"property": {
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 18
"line": 11
}
}
"name": "props"
"type": "Identifier"
}
"type": "MemberExpression"
}
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 7
"line": 11
}
}
"type": "VariableDeclarator"
}
]
"kind": "var"
"loc": {
"end": {
"column": 24
"line": 11
}
"start": {
"column": 3
"line": 11
}
}
"type": "VariableDeclaration"
}
{
"body": {
"body": [
{
"loc": {
"end": {
"column": 33
"line": 15
}
"start": {
"column": 32
"line": 15
}
}
"type": "EmptyStatement"
}
]
"loc": {
"end": {
"column": 34
"line": 15
}
"start": {
"column": 31
"line": 15
}
}
"type": "BlockStatement"
}
"init": {
"declarations": [
{
"id": {
"loc": {
"end": {
"column": 13
"line": 15
}
"start": {
"column": 11
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"init": {
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 14
"line": 15
}
}
"type": "Literal"
"value": 0
}
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 11
"line": 15
}
}
"type": "VariableDeclarator"
}
]
"kind": "let"
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 7
"line": 15
}
}
"type": "VariableDeclaration"
}
"loc": {
"end": {
"column": 34
"line": 15
}
"start": {
"column": 3
"line": 15
}
}
"test": {
"left": {
"loc": {
"end": {
"column": 19
"line": 15
}
"start": {
"column": 17
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"loc": {
"end": {
"column": 23
"line": 15
}
"start": {
"column": 17
"line": 15
}
}
"operator": "<"
"right": {
"loc": {
"end": {
"column": 23
"line": 15
}
"start": {
"column": 20
"line": 15
}
}
"type": "Literal"
"value": 100
}
"type": "BinaryExpression"
}
"type": "ForStatement"
"update": {
"argument": {
"loc": {
"end": {
"column": 27
"line": 15
}
"start": {
"column": 25
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"loc": {
"end": {
"column": 29
"line": 15
}
"start": {
"column": 25
"line": 15
}
}
"operator": "++"
"prefix": false
"type": "UpdateExpression"
}
}
]
"loc": {
"end": {
"column": 4
"line": 17
}
"start": {
"column": 30
"line": 9
}
}
"type": "BlockStatement"
}
"generator": false
"id": {
"loc": {
"end": {
"column": 27
"line": 9
}
"start": {
"column": 9
"line": 9
}
}
"name": "handleAutocomplete"
"type": "Identifier"
}
"loc": {
"end": {
"column": 4
"line": 17
}
"start": {
"column": 0
"line": 9
}
}
"params": []
"type": "FunctionDeclaration"
}
{
"body": [
{
"async": false
"body": {
"body": [
{
"declarations": [
{
"id": {
"loc": {
"end": {
"column": 10
"line": 11
}
"start": {
"column": 7
"line": 11
}
}
"name": "prp"
"type": "Identifier"
}
"init": {
"computed": false
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 13
"line": 11
}
}
"object": {
"loc": {
"end": {
"column": 17
"line": 11
}
"start": {
"column": 13
"line": 11
}
}
"type": "ThisExpression"
}
"property": {
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 18
"line": 11
}
}
"name": "props"
"type": "Identifier"
}
"type": "MemberExpression"
}
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 7
"line": 11
}
}
"type": "VariableDeclarator"
}
]
"kind": "var"
"loc": {
"end": {
"column": 24
"line": 11
}
"start": {
"column": 3
"line": 11
}
}
"type": "VariableDeclaration"
}
{
"body": {
"body": [
{
"loc": {
"end": {
"column": 33
"line": 15
}
"start": {
"column": 32
"line": 15
}
}
"type": "EmptyStatement"
}
]
"loc": {
"end": {
"column": 34
"line": 15
}
"start": {
"column": 31
"line": 15
}
}
"type": "BlockStatement"
}
"init": {
"declarations": [
{
"id": {
"loc": {
"end": {
"column": 13
"line": 15
}
"start": {
"column": 11
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"init": {
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 14
"line": 15
}
}
"type": "Literal"
"value": 0
}
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 11
"line": 15
}
}
"type": "VariableDeclarator"
}
]
"kind": "let"
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 7
"line": 15
}
}
"type": "VariableDeclaration"
}
"loc": {
"end": {
"column": 34
"line": 15
}
"start": {
"column": 3
"line": 15
}
}
"test": {
"left": {
"loc": {
"end": {
"column": 19
"line": 15
}
"start": {
"column": 17
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"loc": {
"end": {
"column": 23
"line": 15
}
"start": {
"column": 17
"line": 15
}
}
"operator": "<"
"right": {
"loc": {
"end": {
"column": 23
"line": 15
}
"start": {
"column": 20
"line": 15
}
}
"type": "Literal"
"value": 100
}
"type": "BinaryExpression"
}
"type": "ForStatement"
"update": {
"argument": {
"loc": {
"end": {
"column": 27
"line": 15
}
"start": {
"column": 25
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"loc": {
"end": {
"column": 29
"line": 15
}
"start": {
"column": 25
"line": 15
}
}
"operator": "++"
"prefix": false
"type": "UpdateExpression"
}
}
]
"loc": {
"end": {
"column": 4
"line": 17
}
"start": {
"column": 30
"line": 9
}
}
"type": "BlockStatement"
}
"generator": false
"id": {
"loc": {
"end": {
"column": 27
"line": 9
}
"start": {
"column": 9
"line": 9
}
}
"name": "handleAutocomplete"
"type": "Identifier"
}
"loc": {
"end": {
"column": 4
"line": 17
}
"start": {
"column": 0
"line": 9
}
}
"params": []
"type": "FunctionDeclaration"
}
{
"body": [
{
"async": false
"body": {
"body": [
{
"declarations": [
{
"id": {
"loc": {
"end": {
"column": 10
"line": 11
}
"start": {
"column": 7
"line": 11
}
}
"name": "prp"
"type": "Identifier"
}
"init": {
"computed": false
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 13
"line": 11
}
}
"object": {
"loc": {
"end": {
"column": 17
"line": 11
}
"start": {
"column": 13
"line": 11
}
}
"type": "ThisExpression"
}
"property": {
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 18
"line": 11
}
}
"name": "props"
"type": "Identifier"
}
"type": "MemberExpression"
}
"loc": {
"end": {
"column": 23
"line": 11
}
"start": {
"column": 7
"line": 11
}
}
"type": "VariableDeclarator"
}
]
"kind": "var"
"loc": {
"end": {
"column": 24
"line": 11
}
"start": {
"column": 3
"line": 11
}
}
"type": "VariableDeclaration"
}
{
"body": {
"body": [
{
"loc": {
"end": {
"column": 33
"line": 15
}
"start": {
"column": 32
"line": 15
}
}
"type": "EmptyStatement"
}
]
"loc": {
"end": {
"column": 34
"line": 15
}
"start": {
"column": 31
"line": 15
}
}
"type": "BlockStatement"
}
"init": {
"declarations": [
{
"id": {
"loc": {
"end": {
"column": 13
"line": 15
}
"start": {
"column": 11
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"init": {
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 14
"line": 15
}
}
"type": "Literal"
"value": 0
}
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 11
"line": 15
}
}
"type": "VariableDeclarator"
}
]
"kind": "let"
"loc": {
"end": {
"column": 15
"line": 15
}
"start": {
"column": 7
"line": 15
}
}
"type": "VariableDeclaration"
}
"loc": {
"end": {
"column": 34
"line": 15
}
"start": {
"column": 3
"line": 15
}
}
"test": {
"left": {
"loc": {
"end": {
"column": 19
"line": 15
}
"start": {
"column": 17
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"loc": {
"end": {
"column": 23
"line": 15
}
"start": {
"column": 17
"line": 15
}
}
"operator": "<"
"right": {
"loc": {
"end": {
"column": 23
"line": 15
}
"start": {
"column": 20
"line": 15
}
}
"type": "Literal"
"value": 100
}
"type": "BinaryExpression"
}
"type": "ForStatement"
"update": {
"argument": {
"loc": {
"end": {
"column": 27
"line": 15
}
"start": {
"column": 25
"line": 15
}
}
"name": "xa"
"type": "Identifier"
}
"loc": {
"end": {
"column": 29
"line": 15
}
"start": {
"column": 25
"line": 15
}
}
"operator": "++"
"prefix": false
"type": "UpdateExpression"
}
}
]
"loc": {
"end": {
"column": 4
"line": 17
}
"start": {
"column": 30
"line": 9
}
}
"type": "BlockStatement"
}
"generator": false
"id": {
"loc": {
"end": {
"column": 27
"line": 9
}
"start": {
"column": 9
"line": 9
}
}
"name": "handleAutocomplete"
"type": "Identifier"
}
"loc": {
"end": {
"column": 4
"line": 17
}
"start": {
"column": 0
"line": 9
}
}
"params": []
"type": "FunctionDeclaration"
}
]]]```
Can you elaborate on the issue, give broken example? |
Yes, you need to use LF line breaks, not CRLF, using current versions. Older versions had a problem with CRLF, but not LF. |
@lawrence-dol I think I saw what you meant and fixed it in #52. I added a test to make sure your snippet with LF and CRLF parses exactly the same. |
@lawrence-dol Can you confirm that this changes works for you now? |
Apologies for not commenting, but yes, I tested this as soon as it landed and confirmed correct reporting in my two test files. When I came back here it was closed, so I figured we were good. Appreciate you checking up on it. |
Similarly to #37 I now find that under some conditions the line number is incorrect when the file uses LF for line separators. Specifically, it again appears related to single-line comments, this time, being out by -1 for every SLC which is followed by at least one blank line. In the examples following, there is a missing
=
in the function's variable assignment.Out by -4:
Out by -3:
and so on.
This does not happen for CRLF line endings. I can induce any syntax error in a file with SLC's and it is consistently misreported.
The text was updated successfully, but these errors were encountered: