Rubocop: Naming/MemoizedInstanceVariableName Cop doesn't work with methods that have a leading underscore

[ashmaroli]

jekyll/convertible.rb has a private method named _renderer which memoizes using the ivar @_renderer as follows:

    private

    def _renderer
      @_renderer ||= Jekyll::Renderer.new(site, self)
    end

---

Expected behavior

Rubocop should not flag the above code when I configure the Naming/MemoizedInstanceVariableName with the provided directives for EnforcedStyleForLeadingUnderscores:

Actual behavior

1. Configured with EnforcedStyleForLeadingUnderscores: disallowed

Naming/MemoizedInstanceVariableName:
  EnforcedStyleForLeadingUnderscores: disallowed

Offenses:

lib/jekyll/convertible.rb:244:7: C: Naming/MemoizedInstanceVariableName: Memoized variable
@_renderer does not match method name _renderer. Use @_renderer instead.
      @_renderer ||= Jekyll::Renderer.new(site, self)
      ^^^^^^^^^^



md5-623c13b787990d6f0517f2208a773e92



Naming/MemoizedInstanceVariableName:
  EnforcedStyleForLeadingUnderscores: required



md5-5aa8b70ec666ca70883896e7506688f2



Offenses:

lib/jekyll/convertible.rb:244:7: C: Naming/MemoizedInstanceVariableName: Memoized variable
@_renderer does not match method name _renderer. Use @_renderer instead.
      @_renderer ||= Jekyll::Renderer.new(site, self)
      ^^^^^^^^^^



md5-033e9d4b12ebf14aad3b24e10723bec3



Naming/MemoizedInstanceVariableName:
  EnforcedStyleForLeadingUnderscores: optional



md5-5aa8b70ec666ca70883896e7506688f2



Offenses:

lib/jekyll/convertible.rb:244:7: C: Naming/MemoizedInstanceVariableName: Memoized variable
@_renderer does not match method name _renderer. Use @_renderer instead.
      @_renderer ||= Jekyll::Renderer.new(site, self)
      ^^^^^^^^^^



md5-c3e441410e337bf5b611db264c55f22d



$ rubocop -V
0.58.0

[Drenmi]

Problem is the cop does not expect methods starting with _.

[kenman345]

@Drenmi I am trying to understand so maybe I can help fix this issue. Why would a setting for the cop exist that is specifically for handling underscores yet it is not expecting to see underscores? How does code like that get added?

Can you help direct me towards where the issue might be that I can try and resolve it? I am not familiar with rubocop yet and it would save me a little time to get some direction on this.

[kenman345]

Nevermind, I think I found the area, but still confused why a new option wouldnt actually work or have a test proving it works before its added to a release.

[Drenmi]

@kenman345 The setting expects a prefix underscore in the variable name, but doesn't expect it in the method name. I.e. it expects this:

def foo
  @_foo ||= ...
end

and it checks it by stripping the underscore from the variable name (only) and comparing it to the unaltered method name. So in the case of an underscore method, it compares foo (variable name) to _foo (method name.)

[ashmaroli]

@bbatsov This issue has not been resolved as expected.
It is reproducible with the latest rubocop:master

Edit: Nevermind. EnforcedStyleForLeadingUnderscores defaults to disallowed. So, its expected.. :+1:

[timoschilling]

This is still an issue, 0.58.2 didn't fix this issue in my case:

    def _warden_user
      @_warden_user ||= _warden.authenticate(scope: :user)
    end

authentication.rb:40:7: C: Naming/MemoizedInstanceVariableName: Memoized variable @_warden_user does not match method name _warden_user. Use @_warden_user instead.
      @_warden_user ||= _warden.authenticate(scope: :user)
      ^^^^^^^^^^^^^

[kenman345]

That’s a bit different as your method name is in fact different from the
variable name. This is not a cop purely to enforce underscores for
memoization

On Tue, Jul 24, 2018 at 7:16 AM Timo Schilling [e-mail]
wrote:

This is still an issue, 0.58.2 didn't fix this issue in my case:

def _warden_user
  @_warden_user ||= _warden.authenticate(scope: :user)
end

