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 #7320] support Naming/MethodName for attr_reader / attr_writer / attr_accessor #7372
Conversation
I still don't see any tests covering the use of |
|
||
MSG = 'Use %<style>s for method names.' | ||
|
||
def_node_matcher :attr?, <<~PATTERN |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Btw, there's also define_method
that should be considered. In general it might be a good idea be able to provide a list of method names that define methods via configuration so custom macros could be accounted as well.
@bbatsov added. But.... It works only when you pass symbols, but should it also support strings ? |
I guess it should handle strings as well. |
it 'registers an offense for camel case methods names in attr.' do | ||
expect_offense(<<~RUBY) | ||
attr :my_method, :myMethod | ||
^^^^^^^^^^^^^^^^^^^^^ Use #{enforced_style} for method names. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we highlight just the second symbol here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will check it. Do you know maybe some cops that have similar behavior?
So it should be something like that ?
attr :my_method, :myMethod, my_snake_method, :myCamelMethod
^^^^^^^^^^ ^^^^^^^^^^^^^^ Use #{enforced_style} for method names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep. See #7385
…wasn't caught by rubocop but will in the future with this rubocop/rubocop#7372
Resolves #7320
Before submitting the PR make sure the following are checked:
[Fix #issue-number]
(if the related issue exists).master
(if not - rebase it).bundle exec rake default
. It executes all tests and RuboCop for itself, and generates the documentation.