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

Quote choice is ignored if inside an interpolated string #1181

Closed
jlabarbera11 opened this issue Jun 28, 2014 · 8 comments
Closed

Quote choice is ignored if inside an interpolated string #1181

jlabarbera11 opened this issue Jun 28, 2014 · 8 comments
Assignees

Comments

@jlabarbera11
Copy link

This is relative to the StringLiteral cop. When the enforced style for this is set to double_quotes, the string:

"hello#{hash['there']}"

should produce an error, since single quotes were used when not necessary. However, rubocop does not recognize it as such, since 'there' is part of an interpolated string, and thus ignored. This suggests that checking whether or not strings are a part of an ignored node is being done in the wrong place.

@jonas054
Copy link
Collaborator

Yes I guess we forgot about this case when implementing StringLiterals. On the other hand I think that even if double_quotes is set, many people would prefer

"hello#{hash['there']}"

over

"hello#{hash["there"]}"

since the latter looks kind of weird. We can even come up with code that looks even stranger with inner double quotes:

"#{s+"}"}"

My suggestion is that we leave the implementation as it is, but add a spec example that demonstrates how this case is handled.
Cc: @bbatsov @yujinakayama

@guilhermesimoes
Copy link
Contributor

Has this been "fixed" in the meantime?

The following code:

"#{systems[0...-1].join(', ')} or #{systems[-1]}"

is yielding the error:

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

But I don't want to use join(", ").

Like @jonas054 said, I'd definitely prefer

"hello#{hash['there']}"

over

"hello#{hash["there"]}"

Please make this configurable :)

@jonas054 jonas054 self-assigned this Oct 7, 2014
@guilhermesimoes
Copy link
Contributor

Thanks @jonas054! 👍

@guilhermesimoes
Copy link
Contributor

So, I just tried this out and the example previously discussed is not working.

With the configuration:

Style/StringLiterals:
  EnforcedStyle: double_quotes

Style/StringLiteralsInInterpolation:
  EnforcedStyle: single_quotes

And the code:

"hello#{hash['there']}"

An offense is still registered, but it shouldn't.

The issue is with StringLiterals though, not StringLiteralsInInterpolation.

@jonas054
Copy link
Collaborator

Yep, still not right. I'll have to fix that.

@jonas054 jonas054 reopened this Nov 13, 2014
bbatsov added a commit that referenced this issue Nov 13, 2014
[Fix #1181] Don't report strings in interpolations in StringLiterals
@guilhermesimoes
Copy link
Contributor

You are restless @jonas054. Thanks once again! 👍

@dpo
Copy link

dpo commented Nov 16, 2014

I've been trying to get the same configuration to work. I'm using 0.27.1. When will this be added to the gem?

@bbatsov
Copy link
Collaborator

bbatsov commented Nov 16, 2014

I doubt we'll do a new release before Christmas, but you can build the gem directly from the github master until then.

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

5 participants