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

[Fix #6349] MemoizedInstanceVariableName allows underscored method name #6587

Conversation

matthewrudy
Copy link

@matthewrudy matthewrudy commented Dec 19, 2018

We should be allowed to write code like this:

def _foo
  @_foo ||= :foo
end

namely if the method name has a leading _, the memoized variable name should have it too.
Currently it gives an error, saying the variable should be named @foo.

This should fix #6349

…thod name

The following code should be valid:

    def _foo
      @_foo ||= :foo
    end

But before this change it complains that the variable name should be @foo.

The rules need to take into account what the method name is,
and the configured style.
@matthewrudy matthewrudy force-pushed the memoized-instance-variable-same branch from 8624ded to 2804f06 Compare December 19, 2018 12:18
@bbatsov
Copy link
Collaborator

bbatsov commented Mar 17, 2019

Hmm, I thought we have fixed this already. Seems to me that's the 3rd time I see some issue about fixing vars with underscores. 😆

@matthewrudy
Copy link
Author

@bbatsov maybe so.

Was still a problem when I did this
(Which was prompted by an earlier issue with the same thing that I worked around)

If it's fixed on master, then fairdos.

Appreciate you taking a look.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 3, 2019

The changes look good, but your branch has to be rebased on top of the current master branch due to merge conflicts. And add an entry in the changelog.

@stale
Copy link

stale bot commented May 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

@stale stale bot added the stale Issues that haven't been active in a while label May 8, 2019
@stale
Copy link

stale bot commented Jun 7, 2019

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

@stale stale bot closed this Jun 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues that haven't been active in a while
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MemoizedInstanceVariableName Still complaining about leading underscores
2 participants