Project

General

Profile

Actions

Bug #13872

closed

Duplicate assignment no longer silences "assigned but unused variable" warning

Added by segiddins (Samuel Giddins) about 7 years ago. Updated about 7 years ago.

Status:
Closed
Assignee:
-
Target version:
-
ruby -v:
ruby 2.5.0dev (2017-09-04 trunk 59742) [x86_64-darwin16]
[ruby-core:82656]

Description

On ruby 2.4.1:

ruby -W -e 'def a; var = var = "foo"; end'

prints no warning

On ruby 2.5.0dev (2017-09-04 trunk 59742) [x86_64-darwin16]

ruby -W -e 'def a; var = var = "foo"; end'

prints warning -e:1: warning: assigned but unused variable - var

This feature was useful when the local binding would be passed to another scope (like to ERB), and those variables were only accessed via the binding

Updated by ted (Ted Johansson) about 7 years ago

Previously, foo = foo = "bar" was used as a workaround. Now that the workaround is "fixed", I see incoming PRs changing the workaround to foo = "bar"; foo = foo.

It would be nice if we had a way to handle these false positives directly, without having to resort to tricks. :-)

segiddins (Samuel Giddins) wrote:

On ruby 2.4.1:

ruby -W -e 'def a; var = var = "foo"; end'

prints no warning

On ruby 2.5.0dev (2017-09-04 trunk 59742) [x86_64-darwin16]

ruby -W -e 'def a; var = var = "foo"; end'

prints warning -e:1: warning: assigned but unused variable - var

This feature was useful when the local binding would be passed to another scope (like to ERB), and those variables were only accessed via the binding

Updated by shevegen (Robert A. Heiler) about 7 years ago

Well, the first could be solved if there is some way to also propagate (local) variables towards ERB
or other bindings. Like to have different binding "levels" available. Copy environments and stuff. :D
(Actually ... one could probably store stuff in ENV and have them then propagated? But admittedly that
is also another work around towards anyone just wanting to use local variables instead).

But I think that, to the topic at hand, there may also be cases where people may not have wanted
to do such assignments; could happen due to some quick copy/paste or so. So for these cases, I
think that the 2.5.x behaviour is better, though it also does not seem to distinguish between
same_name_variable = same_name_variable = 'something else' - the "did you mean" gem could also
ask a question such as "did you really mean to give the variable the same name again"? :-)

The "workaround" was probably never intended to be used or be meaningful; at the least I can
not think that matz would think that the above was ... well, revealing the intent.
(self-assignment of variables? By the way, I did not know this workaround), but I agree that
some people may have liked that workaround. Perhaps ruby core may consider if you could add
some alternative proposal without the double assignment?

I mean, if the major use case is in regards to bindings and local variables then perhaps some
other way to use/propagate variables like that could convince matz / the core team?

Updated by matsuda (Akira Matsuda) about 7 years ago

This is not a bug. The warning you see is a result of an improvement of "unused variable" detection at r59585.
Now it simply checks whether each variable is actually "used" or not. Please also take a look at #13809 to see what was solved here.
var = var = "foo" warns now because var = var = "foo" doesn't really "use" the variable var.

Please update your code to actually "use" all the assigned local variables, in this case, to var = "foo"; var = var.

Updated by normalperson (Eric Wong) about 7 years ago

wrote:

Please upgrade your code to actually "use" all the assigned
local variables, in this case, to var = "foo"; var = var.

The problem is with binding:

require 'erb'
foo = :bar
puts ERB.new('<%= foo %>').result(binding)

The above uses foo, but binding isn't handled by the parser.
Maybe the parser should not warn if the word binding' appears in scope? And check for local variables named binding'...

But yes, this is annoying with ERB when I want my code
to be warning-clean.

Updated by matsuda (Akira Matsuda) about 7 years ago

Yes, in my understanding that is exactly the case we use this idiom for, usually when the contents of ERB is user-given and so it's uncertain if the local variable 'foo' will be used there or not.

 require 'erb'
-foo = :bar
+foo = :bar; foo = foo
 puts ERB.new('<%= foo %>').result(binding)

Updated by nobu (Nobuyoshi Nakada) about 7 years ago

There could be:

  1. add assignments in user code
  2. enable duplicate assignment as a special case
  3. add another way to suppress warnings, e.g. magic comment
  4. etc
Actions #7

Updated by nobu (Nobuyoshi Nakada) about 7 years ago

  • Status changed from Open to Closed

Applied in changeset trunk|r60339.


parse.y: workaround for warnings

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0