Extra EOL breaking line numbers

[Arcanemagus]

This seems to be the exact opposite of #126.

While working on linter-js-yaml I kept running into a strange bug where the linter was reporting errors on the file on lines past the end of the file. I've traced it down to the fact that it is adding an EOL character to the end of the input, which causes some of the messages to be "past" the input.

For example, if you have this as the contents of the file:

key:
  subkey: "Value"
a

Then the contents of the buffer passed to this are:

JSON.stringify(TextEditor.getText())
""key:\n  subkey: \"Value\"\na""

However the message received from this linter is running on the following:

JSON.stringify(message.mark.buffer)
""key:\n  subkey: \"Value\"\na\n\u0000""

This leads to it reporting can not read a block mapping entry; a multiline key may not be an implicit key in "test.yml" at line 4, column 1: ^ on a file that only has 3 lines.

For now I will be working around this by capping the line number at the maximum for the file, but this seems like poor behavior on the part of the linter to be reporting things that don't exist.

[puzrin]

So, what do you suggest?

[Arcanemagus]

I don't know the internals of this linter, so I don't know why that was originally put in there. None of the other linters I have experience with modify their input (unless asked to do so).

Personally I would see how difficult it would be to fix the checks that were breaking without the extra line on the end, but maybe that is required here?

[puzrin]

• you can see the same hack in ruby yaml parser, we did not invented anything special
• we had a huge number of false bugreports without this hack, and none with it :)

It's technicaly possible to change everything, but will it do things better? I you do dev tool, then special processing of error on fake line looks the most simple solution.

[puzrin]

I mean, that doing change that will be used only in your linter looks more strange, than leaving persistent (but useful) kluge as is.

[Arcanemagus]

I just thought I would report the issue as the results aren't actually "correct", and the whole point of linters is to fix things that are incorrect :wink:

Feel free to close this if you want as I've worked around it in the package and it's no longer causing errors there.

Thanks for making this linter as it's always nice to have an extra set of "eyes" looking over the code.

[puzrin]

That's not a linter, that's parser only. I don't reject to make fixes, but i don't understand what can be fixed in this specific case.

Do you mean it would be better to relocate error position for edge case? We can not skip adding EOL at the end.

[dervus]

Well, I agree that reporting wrong error on a non-existing line is probably a problem. I think this could be solved by changing the way we add that "ghost" line — move "ghost" line logic into line break reader instead of adding an actual line break character. But it's just an idea, I can't predict if it can work this way or not.

By the way, @Arcanemagus , relying on JS-YAML as a linter backend is probably not the best idea. It was designed to be high-performance parser for practical YAML use-cases. So it has a non-canonical design for YAML parsers, very complicated and hard to read/verify code base, and at least two long-running defects in grammar parsing code (see the issue tracker).

[Arcanemagus]

Good to know, I just assumed it was a linter as that's what the original author wrote it as. I'll update the readme to mention that it isn't actually intended to be a linter for YAML files so the results may not be perfect.

Again, I'm not sure why this was originally put in or what edge cases it fixes, all that I know is that it was reporting an error on a line that doesn't exist.