Fix: Emit correct endLine numbers

[plmrry]

Fixes #87

This adds each blocks start.line to each message's endLine, if it exists on the message.

This PR also updates the eslint used in tests to version 4. The older version of eslint did not emit endLine numbers.

[jsf-clabot]

All committers have signed the CLA.

[plmrry]

Hmm, further testing shows something is weird with the way comment blocks are being used in the line number calculations...

[plmrry]

Fixed! I wasn't actually adding the leadingCommentLines in my calcuation

[plmrry]

It seems Node < 4.0 doesn't like eslint 4.0 😕

To get the build passing, I added a test to processor.js and skipped my tests in plugin.js (which require eslint 4.0 to pass, since before eslint 4.0, endLine was not a part of the message object)

[aladdin-add]

yes, eslint v4 does not support Node < 4. and the up-coming eslint v5 will drop supporting Node < 6.

So we might need to do something to drop the older Node?

[plmrry]

@aladdin-add I think we would just need to update .travis.yml to not build against those old Node versions?

[aladdin-add]

I just created a PR #89 to do it. (maybe need update the docs).

[btmills]

@plmrry thanks for doing this! As you've figured out, and to clarify for anyone else reading, the endLine key wasn't added to messages until [email protected], but this plugin is currently tested all the way back to node 0.10, which means it's tested against eslint@^2, since [email protected] dropped support for node < 4. That prevents a full integration test, though this change can still be unit tested by manually supplying endLine in the processor tests.

I left #89 (comment) trying to figure out what to do with this, and I'd like to hear your thoughts as well.

If we do decide against dropping support for eslint < 4, it's possible with a small tweak to implement this change in a way that's fully backwards-compatible with eslint prior to 3.1.0:

diff --git a/lib/processor.js b/lib/processor.js
index 113b289..a13b795 100644
--- a/lib/processor.js
+++ b/lib/processor.js
@@ -129,16 +129,17 @@ function adjustBlock(block) {
     return function adjustMessage(message) {

         var lineInCode = message.line - leadingCommentLines;
-        var endLine = message.endLine - leadingCommentLines;
         if (lineInCode < 1) {
             return null;
         }

         var out = {
             line: lineInCode + blockStart,
-            endLine: endLine ? endLine + blockStart : endLine,
             column: message.column + block.position.indent[lineInCode - 1] - 1
         };
+        if (message.endLine >= 0) {
+            out.endLine = message.endLine + blockStart;
+        }

         return assign({}, message, out);
     };

[btmills]

Sorry about the delay - I just merged #89, which means this PR can now assume a minimum of ESLint v4! I commented in two places where the tests will now work, so just those two tweaks and then LGTM

[btmills]

These two tests should now pass with ESLint 4!

[btmills]

Now that #89 dropped support for ESLint < v4, we should be able to test without any NaN here, so this would be 1, and the assertion below would become 4.

[btmills]

Just pushed some changes to the fork to re-enable the endLine tests. Leaving this open for a couple days in case anyone else has comments.