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 false negatives for Rails/EnumHash cop #96

Merged
merged 1 commit into from Jul 26, 2019
Merged

Conversation

pocke
Copy link
Contributor

@pocke pocke commented Jul 26, 2019

Rails/EnumHash cop has been introduced in #94. Thanks!

But the cop has two false negatives. This pull request will fix them.

with non-symbol key

This cop does not detect enum call with non-symbol key. For example:

KEY = :status
enum KEY => %i[active archived]

with multiple keys

enum allows defining multiple enums in one enum method call.

enum status: %i[active archived],
     role: %i[owner member]

But the cop is not aware of the code.

Note

The cop has not been released yet, so I did not write any changelog for the pull request.

I wait merging #95 to review this pull request. Thanks.


Before submitting the PR make sure the following are checked:

  • Wrote good commit messages.
  • Commit message starts with [Fix #issue-number] (if the related issue exists).
  • Feature branch is up-to-date with master (if not - rebase it).
  • Squashed related commits together.
  • Added tests.
  • Added an entry to the Changelog if the new code introduces user-observable changes. See changelog entry format.
  • The PR relates to only one subject with a clear title
    and description in grammatically correct, complete sentences.
  • Run bundle exec rake default. It executes all tests and RuboCop for itself, and generates the documentation.

@@ -10,7 +10,7 @@
it 'registers an offense' do
expect_offense(<<~RUBY)
enum status: %i[active archived]
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead.
^^^^^^^^^^^^^^^^^^^ Enum defined as an array found in `status` enum declaration. Use hash syntax instead.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum status: is not a bad part, so I think the cop should add an offense only to the array.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement ❤️

@koic koic merged commit 51c57d8 into rubocop:master Jul 26, 2019
@koic
Copy link
Member

koic commented Jul 26, 2019

Thanks!

@pocke pocke deleted the enumhash branch July 26, 2019 03:25
@santib
Copy link
Contributor

santib commented Jul 26, 2019

@pocke Nice improvements! Given we based the Rails/EnumHash cop in Rails/EnumUniqueness, I guess these improvements apply for that cop too. Thoughts? cc: @koic

@pocke
Copy link
Contributor Author

pocke commented Jul 28, 2019

@pocke Nice improvements! Given we based the Rails/EnumHash cop in Rails/EnumUniqueness, I guess these improvements apply for that cop too. Thoughts? cc: @koic

It is a good idea! Thank you.
Can you open a pull request for the cop? I'm busy this week, so I'll open it next week if I do that.

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

Successfully merging this pull request may close these issues.

None yet

3 participants