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

Rails-compatible indentation of private and protected methods #1342

Closed
gregnavis opened this issue Sep 22, 2014 · 16 comments
Closed

Rails-compatible indentation of private and protected methods #1342

gregnavis opened this issue Sep 22, 2014 · 16 comments
Assignees

Comments

@gregnavis
Copy link

Problem

Currently Rubocop can check the indentation of private, protected and public modifiers. Unfortunately it isn't able to check whether the code that follows uses the Rails convention of indenting private and protected methods with respect to private and protected. For example the style

class MyClass
  def a_public_method
  end

  private

    def a_private_method
    end
end

should be supported.

Solution

The Style/AccessModifierIndentation cop could support a new EnforcedStyle called rails that would use the Rails convention.

rubocop -V returns

0.26.1 (using Parser 2.2.0.pre.4, running on ruby 2.0.0 x86_64-linux)
@bbatsov
Copy link
Collaborator

bbatsov commented Sep 22, 2014

Isn't the Rails convention actually:

class MyClass
  def a_public_method
  end

private

  def a_private_method
  end
end

@gregnavis
Copy link
Author

The guide isn't clear. It just says Indent after private/protected. I just browsed the Rails source code a little bit and it uses all three styles: indent, dedent and the one I described in the issue. An example of the last one is https://github.com/rails/rails/blob/master/activerecord/lib/active_record/aggregations.rb#L230.

Either way I'd like to enforce this style in my project and it'd be great if Rubocop gave me a hand.

@mikegee
Copy link
Contributor

mikegee commented Sep 22, 2014

I wouldn't want to call your preferred style the "rails" style for the reasons below. I can't think of a good name though. (Naming things is hard.)

  • Their guide isn't clear on the indent level on the private/protected keywords.
  • Rails' source isn't consistent.
  • I initially interpreted the "rails" style as the style prescribed to Rails applications, not "like Rails' source".

@gregnavis
Copy link
Author

Absolutely agree, @mikegee. Initially I assumed that this is the style used in Rails source code but in reality it's very inconsistent. I must admit that I don't have an idea for a nice name. double_indent, maybe?

@dzhlobo
Copy link

dzhlobo commented Feb 1, 2015

I would like to implement this. In my style guide it may looks like:

  1. Don't use explicit public access modifier. Keep all public methods on top of class defenition.
  2. Write protected and private keywords at the same level of indentation as public methods.
  3. Indent protected and private methods relative to protected and private keywords.

Example:

class Cat
  def meow
    puts('Meow!')
  end

  protected

    def can_we_be_friends?(another_cat)
      # some logic
    end

  private

    def meow_at_3am?
      rand < 0.8
    end
end

As I understand changes could affect Cop::Style::IndentationConsistency and Cop::Style::IndentationWidth. I'm not sure where to store config for this. I think about:

Style/IndentationConsistency:
  AllowProtectedAndPrivateMethodsBeIndented: true

but is it ok if I will check this config in IndentationWidth cop?

@bbatsov can you please help me? Will it be merged?

@fgarcia
Copy link

fgarcia commented Feb 2, 2015

Initially I assumed that this is the style used in Rails source code but in reality it's very inconsistent.

@mikegee. I thought that Rails source code was inconsistent because they could not use Rubocop goodness to solve it 😌

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 2, 2015

I'm OK with adding support for the suggested style. Not sure what'd be best regarding IndentationConsistency. Guess @jonas054 will be of more help regarding this.

@dzhlobo
Copy link

dzhlobo commented Feb 11, 2015

@jonas054 What do you think about it?

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 11, 2015

@jonas054 is on vacation. He'll be back in about a month.

@dzhlobo
Copy link

dzhlobo commented Feb 11, 2015

Oh, ok. Thank you for response, Bozhidar @bbatsov.

On Feb 11, 2015, at 4:25 PM, Bozhidar Batsov notifications@github.com wrote:

@jonas054 https://github.com/jonas054 is on vacation. He'll be back in about a month.


Reply to this email directly or view it on GitHub #1342 (comment).

@jonas054
Copy link
Collaborator

@Proghat Go ahead and start coding if you like. I'll try to review your work if you submit a PR. Some thoughts:

  • Just check which cops report offenses for your style. Those are the ones you need to make changes in.
  • Yes, it's okay to store configuration in one cop and use it from several cops.
  • Don't allow two styles at the same time. You used the word "allow" in the parameter name but we should always enforce one style or another, not leave room for preferences within one style.

@lion-man44
Copy link

Hello.
How's the hear?

@jonas054 jonas054 self-assigned this Mar 29, 2015
0xazure added a commit to WorkingScholar/workingscholar-alpha that referenced this issue Apr 5, 2015
Rubocop doesn't (yet) support the Rails style of private method
indentation (see rubocop/rubocop#1342).
0xazure added a commit to WorkingScholar/workingscholar-alpha that referenced this issue Apr 5, 2015
Rubocop doesn't (yet) support the Rails style of private method
indentation (see rubocop/rubocop#1342).
0xazure added a commit to WorkingScholar/workingscholar-alpha that referenced this issue Apr 5, 2015
Rubocop doesn't (yet) support the Rails style of private method
indentation (see rubocop/rubocop#1342).
bbatsov added a commit that referenced this issue Apr 5, 2015
0xazure added a commit to WorkingScholar/workingscholar-alpha that referenced this issue Apr 11, 2015
Rubocop doesn't (yet) support the Rails style of private method
indentation (see rubocop/rubocop#1342).
0xazure added a commit to WorkingScholar/workingscholar-alpha that referenced this issue Dec 11, 2015
Rubocop doesn't (yet) support the Rails style of private method
indentation (see rubocop/rubocop#1342).
@SaimonL
Copy link

SaimonL commented Aug 21, 2017

This rubocop is designed for pure Ruby. Rails is a full framework and uses different style guides all together. Just use Rail generator and you will see the style is different, which will not change for 1000 years and same goes for the community. For Rails please use the gem "rubocop-rails" instead with will work with the '.rubocop.yml' file. For more visit https://github.com/bbatsov/rubocop

@bbatsov
Copy link
Collaborator

bbatsov commented Aug 21, 2017

This rubocop is designed for pure Ruby. Rails is a full framework and uses different style guides all together. Just use Rail generator and you will see the style is different, which will not change for 1000 years and same goes for the community. For Rails please use the gem "rubocop-rails" instead with will work with the '.rubocop.yml' file. For more visit https://github.com/bbatsov/rubocop

Internally they can use whatever code style they want, but no framework defines the code style of a project.

@carlomartinucci
Copy link

carlomartinucci commented Nov 30, 2017

For those who came here looking for a solution (noobs like me), just use:

Layout/IndentationConsistency:
  EnforcedStyle: rails

EDIT 2019/08/01:
As @Molszew pointed out, as of Rubocop v0.72 you now have to use indented_internal_methods instead of rails. Thanks for the update.

@Molszew
Copy link

Molszew commented Aug 1, 2019

Since Rubocop v0.72 EnforcedStyle: rails have beed renamed. Use this instead:

Layout/IndentationConsistency:
  EnforcedStyle: indented_internal_methods

boddhisattva added a commit to boddhisattva/dotfiles that referenced this issue Mar 22, 2020
As mentioned here:
rubocop/rubocop#1342 (comment)
 this setting is now updated to use a different value to enforce the
intended style
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

10 participants