Ensure that JSHint is done after preprocessors.

[rwjblue]

JSHint currently does not work for JS that needs to be preprocessed. Specifically, this affects CoffeeScript users and folks that use custom sweet-js macros.

Also, removed left over commented out references to validateES6.

[stefanpenner]

LGTM

[gf3]

Is this actually desirable? Getting a ton of JSHint "errors" from generated code. Things that one can't change. I think a better solution might be to allow users to load different linter plugins, e.g.: a broccoli-coffeelint plugin, etc...

[rwjblue]

@gf3 - Are you saying that the generated .coffee -> .js files have JSHint errors?

[gf3]

@rjackson Yes, those errors are from the js generated from our coffee files.

[rwjblue]

The Ember is not defined errors are valid (unless you import Ember from 'ember';), the others seem project related (like the Identifier 'property_key_path' is not in camel case.) and can be fixed by simply bringing your .jshintrc file inline with your project preferences.

I'm not sure about the leaking variable ones though.

[rwjblue]

Which leaves only 2 errors from your screenshot, and I'm unsure if _ref and _ref1 are in your code, or are generated by the Coffee compiler (but it seems like it is auto-generated as those names aren't super descriptive).

[gf3]

@rjackson property_key_path, _ref1, and _ref2 are from the coffeescript compiler.

[gf3]

@rjackson Regardless, IMHO it would much more valuable to not run jshint over any sort of generated files and rely on different/more specialized linters. A la coffeelint.

[rwjblue]

@gf3 I disagree as some of our preprocessors would actually break JSHint if we don't run them first (many SweetJS macros result in invalid JS code before preprocessing). Essentially, your code was not previously being JSHint'ed (as your *.coffee files were ignored). You can get back to that point by disabling in your Brocfile.js:

var app = new EmberApp({
  hinting: false
});

I do think that we should make the hinting pluggable though, and will add that to my list of addon enabling things once my broccoli perf refactorings are done.

[gf3]

@rjackson Part of the issue is that I'd still like to use JSHint in a couple places in the project—I don't want to turn it off altogether. I can imagine others being in a similar situation, too. Additionally, specialized linters can provide more useful and context-sensitive information; things JSHint would not even be aware of.

[jdjkelly]

Seeing the same thing here re: JSHint complaining about the coffeescript compiler :'(

[vladikoff]

Don't JSHint generated files.
If your source files are .js use JSHint
If your source files are .coffee use CoffeeLint or something

[WMeldon]

@rjackson +1for pluggable linters, though I see 3 conflicting possible desired behaviors outside of the scope of the addon.

1. You DO want .js files to be linted
2. You DON'T want compiled .coffee files to be linted.
3. You DO want compiled SweetJS files to be linted.

I've been fine with just turning it off entirely as everything in my project is CS, but I'm curious what @gf3's use case is. And also if anyone can think of a reason to run compiled CS through JSHint, because I can't.

[gf3]

@WMeldon Indeed, even in the case of something like SweetJS, it almost makes more sense to me to be able to make JSHint aware of the syntax of your macros vs. linting the compiled JS. As you could run into similar issues as other compilers.

[kieran]

this fills me with the quiet rage of a thousand disapproving grandmothers.

Having lint errors tied to code that's not directly under your control essentially makes the linting useless - maybe we need two lint-levels?

[stefanpenner]

proposed solution: don't use coffee-script or disable the linter

I see a path where coffee-script is an ember-cli-addon, that brings its own linter. Unfortunately that will have to be a community effort. We will help to ensure our add-on system is sufficient to cover this case.

[jennschiffer]

Since it causes an issue for non-js either way, then I think the hinting should go back to being done pre-compile. Like @vladikoff said, hinting isn't for generated/compiled files.

[jdjkelly]

+1 for supporting disabling the linter for now. It's a nice add-on, but we'd probably take coffee-script support over it atm.

EDIT: In case this isn't clear - I mean +1 for supporting the ability for the end user to disable the linter - not removing it frmo the project itself.

[rwjblue]

@jdjkelly - The linter is currently disableable:

var app = new EmberApp({
  hinting: false
});

[jdjkelly]

@rjackson you're amazing

[jdjkelly]

[stefanpenner]

@jdjkelly ok, now I have to run and get a tasty snack....