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

Bundler/OrderedGems cop does not catch gems that have a comment between them #4004

Closed
betesh opened this issue Feb 2, 2017 · 6 comments
Closed
Labels

Comments

@betesh
Copy link
Contributor

betesh commented Feb 2, 2017

When there is a comment between 2 gems in the Gemfile, the OrderedGems cop should still enforce that gems are listed alphabetically.

The docs make it clear that if there's an empty line between them, they don't need to be sorted. But if a comment is between them, that seems different and I would expect it to sort them.

My reasoning is as followed:

An empty line indicates that they are separate groups of gems.
A comment, on the other hand, does not indicate a separate group--it may indicate that one of the gems within this group requires some explanations. If you've enabled Style/InlineComment, then you have no other choice but to place the comment on the line above them gem, even if it's in middle of a group.


Expected behavior

Rubocop should detect 1 offense, just as it does when the comment is not there

Actual behavior

Rubocop does not detect the offense.

Steps to reproduce the problem

$ cat Gemfile
# frozen_string_literal: true
source 'https://rubygems.org'

gem 'rubocop'
# A comment
gem 'parser'
$ rubocop --only Bundler/OrderedGems
Inspecting 1 file
.

1 file inspected, no offenses detected

RuboCop version

$ rubocop -V
0.47.1 (using Parser 2.3.3.1, running on ruby 2.2.2 x86_64-linux)
@betesh betesh changed the title Bundler/OrderedGems cop does not catch gems that have a comment in between Bundler/OrderedGems cop does not catch gems that have a comment or an empty line in between Feb 2, 2017
@betesh betesh changed the title Bundler/OrderedGems cop does not catch gems that have a comment or an empty line in between Bundler/OrderedGems cop does not catch gems that have a comment between them Feb 2, 2017
@mikegee
Copy link
Contributor

mikegee commented Feb 2, 2017

I agree. I use comments in the Gemfile like that to explain why we have to use a branch of a gem, for instance.

@bbatsov bbatsov added the bug label Feb 3, 2017
@bbatsov
Copy link
Collaborator

bbatsov commented Feb 3, 2017

Yeah, that's definitely a bug. Thanks for spotting and reporting it.

@mikegee
Copy link
Contributor

mikegee commented Feb 3, 2017

@pocke some of us think this behavior is a bug, but it looks like you intended it to behave this way. Would you kindly contribute your opinion here?

@pocke
Copy link
Collaborator

pocke commented Feb 4, 2017

but it looks like you intended it to behave this way.

I reconsidered this implementation. I think the proposed behavior is correct. However, current behavior is correct, too.

I have investigated several Gemfiles. Then, I can find that both usages.

for example

### proposed usage
# Comment is for one gem description. 
# This example is created by `rails new` command.

# Bundle edge Rails instead: gem 'rails', github: 'rails/rails'
gem 'rails', '~> 5.0.1'
# Use sqlite3 as the database for Active Record
gem 'sqlite3'
# Use Puma as the app server
gem 'puma', '~> 3.0'
# Use SCSS for stylesheets
gem 'sass-rails', '~> 5.0'
# Use Uglifier as compressor for JavaScript assets
gem 'uglifier', '>= 1.3.0'
# Use CoffeeScript for .coffee assets and views
gem 'coffee-rails', '~> 4.2'


### A usage that is current behavior
# Command is a description for a gem group.

# Authentication
gem 'devise'
gem 'omniauth'
gem 'omniauth-github'
gem 'omniauth-twitter'
gem 'omniauth-facebook'
# Web API
gem 'octokit', '~> 4.1.0'
gem 'koala', '~> 2.2'
gem 'twitter'

So, I think we should implement the proposed behavior as an option(EnforcedStyle).
And the proposed behavior should be used as default.

WDYT?

@konto-andrzeja
Copy link
Contributor

If nobody is working on this, I'll try to implement it.

@pocke
Copy link
Collaborator

pocke commented Mar 16, 2017

@konto-andrzeja Thanks!!! I don't have time for this issue, so that is very helpful.

jonas054 added a commit that referenced this issue Mar 25, 2017
[Fix #4004] Allow not treating comment lines as gem group separators in `Bundler/OrderedGems` cop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants