Style/AccessModifierDeclarations reporting private attr_readers

[simonc]

Hi,

I'm facing an issue with Style/AccessModifierDeclarations when choosing the inline mode while declaring private accessors in a class. I understand it is a tricky situation but I had to disable the cop since it was reporting false positives.

---

Expected behavior

This is the style we use is something like this since attr_… returns nil:

class Greeter
  def initialize(greeting)
    @greeting = greeting
  end

  def call(name)
    puts build_message(name)
  end

  private def build_message(name)
    "#{greeting} #{name}"
  end

  private

  attr_reader :greeting
end

(I know this code makes no sense, it's just for the sake of the example)

Here is our .rubocop.yml

Style/AccessModifierDeclarations:
  EnforcedStyle: inline

I would expect not to get any offense reported since there is not def involved after the private call.

Actual behavior

Rubocop reports this offense as follows

Style/AccessModifierDeclarations: private should be inlined in method definitions.
  private
  ^^^^^^^

Steps to reproduce the problem

Run rubocop with the above config on a file containing the above code.

RuboCop version

$ rubocop -V
0.57.2 (using Parser 2.5.1.0, running on ruby 2.5.1 x86_64-darwin17)

[wata727]

I guess inline style enforce attr_reader declaration like the following:

attr_reader :greeting
private :greeting

See also #5953 (duplicate?)

[brandonweiss]

@simonc I don’t think that’s actually a false positive… @wata727 has an example of how you would do it!

[simonc]

@wata727 @brandonweiss I understand the idea but it would mean a lot more code to write like

attr_accessor :a, :b, :c
private :a
private :a=
private :b
private :b=
private :c
private :c=

That seems a bit cumbersome.
I think it'd be beneficial to allow for some "mixed" styling but I understand it makes for a much more complicated check.

[brandonweiss]

@simonc I think that’s really the point of it. inline is a much more explicit, verbose style. Some people think the benefits outweigh the repetition… but if you don’t then you should probably use the group style.

Using a “mixed” style gets extremely fiddly. I’m not necessarily concerned about the complexity of the logic to do the check, I’d be more concerned about the ability for people to understand when/how to follow the rule. I think if someone wants a mixed style they basically just want to disable the cop entirely.

[simonc]

@brandonweiss

inline is a much more explicit, verbose style

I don't entirely agree. Yes you're adding private before every def but it doesn't add any line of code. I'm wondering how many inliners write their private readers the verbose way.

Anyway, I guess you're right, I'll just disable the cop and move on ^^

Thanks