authentication.rb:40:7: C: Naming/MemoizedInstanceVariableName: Memoized variable @_warden_user does not match method name _warden_user. Use @_warden_user instead.
@_warden_user ||= _warden.authenticate(scope: :user)
^^^^^^^^^^^^^

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rubocop-hq/rubocop/issues/6084#issuecomment-407370135,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKeM1wMOn6YL3urjESTsIaXUzAECm7V2ks5uJwIKgaJpZM4VGnqb
.

[timoschilling]

I don't see a difference between _warden_user and @_warden_user

[ashmaroli]

@timoschilling you've to override the Rubocop configuration as well:

# .rubocop.yml
 Naming/MemoizedInstanceVariableName:
   EnforcedStyleForLeadingUnderscores: optional

[kenman345]

Sorry, I think that it’s using the memoization statement as the method def
and variable names there. Odd right? Try the optional enforcement and see
what happens. Can you provide your repo link and I can give it a try?
Otherwise I’ll just add in a method like that and see what’s being matched.

On Tue, Jul 24, 2018 at 7:29 AM Ashwin Maroli [e-mail]
wrote:

@timoschilling https://github.com/timoschilling you've to override the
Rubocop configuration as well:

.rubocop.yml

Naming/MemoizedInstanceVariableName:
EnforcedStyleForLeadingUnderscores: optional

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rubocop-hq/rubocop/issues/6084#issuecomment-407373197,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKeM182aOpfBGYKN0TL5AatYkDH8mzdnks5uJwUIgaJpZM4VGnqb
.

[timoschilling]

simply use this:

class Test
  def ok
    @ok ||= 1
  end

  def _not_ok
    @_not_ok ||= 1
  end
end

$ bundle exec rubocop -v
0.57.2
$ bundle exec rubocop
Inspecting 1 files
.

1 files inspected, no offenses detected

$ bundle exec rubocop -v
0.58.2
$ bundle exec rubocop
Inspecting 1 files
C

Offenses:

test.rb:7:5: C: Naming/MemoizedInstanceVariableName: Memoized variable @_not_ok does not match method name _not_ok. Use @_not_ok instead.
    @_not_ok ||= 1
    ^^^^^^^^

1 files inspected, 1 offense detected

[kenman345]

I’m not at my Pc right now but that seems off. That’s basically the exact
thing we have in the spec tests for my PR.

Is this with Ash’s recommendation to configure it? Just want to know
everything before testing when I have the chance

On Tue, Jul 24, 2018 at 7:46 AM Timo Schilling [e-mail]
wrote:

simply use this:

class Test
def ok
@ok ||= 1
end

def _not_ok
@_not_ok ||= 1
endend

$ bundle exec rubocop -v
0.57.2
$ bundle exec rubocop
Inspecting 1 files.

1 files inspected, no offenses detected

$ bundle exec rubocop -v
0.58.2
$ bundle exec rubocop
Inspecting 1 files
C

Offenses:

test.rb:7:5: C: Naming/MemoizedInstanceVariableName: Memoized variable @_not_ok does not match method name _not_ok. Use @_not_ok instead.
@_not_ok ||= 1
^^^^^^^^

1 files inspected, 1 offense detected

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/rubocop-hq/rubocop/issues/6084#issuecomment-407377397,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AKeM1ykPW-5U7-67fokb5bMFQYRiHAjUks5uJwkdgaJpZM4VGnqb
.

[timoschilling]

It's without any config.

[timoschilling]

Now I have read https://github.com/rubocop-hq/rubocop/blob/4ed9aab2b2ba8f1a004c58e5765940540742623d/manual/cops_naming.md#namingmemoizedinstancevariablename.
And I still think it is a bug.

EnforcedStyleForLeadingUnderscores: disallowed (default)
Means the variable must be qual the method name.
This is the case in my code and config. But Rubocop detects a offense. That is a bug.

Sure it works with optional but it should work with required too.

And even the error message shows up a bug:

Memoized variable @_not_ok does not match method name _not_ok. Use @_not_ok instead.
The given and the suggested var name are the same.

[timoschilling]

I have checked the tests and my case isn't covered, I will provide a PR with the (failling) example for that case.