Feature #6693: Don't warn for unused variables starting with _

• Status: Closed
• Assignee: matz (Yukihiro Matsumoto)
• Target version: 2.6

[marcandre (Marc-Andre Lafortune)]

Currently, variables which are set but not used will generate a warning (ruby -w), except if they have the special name "_".

So best practice is to use "_" for all unused variables. This does not encourage readable code.

# Currently must read:
_, _, _, suffix = parse_name

# could read:
_first, _middle, _last, suffix = parse_name

We should not warn for unused variables starting with a "_".

This would create an option (but no obligation) to use more descriptive names than "_" without generating warnings.

[regularfry (Alex Young)]

On 04/07/12 05:01, marcandre (Marc-Andre Lafortune) wrote:

Issue #6693 has been reported by marcandre (Marc-Andre Lafortune).

I don't know about anyone else, but I'm already using _foo as a pattern
for private-like markers. I think I picked it up from Python.

What I mean when I say "private-like" is "you can see this but shouldn't
have to touch it", or "I need a visible identifier here which won't
collide with your code". It often crops up with delegates or proxies
where I need to obviously separate out the methods on the intermediary
from the methods on the target. For example:

class VCR

  attr_reader :_calls

  def initialize( delegate )
    @delegate = delegate
    @_calls = []
  end

  def method_missing(sym, *args, &blk )
    @_calls << [sym, args, blk]
    @delegate.__send__( sym, *args, &blk )
  end

end

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

This issue was solved with changeset r36337.
Marc-Andre, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

---

