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

[New Cop] Style/PreferInterpolation #5067

Closed
dkniffin opened this issue Nov 16, 2017 · 8 comments
Closed

[New Cop] Style/PreferInterpolation #5067

dkniffin opened this issue Nov 16, 2017 · 8 comments

Comments

@dkniffin
Copy link
Contributor

@dkniffin dkniffin commented Nov 16, 2017

I'd like to request a new cop, similar to eslint's prefer-template rule. It would check for places where string interpolation could be used in place of concatenation.

Here's the "bad/good" example:

# bad
a = "abc"
b = a + "def"

# good
a = "abc"
b = "#{a} def"
@rrosenblum
Copy link
Contributor

@rrosenblum rrosenblum commented Nov 16, 2017

This sounds like a good idea to me. For the most part, I think this would involve checking both sides of a call to +. If one of them is a string and the other is a variable, interpolation can be used. Can anyone think of any potential edge cases that could cause issues?

@akhramov
Copy link
Contributor

@akhramov akhramov commented Nov 18, 2017

@rrosenblum the left hand side matters the most. In JS a + "b" will always result in string due to fancy type conversions.

Here's the real world example on why the things are not so bright in Ruby. Consider Pathname#+:

home = Pathname.new('/Users/jane')
home + 'Documents' # => #<Pathname:/Users/jane/Documents>
"#{home}Documents" # => "/Users/janeDocuments"

That being said, the left hand side must be a string.

@rrosenblum
Copy link
Contributor

@rrosenblum rrosenblum commented Nov 18, 2017

@akhramov thanks for the example with Pathname. That should help prevent several issues after this is implemented.

That being said, the left hand side must be a string.

That should make this easier to implement.

@dkniffin
Copy link
Contributor Author

@dkniffin dkniffin commented Nov 18, 2017

Hmm, it's shame it can't be checked on both sides, but that is a valid point, @akhramov

@rrosenblum
Copy link
Contributor

@rrosenblum rrosenblum commented Nov 29, 2017

I thought of a few more things that we will need to account for. We will want to make sure that the correction does not exceed the configured line length. This should only be an issue when the offense spans more than one line to begin with. We will also need to make sure that we don't change the meaning of the string '\t' vs "\t". Some of these checks have already been implemented in other cops.

@rjurado01
Copy link

@rjurado01 rjurado01 commented Jul 25, 2018

Any new on this matter? This rule is included in the styleguide but not cop about it.

@bbatsov
Copy link
Collaborator

@bbatsov bbatsov commented Jul 25, 2018

I'm hoping someone's going to contribute it. I don't have time to work on this myself now.

@koic koic added the feature request label Aug 1, 2018
@bbatsov
Copy link
Collaborator

@bbatsov bbatsov commented Sep 15, 2018

There hasn't been much activity on this ticket and our Core Team is spread too thin on so many tasks, so I'll just close it for the sake of having a cleaner lists of tasks to focus on. We'd gladly review a PR, but it's unlikely that'd we're going to tackle this ourselves in the foreseeable future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

6 participants
You can’t perform that action at this time.