upgrade to eslint 2.2.0

[rodneyrehm]

The current version of eslint is 2.2.0, codeclimate-eslint is using codeclimate/eslint which is on version 1.10.3.

The discrepancy is causing messages like Definition for rule 'keyword-spacing' was not found to fail my repository

[pbrisbin]

Hi @rodneyrehm thanks for reporting this. Unfortunately, upgrading to 2.0 brings a number of incompatibilities and so we need to be more careful than usual to ensure users who haven't updated their configurations for 2.0 don't suddenly start erroring. We're definitely working on this though, and will update here when we have more information.

In the meantime, you could remove any unknown rules from your eslintrc file to get things working.

[dancrumb]

Hi @pbrisbin: I'm keen to see this happen, but before I dig in, I wanted to see if you folks are having internal discussions about it.

I see a few options. Since this, ultimately, run in a container, a simple approach would be to fork a second container which supports ESLint 2.x
Alternatively, you could build 2 versions of the engine inside the single container and then use some configuration value to select the appropriate version.

Doing this super cleanly is probably stymied by this issue: npm/npm#5499
but it could probably be done in a not-too-kludgy way.

I'd like to assist, but I'd need to understand the reasoning behind the current fork of eslint/eslint to codeclimate/eslint. This seems like it is more than just a version pin... is there some other reason?

[gdiggs]

Hey @dancrumb! Here was our idea for this. I'm excited that you're interested in helping out!

We basically want to make a codeclimate-eslint-2 engine (exactly what you outlined), which runs eslint 2.x. This will become the new default ESLint engine (for people who run codeclimate init or are using our default configuration on codeclimate.com). We will continue to have codeclimate-eslint for those who are already using it, so that we don't break their builds with configuration changes.

The biggest considerations on our end are actually around alerting people and setting up a time when we will promote codeclimate-eslint-2 to codeclimate-eslint and drop support for ESLint 1.x. That's not as much your concern, but I thought it would be good to be transparent about that discussion we're having.

In terms of our fork of ESLint: we support a limited number of ESLint packages, but can't support others due to security concerns. The packages we support are

• the airbnb config
• the babel parser and plugin
• the react plugin

You can see the changes we made to support some but not all plugins in this diff. We will need to re-apply these to the new version of ESLint.

Hope that helps! If I can be of any more help as you start to dig in, let me know! I've made an eslint-2 branch on this repo and on our eslint repo - would be good to make PRs against those as our plan is to keep master on the 1.x for now (and eventually merge the eslint-2 branch in).

[wingrunr21]

So there's no good way to use other plugins? For example, we actually ended up moving away from the airbnb config for our React app as their settings increasingly started to deviate away from what we wanted and we preferred to create our own config vs maintaining a list of overrides.

[dancrumb]

@gordondiggs cool... that really helped.

So... I'm wondering if there might be a better way to implement the plugin restrictions. I'm going to fork your eslint-2 branch and then offer an alternative... more than happy for you to pass on it, but I'd like to see if we can do something that's easier for you to maintain.

[gdiggs]

@dancrumb sounds great - go for it!

[wingrunr21]

@dancrumb let me know if I can be of any assistance. We are ramping up Code Climate for a lot of our Ruby stuff and would like to move the JS there as well (vs linting in a Travis job), so we are pretty interested in getting something viable.

[dancrumb]

@gordondiggs I just threw this together: #79

Uses AOP advice to monkey-patch the methods that you've modified (since the modifications are simply guard modifications).

If you're averse to adding another dependency, it wouldn't be too difficult for me to write specific code to achieve this, but using a tested module seemed smarter.

I have a few questions in the PR around the code's shortcomings... love to hear your thoughts.

[gdiggs]

@dancrumb I like the approach! Happy to answer whatever questions you have

[dancrumb]

In short... what's the docs.js aiming to achieve? I don't see it being accessed anywhere in your diff. It's appended to the API, but where is it called?

OK... now I see it... it's called by the engine in eslint.js

[wingrunr21]

Is maintaining a plugin whitelist preferable to sandboxing the node process and whitelisting what it is allowed to access?

[dancrumb]

OK... I think I've got the docs stuff down. That's handled in the image build phase, so may feel a little tricksy... take a look and see what you think.

Sadly, this can't be 100% decoupled from the eslint codebase, since we need to know which methods to advise with the AOP functions... but it does mean that you're no longer forking the ESLint repo, which should make it easier to keep pace.

Also, if the relevant ESLint API doesn't change too much, then your ESLint1 and ESLint2 build processes should be pretty simple

[dancrumb]

Pull Request #79 is under review

[sagiegurari]

just curious but when will codeclimate support for eslint 2 is expected to happen?

[rajatvig]

This affects us adopting Code Climate as the generated rules by Code Climate for ESLint 1.x throws up lots of errors. So using it as is isn't going to work.

[RobbieTheWagner]

+1 this should be easy to support, if it's just broken into two separate options that we could choose in our .codeclimate.yml

[lewismoten]

I've been running into the ESLint rule configuration bugs with a bit of confusion since this was my first project that I put into code climate. +1 to anyone who can support multiple versions of eslint. I agree with @rwwagner90 - give me an option like

engines:
  eslint:
    enabled: true
    version: 2

[abritinthebay]

to add to that - ideally version should be explicit. 2 vs 2.1 vs 2.2, etc. That way it future proofs this config variable (vs only 1 vs 2)

[abritinthebay]

Any progress on this? It's a major issue and honestly is starting to make my company look at other possible solutions...

[gdiggs]

Hey there! We are working on a solution for this issue now and hope to have something to share in the next week or two!

[abritinthebay]

Oh thank god. Hopefully it'll finally get rid of React was defined and never used on every. damn. component. (amongst other issues)

I'll bug you in a week or two if I hear nothing 😝

[RobbieTheWagner]

+1 would definitely like to see this implemented soon

[dblandin]

Hey everyone!

I'm happy to announce that ESLint v2 is now available for this engine as a new channel! For now, the default stable channel will still target ESLint v1 but you can opt-in to the ESLint v2 channel within your .codeclimate.yml configuration file:

engines: 
  eslint:
    enabled: true
    channel: "eslint-2"

This is currently available via the CLI after upgrading with brew update && brew upgrade codeclimate and on codeclimate.com.

Many thanks to @dancrumb for his work in #79 and everyone else for the support!

Let me know if you have any questions!

[kunagpal]

Will a channel value of eslint-3 work in this case, or has support for ESLint 3 not been added yet?

[wfleming]

@kunagpal Sorry to say there is not yet an eslint-3 channel. We'd of course like to support ESLint 3, but haven't gotten to doing that work yet. It's something we'd love help on from the community if you're able to help!

[kunagpal]

@wfleming Alright, I'll see what I can come up with. :)