clutchski / coffeelint

Lint your CoffeeScript.
http://www.coffeelint.org
Other
1.18k stars 171 forks source link

eol_last: appears broken #533

Closed bdkjones closed 8 years ago

bdkjones commented 8 years ago

If I enable eol_last, CoffeeLint tells me that this file does not end with a single newline:

screen shot 2015-11-18 at 20 11 12

It very clearly does. And it doesn't matter if I add one additional newline there: CoffeeLint still tells me the file does not end with a single newline.

Additional Error:

CoffeeLint reports the line number of this error as 384. The file itself has only 382 lines. (Although in your text editor it appears to have 383 because the editor inserts the next line for you to start typing on.) If you count the instances of \n, however, you'll find only 382 in the file.

This makes a difference for tools that might use the line number reported by CoffeeLint to do things. The line number needs to be accurate. It's never going to make sense for the reported line number of an error to exceed the number of lines in a file.

bdkjones commented 8 years ago

In case it helps: this file uses Unix newlines \n. Although CoffeeLint should be able to handle Windows line-endings as well: \r\n and even old-school newlines from BeOS and variants of Linux: \r. I have, in my travels, encountered files that use a mix.

Not sure if you guys support a mix, but this file doesn't have anything but \n endings.

swang commented 8 years ago
  1. Can you make sure you're on the latest CoffeeLint (or at least 1.13.x)?
  2. Can you provide that sample file in a gist, or trim it down to where it still generates the error? I tried typing some similar examples as the last two tests but CoffeeLint linted it fine.

Lastly, CoffeeLint just splits on \n, then returns the last line where it found the extra \n I can't tell what you have in that file (and it does look like just one \n) but if there were two newlines there, it would count the last newline it saw, which should be "line" 384. Again though I don't really know what's going on here yet.

Also what editor are you using? In Sublime when I tell it to add the extra newline I see it as a blank line at the end of the file but in vi I see nothing.. That may just be because of how vi displays files though...

bdkjones commented 8 years ago

I'm using the coffeelint.js script linked from the coffeelint.org website—literally the script linked from the <script> tag in the website source. Is that not the latest?

As for my test file: it's just assignment.coffee from the CoffeeScript source (in the test folder).

As for the line-number issues: in a file with ONLY unix line endings \n, the number of lines is exactly equal to the number of times \n occurs IF the EOF marker comes directly after the last \n. Otherwise, if there are other characters between the last \n and EOF, then the number of lines must be the number of times \n occurs plus one (the EOF marker essentially marks the end of our last line).

swang commented 8 years ago

It does look like coffeelint.org is using the latest version of coffeelint (1.14.0)

If you are pasting this into the site, keep in mind that it looks like when you do so, it eats the last newline generated by the text editor. It did so when I tried pasting in the file. (But, also a bug, there is no message for the lack of newline)

Also the assignments.coffee file you are pasting seems to be from almost 3 years ago??? Are you using a version of coffee-script that old?

bdkjones commented 8 years ago

I'm not pasting the code into the site. I'm taking that coffeelint.js file and running it in JavaScriptCore within a Cocoa application. (I'm the guy that writes CodeKit). CoffeeLint is running just fine and the line number it's reporting does not rely on my own code in any way---that's exactly what CoffeeLint is spitting out.

And nope, I'm not running a version of CoffeeScript that old; that's just the file in my test suite.

I DID notice that the CoffeeLint.org website links CoffeeScript 1.7.1 whereas I provide CoffeeLint with CoffeeScript 1.9.1 in my environment. Not sure if that makes a material difference, but it is different.

swang commented 8 years ago

Thanks for bringing that to my attention. I'm not sure if that is the problem but it is a problem. Will push a new version soon.

To clarify: When you run coffeelint.js through jsc (and enable eol_last), it returns an error, even though the editor displays an empty last line?

Also, is it possible somewhere in your code it is stripping out/trimming any newlines? Like when the file string is passed through the coffeelint.lint function? I ask this because even with the current version in the browser, if I goto the console of coffeelint.org and type the following it does not generate an error

coffeelint.lint("test\n", { eol_last: { level: 'error'}})

And these two lines do

coffeelint.lint("test\n\n", { eol_last: { level: 'error'}})
coffeelint.lint("test", { eol_last: { level: 'error'}})
bdkjones commented 8 years ago

To clarify: When you run coffeelint.js through jsc (and enable eol_last), it returns an error, even though the editor displays an empty last line?

Correct.

is it possible somewhere in your code it is stripping out/trimming any newlines?

It is possible, but I can't verify. I read-in the exact bytes of the file and then create a string in JSC using this:

screen shot 2015-11-18 at 23 58 19

Those functions are all opaque to me. They're part of JavaScriptCore. That said, these same functions are what power WebKit's JavaScript engine, so if they were dropping the last newline I would expect them to do so in Safari as well. It would seem strange that Apple would modify strings that way. Nevertheless, I can't rule it out.

Is there a way to have CoffeeLint log the exact input string it receives? Or the AST or something similar?

bdkjones commented 8 years ago

Does the command line version of CoffeeLint show this behavior when run on the assignments.coffee file?

swang commented 8 years ago

I ran coffeelint on the version of assignments.coffee you were using to test and I don't get any errors.

Easiest way to find out what it's actually inputting is to just print out source being passed through coffeelint.lint on line 297 and see what its doing. Not sure if there are other ways to debug in jsc.

bdkjones commented 8 years ago

Hmm. Interesting. I'll take a closer look over here. Thanks!

swang commented 8 years ago

I think this issue was resolved. If it isn't, please reopen.