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

Cop idea: string_literal.to_sym #9353

Closed
marcandre opened this issue Jan 8, 2021 · 7 comments · Fixed by #9362
Closed

Cop idea: string_literal.to_sym #9353

marcandre opened this issue Jan 8, 2021 · 7 comments · Fixed by #9362

Comments

@marcandre
Copy link
Contributor

I was surprised to see that RuboCop doesn't pickup on this in a recent PR:

# bad
'string-literal'.to_sym
# good
:'string-literal'
@koic
Copy link
Member

koic commented Jan 8, 2021

I think this can be suggested to the Ruby style guide :-)

@bbatsov
Copy link
Collaborator

bbatsov commented Jan 8, 2021

I guess this cop should also check for cases like 'string_literal'.to_sym and :'string_literal', as obviously those should have been :string_literal.

@marcandre
Copy link
Contributor Author

@deepj Why not? keys may start with uppercase, can also be ruby keywords:

h = {
  ABC: 42,
  class: :hello,
  end: 666,
} # => no problem

@dvandersluis
Copy link
Member

Exactly what @marcandre said. 'UAE': 'United Arab Emirates' maybe looks like the key will be a string, but it is already a symbol and the quotes are redundant.

{ 'UAE': 'United Arab Emirates' }
=> {:UAE=>"United Arab Emirates"}

@dvandersluis
Copy link
Member

dvandersluis commented Jan 29, 2021

The cop doesn't change states['UAE'] to states[:UAE], so you can still access your hash the same as you were previously. The only difference is that the extraneous quotes are removed from the symbol itself (again, your key is not a string, but a symbol, despite the quotes). I don't agree that hashes should not be processed, as I would much rather than UAE: foo than 'UAE': foo.

:'UAE' == :UAE
=> true
:'UAE' == 'UAE'
=> false

@marcandre
Copy link
Contributor Author

@deepj Maybe you meant to write {'UAE' => 'United Arab Emirates'}? IMO, user facing labels (like 'UAE') should be strings, not symbols.

Let's try and keep HashWithIndifferentAccess out of this, because I can't be partial about this (it's an abomination that was always a bad idea that is no longer needed with strong params)

@deepj
Copy link
Contributor

deepj commented Jan 29, 2021

I apologize, I'm just mixed up several things together.

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 a pull request may close this issue.

5 participants