codeclimate / codeclimate-eslint

Code Climate Engine for ESLint
MIT License
95 stars 93 forks source link

Enable plugin? #234

Closed tillkruss closed 7 years ago

tillkruss commented 7 years ago

Hello!

I see that eslint-plugin-html has been added to the list of supported plugins, however it doesn't seem to be enabled by default. Do I need to enable the plugin somehow in my .codeclimate.yml?

dblandin commented 7 years ago

Hi @tillkruss,

You can update your ESLint configuration to enable the html plugin as described in the project's README:

{
  "plugins": [ "html" ]
}

https://github.com/BenoitZugmeyer/eslint-plugin-html#usage

tillkruss commented 7 years ago

Yes, I did that and when I run eslint locally I see a couple of issues, however CC only reports ESLint is running with the parser., but all my .vue files are A-rated.

dblandin commented 7 years ago

Ahhh, I see what's going on.

Looks like we're hard-coding the scanned extensions to [".js"] here: https://github.com/codeclimate/codeclimate-eslint/blob/473c99a4d4358243ee9ab6cba82e6b05979647a5/lib/eslint.js#L15

We're going to want to allow an override configuration or widen that scope by default.

I'll see about getting that change in. Thanks for the issue. :ok_hand:

tillkruss commented 7 years ago

Cool, thanks.

I've got this in my .eslintrc.json:

{
  "settings": {
    "html/html-extensions": [".blade.php", ".vue"]
  }
}
wopian commented 7 years ago

Isn't it already possible to overide that default setting by adding .vue to codeclimate's eslint engine config? eslint.js#L157

eslint:
  config:
    extensions:
    - .js
    - .vue

Although it appears eslint is trying to parse the entire .vue file instead of just within <script> for me (probably an entirely different issue?) - got a load of issues where eslint failed parsing various parts of pug within <template>: https://codeclimate.com/github/wopian/hibari/issues

dblandin commented 7 years ago

Isn't it already possible to overide that default setting by adding .vue to codeclimate's eslint engine config? eslint.js#L157

😞 Yep! Didn't look closely enough. You're absolutely right. You can already provide an alternate list of file extensions.

@wopian I might try running ESLint locally to see if you can reproduce those same parsing errors. Might be an issue with the eslint-config-html plugin itself.

wopian commented 7 years ago

@wopian I might try running ESLint locally to see if you can reproduce those same parsing errors. Might be an issue with the eslint-config-html plugin itself.

ESLint's running fine locally and shows linting warnings when they occur, as hibari also wont build with parsing errors 😃

Edit: ./node_modules/.bin/eslint --ext .js,.vue client test/unit/specs

dblandin commented 7 years ago

@wopian Are you using a custom eslint parser for vue files like https://github.com/mysticatea/vue-eslint-parser ?

wopian commented 7 years ago

@dblandin using babel-eslint as the parser.

.eslintrc.yml (nested configs also in client and test/unit) and .codeclimate.yml


Noticed this on the past builds page and looked into the patch provided from this:

We automatically generated one or more configuration files for this repo. You can download this configuration to customise it.

And it appears the patch wants to remove the HTML plugin from my eslint config and enable JSX:

 ---
-root: true
-parser: babel-eslint
 parserOptions:
   sourceType: module
-extends: standard
-plugins:
-- html
+  ecmaFeatures:
+    jsx: true

I've explicitly disabled it in the root eslint config now (defaults to false)...but codeclimate's still turning JSX on.

 ---
-root: true
-parser: babel-eslint
 parserOptions:
   sourceType: module
   ecmaFeatures:
-    jsx: false
-extends: standard
-plugins:
-- html
+    jsx: true

So appears to be unrelated to this issue - better off continuing this in a new issue?

wopian commented 7 years ago

Experimented around a bit and JSX's state doesn't matter (although its kinda annoying its enabling a disabled option, as JSX will break the app).

Removing plugin: - html produces the same warnings as on the issues page, so is due to this issue after all 😭

tillkruss commented 7 years ago

@dblandin Adding vue the extensions list did the trick for me.

It might be nice if the html/html-extensions property would be parsed automatically, tho.

engines:
  eslint:
    enabled: true
    config:
      extensions:
        - .js
        - .vue 
dblandin commented 7 years ago

@wopian And it appears the patch wants to remove the HTML plugin from my eslint config and enable JSX:

That looks a bug on our end. We should not be overriding your configuration file. Looking into this now. Feel free to create a new issue. Looks like the original issue here has been addressed.