Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add some linters specifically for the Gemfile #3600

Closed
2 of 7 tasks
dkniffin opened this issue Oct 12, 2016 · 11 comments
Closed
2 of 7 tasks

Add some linters specifically for the Gemfile #3600

dkniffin opened this issue Oct 12, 2016 · 11 comments

Comments

@dkniffin
Copy link
Contributor

dkniffin commented Oct 12, 2016

It'd be awesome if there were some linters specifically geared towards the Gemfile.

There's probably a bunch of ones that could be made, but here's some ideas off the top of my head:

  • Make sure a gem group isn't defined twice
  • Enforce alphabetization of "groups" of gems (lines that are next to each other)
  • Make sure a gem is only listed once (although this is actually built into bundler)
  • Specify requirements for gem versions.
    • Require a version for every gem
    • Require versions to only be at the "major" level
    • Require no versions to be specified, unless it has a comment with it (related blog post)
  • require the ruby version to be specified
  • require source 'http://rubygems.org' at the top of the file
  • A check specifically for this vulnerability
@mikegee
Copy link
Contributor

mikegee commented Oct 13, 2016

I think this is a great idea. Some of those checks will need configurability, of course. (For instance, where I work apps are required to pull from the local gem server, not rubygems.)

A check specifically for this vulnerability would be cool, too.

@dkniffin
Copy link
Contributor Author

Interesting. I hadn't heard of that vulnerability. I think it'd be worthwhile to have a check for it. I'll add it to the list in my original post.

@bbatsov
Copy link
Collaborator

bbatsov commented Oct 13, 2016

I agree. I'd accept Gemfile linters if someone is willing to write them. :-)

@jmks
Copy link
Contributor

jmks commented Oct 14, 2016

Make sure a gem is only listed once

I'll give this one a go

jmks added a commit to jmks/rubocop that referenced this issue Nov 1, 2016
The DuplicatedGem cop checks for duplicate gem entries in Gemfiles.
bbatsov pushed a commit that referenced this issue Nov 1, 2016
The DuplicatedGem cop checks for duplicate gem entries in Gemfiles.
pocke added a commit to pocke/rubocop that referenced this issue Dec 1, 2016
See also rubocop#3657 rubocop#3600

Goal
----

Auto Correct not sorted gems.

e.g.

```ruby
gem 'b'
gem 'd'
gem 'c'
gem 'a'

gem 'a'
gem 'b'
gem 'c'
gem 'd'
```

Note
-----

I added `autocorrect_source_with_loop` test helper method to execute auto-correction with loop.
This method is based on do_inspection_loop.
https://github.com/bbatsov/rubocop/blob/ed4aeb845bfcaaff0648d365c5b46a2e725347f7/lib/rubocop/runner.rb#L179-L202
bbatsov pushed a commit that referenced this issue Dec 1, 2016
See also #3657 #3600

Goal
----

Auto Correct not sorted gems.

e.g.

```ruby
gem 'b'
gem 'd'
gem 'c'
gem 'a'

gem 'a'
gem 'b'
gem 'c'
gem 'd'
```

Note
-----

I added `autocorrect_source_with_loop` test helper method to execute auto-correction with loop.
This method is based on do_inspection_loop.
https://github.com/bbatsov/rubocop/blob/ed4aeb845bfcaaff0648d365c5b46a2e725347f7/lib/rubocop/runner.rb#L179-L202
@RKushnir
Copy link

RKushnir commented Jun 2, 2017

The idea of enforcing an order in a Gemfile is just wrong. The order is important because the gems are loaded as they are listed and may contain logic dependent on the gems already in memory.

@mikegee
Copy link
Contributor

mikegee commented Jun 2, 2017

@RKushnir, the new cop enforces ordering of groups of adjacent gems, not the whole Gemfile. No one has reported it breaking their app in the six months since it was added.

@RKushnir
Copy link

RKushnir commented Jun 2, 2017

@mikegee I see, I didn't immediately grasp what is a group. Then it assumes the dependent gems are placed in different "batches". So if you need to enforce a specific order, you have to put a newline between them.

@bbatsov
Copy link
Collaborator

bbatsov commented Sep 12, 2018

We've got some cops in this group already, so I guess we can close this one.

@bbatsov bbatsov closed this as completed Sep 12, 2018
@vassilevsky
Copy link
Contributor

I don't understand why gems have to be sorted alphabetically.

@dkniffin
Copy link
Contributor Author

dkniffin commented Mar 5, 2020

@vassilevsky They don't have to be. You can disable that linter config (cop). The reason you might want it is so that you can quickly scan down the list visually and see whether a gem is there or not.

@vassilevsky
Copy link
Contributor

Yeah that's one reason to have them sorted :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants