Feature: sourcemap support about eslint

[nzakas]

Can you explain more about what you're looking for with this?

[michaelficarra]

@nzakas: I believe @Bartvds would like eslint to take a source map as input and, when errors are reported, trace the line/column numbers using the source map and instead report the position of the code that generated the erroneous code. This shouldn't be very difficult if you use mozilla/source-map's SourceMapConsumer interface. 2 lines of code, easy.

[nzakas]

Ah, interesting! I'll take a look at SourceMapConsumer. Thanks.

[Bartvds]

Indeed. If enabled it would see the //@sourceMapURL=/path (or whatever the current spec is) in the source and use mozilla's module to allow ESLint to report the original position.

It's a little more then just 2 lines though: for example here is the implementation in node-source-map-support.

If you're interested I could add this myself, maybe in there: https://github.com/nzakas/eslint/blob/master/lib/eslint.js#L114 ?

[michaelficarra]

@Bartvds: That module is doing a lot more to set up automatic error handling and rewritten stack traces. All you really need is the SourceMapConsumer construction, the originalPositionFor call, and a little regex to extract the sourceMappingUrl value.

[Bartvds]

Yea; I tried highlighting the interesting part in the link. I looked into it and I'm forking to add this (with the caching bit).

Now I have browsed your code I see a nice spot to call it: https://github.com/nzakas/eslint/blob/master/lib/cli.js#L94, just before the messages get send to the formatter. Seems cleaner then I suggested in eshint.js as the api doesn't know anything about files.

[nzakas]

Can you walk me through how you think this would work? It sounds like
you're expecting ESLint to take in the TypeScript or CoffeeScript file and
extract the actual JS file to look at from the comments?

In that case, then I agree it must be done in the CLI since eslint object
isn't supposed to do any file I/O.

Feel free to submit a pull request and we can discuss the code. I'd like to
make sure we get tests covering this. I know that cli.js doesn't have tests
now, but we need to start. :)

On Tue, Jul 16, 2013 at 12:01 PM, Bart van der Schoor <
[email protected]> wrote:

Yea; I tried highlighting the interesting part in the link. I looked into
it and I'm forking to add this (with the caching bit).

Now I have browsed your code I see a nice spot to call it:
https://github.com/nzakas/eslint/blob/master/lib/cli.js#L94, just before
the messages get send to the formatter. Seems cleaner then I suggested in
eshint.js as the api doesn't know anything about files.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/45#issuecomment-21065184
.

---

Nicholas C. Zakas
@SlickNet

Author, Professional JavaScript for Web Developers
Buy it at Amazon.com:
http://www.amazon.com/Professional-JavaScript-Developers-Nicholas-Zakas/dp/1118026691/ref=sr_1_3

[Bartvds]

It works like any normal Javascript lint session but just before report time I check if the file we linted has the magic //@ sourceMappingURL= comment and then perform the source-map lookup and update the messages with the position mapped to the source file. And I add a .filePath property to the message object (as one file can be the generated from of multiple source files).

The basics I have already working locally; I added a function in cli.js and call it at that line 94. But as per usual creating tests and supporting code is more work then implementing the actual functionality :)

For example I had to add a new formatter to display file paths in a usable multiline format.

I'll move my test to a new cli.js test file and implement some others there to get you going. I hit some weird validation errors but I look into those tomorrow.

[nzakas]

I'm not sure I understand the need to add a file path to each message, but
I'll wait to see the code and maybe it'll become more obvious. :)

On Tue, Jul 16, 2013 at 4:48 PM, Bart van der Schoor <
[email protected]> wrote:

It works like any normal Javascript lint session but just before report
time I check if the source has the magic //@ sourceMappingURL= comment
and then perform the source-map lookup and update the message with the
position mapped to the source file. And I add a .filePath property to the
message object (as one file can be the generated from of multiple source
files).

The basics I have already working locally; I added a function in cli.js
and call it at that line 94. But as per usual creating tests and supporting
code is more work then implementing the actual functionality :)

For example I had to add a new formatter to display file paths in a usable
multiline format.

I'll move my test to a new cli.js test file and implement some others
there to get you going. I hit some weird validation errors but I look into
those tomorrow.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/45#issuecomment-21082689
.

---

Nicholas C. Zakas
@SlickNet

Author, Professional JavaScript for Web Developers
Buy it at Amazon.com:
http://www.amazon.com/Professional-JavaScript-Developers-Nicholas-Zakas/dp/1118026691/ref=sr_1_3

[Bartvds]

I've implemented this in a sourcemap branch of my fork: https://github.com/Bartvds/eslint/compare/sourcemap

I also added a bunch of tests so the diff is a bit longish. Before I send pull I'll have to explain what I did:

The source-map lookup itself was easy to implement (I kindly stripped some of the node-source-map-support code).

But to make it useful we need to store the mapped path per message: the one javascript file we are linting could have been the bundled output from many different source files so the single separate filename is not sufficient.

To display this I added a new formatter (similar to my jshint-part-reporter that will display these paths. It does so in a format that can be parsed and linked by tooling like WebStorm's 'External Tool' console view (this makes this reporter very convenient even without using the source-map feature)

To make this consistent for all formatters this enhancement is added in a separate .map() loop so the source-map call can be made optional without formatter breakage.

If you look at the fixture for the source-map tests using path reporter you see the $_PATH_$ placeholder: it is expanded to the full local path. Behind that you'll see how one warning has App.ts but the other has main.ts.

Then there are some other tests, and I modified the cli.js with a writeln() method so we can intercept the log output for testing.

[nzakas]

Cool, I need to find some time to dig through it. My gut tells me that we should just have all reporters be able to use filePath info on each message rather than creating an entirely new formatter.

[Bartvds]

I'd also go for maximum commonality.

Note: This code was based on an older fork (before the recent changes in the formatter api) so the merge against current code might be.. interesting :)

If you want this functionality I can update things to work with the current state. Alternately you could manually extract parts of the functional code and recompose things yourself, as a big part is testing code for the cli-module which was also changed after these edits.

Let me know if you want to pick this up to your own taste or if I should make the changes myself.

[nzakas]

I'd day give it a shot yourself on top of the current repo. Outside of th formatter structure change, I think the rest should be relatively easy.

[Bartvds]

Instead of trying to merge this into the core I moved my work into an external formatter .

The formatter is a again a proper clone of jshint-path-reporter, so the source-map is now handled at the formatter level instead of in the core cli. Also the output supports some color and options. Works great with eslint-grunt.

As this cover my use-case this issue could be closed.

[ilyavolodin]

@nzakas Can you close this issue? @Bartvds already created a formatter that does the same thing.