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

Interpolated variables not enclosed in braces are not noticed #59

Closed
Sleft opened this issue Apr 16, 2013 · 5 comments
Closed

Interpolated variables not enclosed in braces are not noticed #59

Sleft opened this issue Apr 16, 2013 · 5 comments

Comments

@Sleft
Copy link

Sleft commented Apr 16, 2013

When you interpolate global, instance or class variables in strings you can omit the enclosing braces. But RuboCop does not notice variables not enclosed in braces and gives a misleading warnings of

Prefer single-quoted strings when you don't need string interpolation or special symbols.

as double-quotation is required for the strings in question.

For example, consider the following code:

# -*- coding: utf-8 -*-

class Klass
  def meth
    @bar = @@bar = $bar = 'baz'
    puts "foo#@bar"
    puts "foo#@@bar"
    puts "foo#$bar"
  end
end

When inspecting this rubocop says

C:  5: Replace class var @@bar with a class instance var.
C:  6: Prefer single-quoted strings when you don't need string interpolation or special symbols.
C:  7: Prefer single-quoted strings when you don't need string interpolation or special symbols.
C:  7: Replace class var @@bar with a class instance var.
C:  8: Prefer single-quoted strings when you don't need string interpolation or special symbols.

But replacing the strings with single-quoted ones will not allow for interpolation.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 16, 2013

This certainly is a bug, that we'll address soon. On a related note - interpolating variables like this is considered bad style(it's a remnant of the ancient Ruby history), so we should add a check for this as well.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 16, 2013

@jonas054 Here's where the problem with the existing check lies:

[3] pry(main)> Ripper.lex('puts "test#@@top"')
=> [[[1, 0], :on_ident, "puts"],
 [[1, 4], :on_sp, " "],
 [[1, 5], :on_tstring_beg, "\""],
 [[1, 6], :on_tstring_content, "test"],
 [[1, 10], :on_embvar, "#"],
 [[1, 11], :on_cvar, "@@top"],
 [[1, 16], :on_tstring_end, "\""]]
[4] pry(main)> Ripper.lex('puts "test#{@@top}"')
=> [[[1, 0], :on_ident, "puts"],
 [[1, 4], :on_sp, " "],
 [[1, 5], :on_tstring_beg, "\""],
 [[1, 6], :on_tstring_content, "test"],
 [[1, 10], :on_embexpr_beg, "\#{"],
 [[1, 12], :on_cvar, "@@top"],
 [[1, 17], :on_embexpr_end, "}"],
 [[1, 18], :on_tstring_end, "\""]]

The code in StringLiterals currently checks only for :on_embexpr_beg.

@jonas054
Copy link
Collaborator

Thanks, I'll try to solve it tonight.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 16, 2013

That won't be needed - I got a bit of an inspiration and fixed it myself :-) I've also implemented a cop to warn against the use of interpolated variables.

@jonas054
Copy link
Collaborator

Great!

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

3 participants