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

Style/HashAsLastArrayItem when hash is only array item #8409

Closed
honigc opened this issue Jul 28, 2020 · 3 comments
Closed

Style/HashAsLastArrayItem when hash is only array item #8409

honigc opened this issue Jul 28, 2020 · 3 comments

Comments

@honigc
Copy link

honigc commented Jul 28, 2020

I recently started using Style/HashAsLastArrayItem and was surprised to see that it flags an implicit hash that's the only element of an array as a violation, e.g. insisting that [key: value] be written as [{ key: value }].

I understand the reasoning; technically, the only item is also the last item. I don't see as much value in enforcing the rule in this case, though, since the ambiguity that can be present when there's an implicit hash in a multi-element array doesn't seem to be there when it's just a single item.

Is this case intended to be flagged by this cop? I'd like to use this cop because I see value in the multi-item case, but the flagging of the single-item case feels almost like a false positive and reduces the overall value I get from this cop.

@rrosenblum
Copy link
Contributor

Personally, I feel like this should register an offense in this situation. Technically, one item is also the last item. We could introduce a configuration option to allow this use case. I kind of feel like the name of the cop doesn't fully capture the intention behind the cop itself. Something ImplicitHashInArray sounds more accurate to me. I feel like the implicit hash inside of arrays can be confusing to read because it isn't immediately obvious what is going on. I personally always want braces around the hash in an array because an explicit hash is easier to read.

@honigc
Copy link
Author

honigc commented Aug 20, 2020

I agree about the name of the cop, and that if it were called ImplicitHashInArray it would be working as expected. If it's going to stay as HashAsLastArrayItem, I think making this configurable would be a nice, low-lift improvement that would let people use this cop to avoid ambiguous elements in multi-item arrays, while still being able to use condensed syntax for single-item arrays if they'd like.

@honigc
Copy link
Author

honigc commented Sep 11, 2020

I think #8635 resolves the issue here as well, since the cop will now ignore an array with a single hash element.

@honigc honigc closed this as completed Sep 11, 2020
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

No branches or pull requests

2 participants