[WIP] Linters - eslint, jscs, jshint & scss #221

himdel · 2016-07-22T13:07:03Z

This adds two more linters to miq-bot - eslint (for javascript) and scss-lint (for sass/scss).

Their JSON output has a different structure, so this also adds a convert_parsed method to Linter::Base, and overrides to convert to the rubocop format:

eslint output:

[
  {
    filePath: "foo/bar",
    messages: [
      {
        ruleId: "baz",
        severity: 1,
        message: "quux",
        line: 4,
        column: 5,
      },
    ],
  },
]

scss output:

{
  "foo/bar": [
    {
      line: 4,
      column: 5,
      severity: "warning",
      reason: "quux",
      linter: "baz",
    },
  ],
}

converted output:

{
  files: [
    {
      path: "foo/bar",
      offenses: [
        {
          severity: "warning",
          message: "quux",
          location: {
            line: 4,
            column: 5,
          },
          cop_name: "baz",
        },
      ],
    },
  ],
  summary: {
    target_file_count: 1,
    offense_count: 1,
  },
}

TODO:

test converters
requires globally installed eslint-cli in the VM
maybe add stylelint support, Adds CSS linting support manageiq-ui-classic#2541

romanblanco · 2016-09-02T09:39:06Z

related to issue ManageIQ/manageiq#2389

Fryguy · 2016-09-06T18:16:13Z

You may also need to deal with this...perhaps not this PR, but...

miq_bot/app/workers/commit_monitor_handlers/commit_range/rubocop_checker/message_builder.rb

Line 45 in a056a9b

    
           header1 = "Checked #{"commit".pluralize(commits.length)} #{commit_range_text} with ruby #{RUBY_VERSION}, rubocop #{rubocop_version}, and haml-lint #{hamllint_version}"

Fryguy · 2016-10-18T16:45:02Z

@himdel Is this good to go? Or is there more stuff to be done?

himdel · 2016-10-19T09:11:57Z

@Fryguy this it's really missing only 2 things now..

a) verify that all the linter's JSON output formats are compatible enough

b) cache node_modules - in the end I think the fastest solution is to shasum package.json and .. essentially set NODE_PATH=whatever$(sha1sum packages.json) so that we have to run npm install only when the dir is missing

miq-bot · 2017-01-05T00:32:12Z

<pr_mergeability_checker />This pull request is not mergeable. Please rebase and repush.

chessbyte · 2017-01-06T20:51:07Z

@himdel please rebase and push again

…parsed ..so that these can be overriden in linters that have special parsing/running needs the default `parse_output` is the original JSON.parse + exceptions and could be overriden for linters that don't output JSON the default `convert_parsed` is an identity function, but is needed by eslint & scss to convert between the different JSON formats

converting from each format to the one used by rubocop eslint output: ``` [ { filePath: "foo/bar", messages: [ { ruleId: "baz", severity: 1, message: "quux", line: 4, column: 5, }, ], }, ] ``` scss output: ``` { "foo/bar": [ { line: 4, column: 5, severity: "warning", reason: "quux", linter: "baz", }, ], } ``` converted output: ``` { files: [ { path: "foo/bar", offenses: [ { severity: "warning", message: "quux", location: { line: 4, column: 5, }, cop_name: "baz", }, ], }, ], summary: { target_file_count: 1, offense_count: 1, }, } ```

miq-bot · 2017-01-11T19:43:10Z

Checked commits https://github.com/himdel/miq_bot/compare/c1966dde3a5e20dcccb836c123a7ff05ddb95a4b~...156d75696f87554809da6d49aad24493230a7712 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
5 files checked, 5 offenses detected

Gemfile

❗ - Line 47, Col 17 - Style/ExtraSpacing - Unnecessary spacing detected.

lib/linter/base.rb

❗ - Line 67, Col 7 - Style/RedundantBegin - Redundant begin block detected.

lib/linter/eslint.rb

❗ - Line 36, Col 16 - Style/MutableConstant - Freeze mutable objects assigned to constants.
❗ - Line 36, Col 16 - Style/WordArray - Use %w or %W for an array of words.

