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

Add new Style/HashArrayLastItem cop #8271

Merged
merged 1 commit into from Jul 9, 2020

Conversation

fatkodima
Copy link
Contributor

Closes #4286

@bbatsov
Copy link
Collaborator

bbatsov commented Jul 8, 2020

I think we should also mention this in the style guide with a suggestion to use braces for the sake of clarity.

config/default.yml Outdated Show resolved Hide resolved
@koic
Copy link
Member

koic commented Jul 8, 2020

Can you open a PR as the rule to the style guide?

@fatkodima
Copy link
Contributor Author

Changed name to HashAsLastArrayItem, opened rubocop/ruby-style-guide#826 and added link to that.

@marcandre
Copy link
Contributor

Looks good, but will you start using Cop::Base instead of Cop::Cop? In this particular case, I think that instead of having an if such_and_such_a_style three times, it would be more readable if there was a single such if

@fatkodima
Copy link
Contributor Author

I think that instead of having an if such_and_such_a_style three times, it would be more readable if there was a single such if

Didn't get it. Can you point to code?

@marcandre
Copy link
Contributor

Here's what I had in mind. What do you think of this style?

class HashAsLastArrayItem < Base
  include ConfigurableEnforcedStyle

  def on_hash(node)
    return unless node.parent&.array_type?

    if braces_style?
      check_braces(node)
    else
      check_no_brances(node)
    end
  end

  private

  def check_braces(node)
    return unless node.braces

    add_offense(node, message: 'Wrap hash in `{` and `}`.') do |corrector|
      corrector.wrap(node, '{', '}')
    end
  end

  def check_no_braces(node)
    return if node.braces

    add_offense(node, message: 'Omit the braces around the hash.') do |corrector|
      corrector.remove(node.loc.begin)
      corrector.remove(node.loc.end)
    end
  end

  def braces_style?
    style == :braces
  end
end

@fatkodima
Copy link
Contributor Author

Ah, got it. Looks nicer 👍
Updated with your suggestions.

@koic koic merged commit ad4d557 into rubocop:master Jul 9, 2020
@koic
Copy link
Member

koic commented Jul 9, 2020

Thanks!

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.

rule suggestion hash inside array
4 participants