[Feature #6693]

• parse.y (shadowing_lvar_gen, warn_unused_var): no warnings for
  variables starting with _. [ruby-core:46160][Feature #6693]

[ko1 (Koichi Sasada)]

これって入れるって決まったんでしたっけ．

(2012/07/08 7:42), nobu wrote:

nobu 2012-07-08 07:36:25 +0900 (Sun, 08 Jul 2012)

New Revision: 36337

http://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=rev&revision=36337

Log:
[Feature #6693]

* parse.y (shadowing_lvar_gen, warn_unused_var): no warnings for
  variables starting with _.  [ruby-core:46160][Feature #6693]

Modified files:
trunk/ChangeLog
trunk/parse.y
trunk/test/ruby/test_rubyoptions.rb

Index: ChangeLog

--- ChangeLog (revision 36336)
+++ ChangeLog (revision 36337)
@@ -1,3 +1,8 @@
+Sun Jul 8 07:36:19 2012 Nobuyoshi Nakada [e-mail]
+

• • parse.y (shadowing_lvar_gen, warn_unused_var): no warnings for

•  variables starting with _.  [ruby-core:46160][Feature #6693]
  

Sat Jul 7 23:07:30 2012 CHIKANAGA Tomoyuki [e-mail]

 * test/csv/test_features.rb: add require for Tempfile.

Index: parse.y

--- parse.y (revision 36336)
+++ parse.y (revision 36337)
@@ -8595,12 +8595,23 @@
#undef parser_yyerror
}

+static int
+is_private_local_id(ID name)
+{

• VALUE s;
• if (name == idUScore) return 1;
• if (!is_local_id(name)) return 0;
• s = rb_id2str(name);
• if (!s) return 0;
• return RSTRING_PTR(s)[0] == '_';
  +}

#define LVAR_USED ((int)1 << (sizeof(int) * CHAR_BIT - 1))

static ID
shadowing_lvar_gen(struct parser_params *parser, ID name)
{

• if (idUScore == name) return name;

• if (is_private_local_id(name)) return name;
  if (dyna_in_block()) {
  if (dvar_curr(name)) {
  yyerror("duplicated argument name");
  @@ -9324,7 +9335,7 @@
  }
  for (i = 0; i < cnt; ++i) {
  if (!v[i] || (u[i] & LVAR_USED)) continue;

• if (idUScore == v[i]) continue;

• if (is_private_local_id(v[i])) continue;
  rb_compile_warn(ruby_sourcefile, (int)u[i], "assigned but unused variable - %s", rb_id2name(v[i]));
  }
  }
  Index: test/ruby/test_rubyoptions.rb
  ===================================================================
  --- test/ruby/test_rubyoptions.rb (revision 36336)
  +++ test/ruby/test_rubyoptions.rb (revision 36337)
  @@ -497,7 +497,8 @@
  assert_in_out_err(["-we", "1.times do\n a=1\nend"], "", [], [], feature3446)
  assert_in_out_err(["-we", "def foo\n 1.times do\n a=1\n end\nend"], "", [], ["-e:3: warning: assigned but unused variable - a"], feature3446)
  assert_in_out_err(["-we", "def foo\n"" 1.times do |a| end\n""end"], "", [], [])

• assert_in_out_err(["-we", "def foo\n _a=1\nend"], "", [], ["-e:2: warning: assigned but unused variable - _a"], feature3446)

• feature6693 = '[ruby-core:46160]'
• assert_in_out_err(["-we", "def foo\n _a=1\nend"], "", [], [], feature6693)
  end

def test_shadowing_variable
@@ -509,11 +510,9 @@
["-e:3: warning: shadowing outer local variable - a",
"-e:2: warning: assigned but unused variable - a",
], bug4130)

• feature6693 = '[ruby-core:46160]'
  assert_in_out_err(["-we", "def foo\n"" _a=1\n"" 1.times do |_a| end\n""end"],

•                  "", [],
  

•                  ["-e:3: warning: shadowing outer local variable - _a",
  

•                   "-e:2: warning: assigned but unused variable - _a",
  

•                  ], bug4130)
  

•                  "", [], [], feature6693)
  

  end

  def test_script_from_stdin

--
ML: [e-mail]
Info: http://www.atdot.net/~ko1/quickml/

--
// SASADA Koichi at atdot dot net

[nobu (Nobuyoshi Nakada)]

なかだです。

At Mon, 9 Jul 2012 06:32:58 +0900,
SASADA Koichi wrote in [ruby-dev:45925]:

これって入れるって決まったんでしたっけ．

いや、どうでもよさそうな話だったので適当に突っ込んでみました。

--
--- 僕の前にBugはない。
--- 僕の後ろにBugはできる。
中田 伸悦

[kosaki (Motohiro KOSAKI)]

なかだです。

At Mon, 9 Jul 2012 06:32:58 +0900,
SASADA Koichi wrote in [ruby-dev:45925]:

これって入れるって決まったんでしたっけ．

いや、どうでもよさそうな話だったので適当に突っ込んでみました。

この話、反対意見があったと記憶してます。 [ruby-core:46170]
privateっぽいやつに _ つかって見分けやすくすると言うコーディング規約は
納得できるものですし、議論が収束する前にいきなりコミットしてしまうのは
ちょっと感じ悪いのではないでしょうか。せめて ruby-coreでなぜ元提案に
賛成なのかを説明してからコミットして欲しいです。

個人的には

first, middle, last, suffix = parse_name
_ = first = middle = last

ではだめな理由がよく分かりませんでしたが、どっちの結論になっても反対はしません。

[nobu (Nobuyoshi Nakada)]

• Status changed from Closed to Feedback
• % Done changed from 100 to 50

Sorry, missed [ruby-core:46170].

Now I incline to revert the commit. Any idea?

[marcandre (Marc-Andre Lafortune)]

Hi,

regularfry (Alex Young) wrote:

I don't know about anyone else, but I'm already using _foo as a pattern
for private-like markers.

I don't think that it would be confusing. We don't confuse variables with methods already, right?

Also, I feel the convention for this already exists with two underscore before and after the method, in particular SimpleDelegate#getobj and #setobj

Everyone is of course free to use their own. In any case, is it frequent that we delegate to an object for which we don't know the methods in advance?

[regularfry (Alex Young)]

On 09/07/12 14:54, marcandre (Marc-Andre Lafortune) wrote:

Issue #6693 has been updated by marcandre (Marc-Andre Lafortune).

Hi,

regularfry (Alex Young) wrote:

I don't know about anyone else, but I'm already using _foo as a
pattern for private-like markers.

I don't think that it would be confusing. We don't confuse variables
with methods already, right?

You might not, but it's a common gotcha with local variables and private
methods.

Also, I feel the convention for this already exists with two
underscore before and after the method, in particular
SimpleDelegate#getobj and #setobj

That's true, it does exist, but in my head that specific convention is
reserved for core/stdlib. Maybe I'm being overly paranoid about that.

Everyone is of course free to use their own. In any case, is it
frequent that we delegate to an object for which we don't know the
methods in advance?

Well, yes - part of the point of the VCR class I showed is that it's
library code that's usable pretty much anywhere. I use it (and things
like it) in testing a fair amount, where in general I want to be able to
take advantage of method_missing and not have to care about the
specifics of the delegate. Also, while I might know all the methods on
the delegate now, I don't have foreknowledge of all the methods which
will ever be implemented on it as the project develops.

There is precedent for what you're suggesting, in that Erlang does the
same, so it's not unreasonable. It's just close enough to home that it
makes me uncomfortable.

--
Alex

---------------------------------------- Feature #6693: Don't warn
for unused variables starting with _
https://bugs.ruby-lang.org/issues/6693#change-27894

Author: marcandre (Marc-Andre Lafortune) Status: Feedback Priority:
Low Assignee: Category: core Target version: 2.0.0

Currently, variables which are set but not used will generate a
warning (ruby -w), except if they have the special name "_".

So best practice is to use "_" for all unused variables. This does
not encourage readable code.

Currently must read: _, _, _, suffix = parse_name

could read: _first, _middle, _last, suffix = parse_name

We should not warn for unused variables starting with a "_".

This would create an option (but no obligation) to use more
descriptive names than "_" without generating warnings.

[alexeymuranov (Alexey Muranov)]

It this really necessary to treat the variable name _ exceptionally at the interpreter's level? Why not to use the same rule for all variables, and for unused variables introduce a special placeholder, for example - not followed by anything other than a delimiter (comma or bracket):

each_with_index { |-, value| puts value }

-, -, suffix = parse_name

The underscore is treated exceptionally because it can be repeated in block arguments:

{1=>2}.each_with_index { |,| puts "Oho!" } # Ok

{1=>2}.each_with_index { |x,x| puts "Oho!" } # SyntaxError: (eval):2: duplicated argument name

Is this exception really necessary? Maybe for repeated arguments, the variable (_ or x) can be made to hold, for example, the last non-nil argument of those assigned to it? Or, as it works currently for _, the value can be the array of all the arguments assigned to it, but this seems redundant: if one needs all the values, why not to use distinct variables?

[drbrain (Eric Hodel)]

Why should we break compatibility with ruby 1.9?

RDoc uses double assignment to avoid the unused variable warning for use by ERb via a binding. The syntax error would break RDoc, so I dislike it.

[alexeymuranov (Alexey Muranov)]

drbrain (Eric Hodel) wrote:

Why should we break compatibility with ruby 1.9?

RDoc uses double assignment to avoid the unused variable warning for use by ERb via a binding. The syntax error would break RDoc, so I dislike it.

I am sorry, was this about my proposal? I was not suggesting to raise an error, i just suggested that _ be treated as any other variable, for example by making it possible to repeat any variable, not just _. For completely unused variables, a special syntax can be used.

[drbrain (Eric Hodel)]

alexeymuranov (Alexey Muranov) wrote:

It this really necessary to treat the variable name _ exceptionally at the interpreter's level? Why not to use the same rule for all variables, and for unused variables introduce a special placeholder, for example - not followed by anything other than a delimiter (comma or bracket):

Removing warning suppression via "_" reintroduces warnings I've fixed and breaks backward compatibility.

{1=>2}.each_with_index { |x,x| puts "Oho!" } # SyntaxError: (eval):2: duplicated argument name

Adding this SyntaxError will break RDoc.

PS: you should open a separate feature request for these two changes. They are beyond of this change.

[alexeymuranov (Alexey Muranov)]

I am sorry, i think there is misunderstanding.

{1=>2}.each_with_index { |x,x| puts "Oho!" } # SyntaxError: (eval):2: duplicated argument name

is not a feature request, it is the current behavior. I proposed to remove this syntax error and to allow to repeat any variable, not only _.

Yes, i agree that this is a bit off topic, and maybe contradicts to this feature request.
I have created a new one: #6869

I even think that i am against this proposal, because it makes variables behave differently based on their names, and the proposed use case is to use unused variables as comments.

Update 2012-08-15: I would propose, instead of adding special rules for variables with special names, to come up with some "best practice" of annotating such situations with comments. For example:

_, _, _, suffix = parse_name # <= \first/, \middle/, \last/, suffix = ...

Think also about the possibility to use unicode characters in comment annotations!

Also, think about those who will read the code: they will never be sure if variables like _, _first, etc are just placeholders, or if the programmer actually used them somewhere later.

[ko1 (Koichi Sasada)]

• Assignee set to mame (Yusuke Endoh)

[mame (Yusuke Endoh)]

• Assignee changed from mame (Yusuke Endoh) to matz (Yukihiro Matsumoto)

[mame (Yusuke Endoh)]

• Target version changed from 2.0.0 to 2.6

[marcandre (Marc-Andre Lafortune)]

Since 2.0.0p0 has been released with this feature (which I still believe to be good), should we close this?

[marcandre (Marc-Andre Lafortune)]

• Status changed from Feedback to Closed