lib/linter/scss.rb

💣 💥 🔥 🚒 - Line 43, Col 1 - Syntax - unexpected token kEND
(Using Ruby 2.0 parser; configure using TargetRubyVersion parameter, under AllCops)

Fryguy · 2017-01-11T21:19:39Z

lib/linter/base.rb

+      end
+    end
+
+    # overriden in linters with different JSON format


comment typo overriden -> overridden

Fryguy · 2017-01-11T21:21:20Z

lib/linter/eslint.rb

+      files = array.map { |f| convert_file(f) }
+
+      {:files   => files,
+       :summary => {:offense_count     => files.reduce(0) { |memo, f| memo + f.offenses.count },


I believe files.reduce(0) { |memo, f| memo + f.offenses.count } can be simplified to files.map { |f| f.offenses.count }.reduce(:+)

miq-bot · 2017-01-16T19:16:54Z

This pull request is not mergeable. Please rebase and repush.

Fryguy · 2018-02-23T17:07:44Z

#406 will likely supercede this

himdel · 2020-06-16T22:37:49Z

Or #500 will :)

Fryguy · 2020-06-25T20:46:41Z

going to close this in favor of Pronto work as it's being revived.

himdel · 2020-10-14T15:38:49Z

@NickLaMuro for eslint, we need the following npm packages:

babel-eslint
eslint
eslint-config-airbnb
eslint-config-angular
eslint-plugin-angular
eslint-plugin-import
eslint-plugin-jest
eslint-plugin-jsx-a11y
eslint-plugin-react

You can assume current versions, I'll be updating them on ui-classic side soon. (=> ManageIQ/manageiq-ui-classic#7423)
(ui-classic also has eslint-{config,plugin}-standard and eslint-config-promise but these are unused)

I guess it makes sense to just copy those dependencies to manageiq-style, and update both places.

One more catch .. the .eslintrc.json configfile can be in any directory, we need them all (including the relative symlinks, but they can be copies).
(It inherits from parent dirs, so we can have spec-only globals, etc.)

NickLaMuro · 2020-10-14T17:41:53Z

Turns out it is probably for the best that we aren't using proto for this as it looks like the pronto-eslint gem under the hood uses eslintrb, which is super ancient, and uses a version of eslint that is very much out of date:

https://github.com/zendesk/eslintrb/tree/master/vendor [ref]

And probably doesn't have all of the plugins you want.

That said, the list of plugins is going to be a bit of a nightmare to source consistently, so we might want to consider vendoring them into this repo to avoid inconsistencies. This would mean that when run via a potential future "cli client", there is no concern about potentially getting the wrong dependencies.

Alternatively, we could consider creating a stand-alone repo for doing just that, and it can also have a gem component for ruby projects that want to consume it, with the process of updating it just being done with whatever standard JS means is preferred.

himdel · 2020-10-14T18:13:01Z

Oh, well, a CLI client would be great for miq-bot development, but that may just be complicating matters now.
I still expect UI developers to run eslint locally, let's not complicate it. (Not to mention it's impossible for editors to run a custom miq-bot cli linter.)

What about something as simple as having a manageiq-style package.json with the dependencies (and possibly a .nvmrc with a node version), and a base .eslintrc.json to apply when no root eslintrc.json is present.

The primary goal is to get this running on the bot, and it can just be statically installed there.

himdel · 2020-10-14T18:19:16Z

The primary goal is to get this running on the bot, and it can just be statically installed there.

In fact, I'd propose this directory structure, if doable...

root/
  package.json (from manageiq-style)
  .eslintrc.json (from manageiq-style)
  node_modules/ (by running yarn in root/)
  $repo/

That way, yarn run eslint $repo should work, pick up $repo/.eslintrc.json if present, fallback to root/.eslintrc.json if not.
And root/ can be cached on the bot, only updated when manageiq-style changes, while root/$repo gets regularly updated.

himdel · 2020-10-14T18:20:27Z

(and since $repo can be a tempdir with a random name, it should still work in parallel)

himdel mentioned this pull request Aug 2, 2016

eslint - move to hierarchical configuration ManageIQ/manageiq#10197

Merged

himdel mentioned this pull request Aug 10, 2016

Depend on jshint-json for miq-bot ManageIQ/manageiq-ui-service#98

Merged

romanblanco mentioned this pull request Sep 2, 2016

Cleanup Javascript files ManageIQ/manageiq#2389

Closed

4 tasks

miq-bot added the unmergeable label Jan 5, 2017

chessbyte assigned Fryguy Jan 6, 2017

himdel added 7 commits January 11, 2017 18:21

Linter::ESLint

c1966dd

Linter::SCSS

53e39bf

Enable the new linters (eslint, scss)

37e690f

Add scss_lint gem, eslint as a npm package

f16e125

gitignore node_modules

b231b12

miq-bot removed the unmergeable label Jan 11, 2017

Fryguy reviewed Jan 11, 2017

View reviewed changes

lib/linter/base.rb

end

end

# overriden in linters with different JSON format

Copy link

Member

Fryguy Jan 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

comment typo overriden -> overridden

Fryguy reviewed Jan 11, 2017

View reviewed changes

miq-bot added the unmergeable label Jan 16, 2017

himdel mentioned this pull request Nov 1, 2017

Adds CSS linting support ManageIQ/manageiq-ui-classic#2541

Merged

mfeifer added the wip label Jan 24, 2020

gtanzillo added the enhancement label Apr 2, 2020

Fryguy closed this Jun 25, 2020

NickLaMuro mentioned this pull request Oct 19, 2020

[WIP] Add .docker-compose.yml #519

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Linters - eslint, jscs, jshint & scss #221

[WIP] Linters - eslint, jscs, jshint & scss #221

himdel commented Jul 22, 2016 •

edited

Loading

romanblanco commented Sep 2, 2016

Fryguy commented Sep 6, 2016

Fryguy commented Oct 18, 2016

himdel commented Oct 19, 2016 •

edited

Loading

miq-bot commented Jan 5, 2017

chessbyte commented Jan 6, 2017

miq-bot commented Jan 11, 2017

Fryguy Jan 11, 2017

Fryguy Jan 11, 2017

miq-bot commented Jan 16, 2017

Fryguy commented Feb 23, 2018

himdel commented Jun 16, 2020

Fryguy commented Jun 25, 2020

himdel commented Oct 14, 2020 •

edited

Loading

NickLaMuro commented Oct 14, 2020 •

edited by chessbyte

Loading

himdel commented Oct 14, 2020 •

edited

Loading

himdel commented Oct 14, 2020

himdel commented Oct 14, 2020

[WIP] Linters - eslint, jscs, jshint & scss #221

[WIP] Linters - eslint, jscs, jshint & scss #221

Conversation

himdel commented Jul 22, 2016 • edited Loading

romanblanco commented Sep 2, 2016

Fryguy commented Sep 6, 2016

Fryguy commented Oct 18, 2016

himdel commented Oct 19, 2016 • edited Loading

miq-bot commented Jan 5, 2017

chessbyte commented Jan 6, 2017

miq-bot commented Jan 11, 2017

Fryguy Jan 11, 2017

Choose a reason for hiding this comment

Fryguy Jan 11, 2017

Choose a reason for hiding this comment

miq-bot commented Jan 16, 2017

Fryguy commented Feb 23, 2018

himdel commented Jun 16, 2020

Fryguy commented Jun 25, 2020

himdel commented Oct 14, 2020 • edited Loading

NickLaMuro commented Oct 14, 2020 • edited by chessbyte Loading

himdel commented Oct 14, 2020 • edited Loading

himdel commented Oct 14, 2020

himdel commented Oct 14, 2020

himdel commented Jul 22, 2016 •

edited

Loading

himdel commented Oct 19, 2016 •

edited

Loading

himdel commented Oct 14, 2020 •

edited

Loading

NickLaMuro commented Oct 14, 2020 •

edited by chessbyte

Loading

himdel commented Oct 14, 2020 •

edited

Loading