Bug #9593

Keyword arguments default argument assignment behaviour not consistent with optional argument

Added by Jack Chen over 1 year ago. Updated about 1 year ago.

[ruby-core:61299]
Status:Closed
Priority:Normal
Assignee:Yukihiro Matsumoto
ruby -v:2.1.1 Backport:1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: DONTNEED

Description

Given the following code:

def var
  100
end

def foo(var: var + 1)
  puts "var: #{var.inspect}"
end

def bar(var = var + 1)
  puts "var: #{var.inspect}"
end

foo(var: 1)
foo rescue p $!

bar(1)
bar

Ruby 2.0.0:

var: 1
var: 101
var: 1
var: 101

Ruby 2.1.1:

var: 1
#<NoMethodError: undefined method `+' for nil:NilClass>
var: 1
var: 101

What appears to be happening is that since 2.1.1, the keyword argument defines var as a variable before evaluating the default argument. Personally, I prefer 2.0.0 behaviour, but the way 2.1.1 handles default arguments in non keyword arguments is inconsistent.


Related issues

Related to Ruby trunk - Bug #10314: Default argument lookup fails in Ruby 2.2 for circular shadowed variable names Open 10/02/2014
Related to Ruby trunk - Bug #11074: Block with optional parameter of same closure variable name Rejected 04/18/2015
Duplicated by Ruby trunk - Bug #10280: Regression while evaluating default argments of a method Rejected 09/22/2014

Associated revisions

Revision 45272
Added by Nobuyoshi Nakada over 1 year ago

parse.y: optional arguments in rhs

  • parse.y (f_arg_asgn): define optional arguments as argument variables in the rhs default expressions. [Bug #9593]

Revision 45272
Added by Nobuyoshi Nakada over 1 year ago

parse.y: optional arguments in rhs

  • parse.y (f_arg_asgn): define optional arguments as argument variables in the rhs default expressions. [Bug #9593]

Revision 47776
Added by Nobuyoshi Nakada 11 months ago

NEWS: mentioned about [Bug #9593]

Revision 47776
Added by Nobuyoshi Nakada 11 months ago

NEWS: mentioned about [Bug #9593]

Revision 48835
Added by Nobuyoshi Nakada 9 months ago

parse.y: warn reference after method definition

  • parse.y (primary): restore current_arg so that circular reference after a method definition is also warned. [Bug #9593]

Revision 48835
Added by Nobuyoshi Nakada 9 months ago

parse.y: warn reference after method definition

  • parse.y (primary): restore current_arg so that circular reference after a method definition is also warned. [Bug #9593]

History

#1 Updated by Nobuyoshi Nakada over 1 year ago

The behaviors in 2.0 are incorrect both.

#2 Updated by Nobuyoshi Nakada over 1 year ago

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

Applied in changeset r45272.


parse.y: optional arguments in rhs

  • parse.y (f_arg_asgn): define optional arguments as argument variables in the rhs default expressions. [Bug #9593]

#3 Updated by Nobuyoshi Nakada over 1 year ago

  • Backport changed from 1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN to 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED

#4 Updated by Aaron Patterson over 1 year ago

Hi,

Before r45272 this code worked:

require 'minitest/autorun'

class DefaultArg < MiniTest::Unit::TestCase
  class Default
    def foo; 'foo'; end

    def bar foo = foo
      foo
    end
  end

  def test_default
    assert_equal 'foo', Default.new.bar
  end
end

Was this expected to break? Unfortunately we have real code that depends on the behavior:

https://github.com/rails/rails/blob/81d08abcccf2ef1b0ea6e98daf00f6ca375f3d8a/activerecord/lib/active_record/associations/has_many_association.rb#L74

but we can change it

#5 Updated by Nobuyoshi Nakada over 1 year ago

Aaron Patterson wrote:

Was this expected to break?

Yes. It was a bug.
An assignment creates a variable and it hides same name method in its RHS, as you can't call foo method inside bar method:

  def foo; 'foo'; end

  def bar
    foo = foo # nil
  end

Unfortunately we have real code that depends on the behavior:

https://github.com/rails/rails/blob/81d08abcccf2ef1b0ea6e98daf00f6ca375f3d8a/activerecord/lib/active_record/associations/has_many_association.rb#L74

Sorry for the bug.

but we can change it

Always you can call the method explicitly with ().

#6 Updated by Aaron Patterson over 1 year ago

Ok! I will fix the code in Rails. Thank you!

#7 Updated by Aaron Patterson over 1 year ago

Nobu, are you sure this is a bug? This test has worked since 1.9.3, and this code was committed in 2008:

https://github.com/rails/rails/blob/master/activesupport/lib/active_support/values/time_zone.rb#L285

It's been the behavior for at least 6 years. I don't mind changing Rails to work with this, but people who don't test with trunk may get a nasty surprise when Ruby 2.2 is released.

Edit: here's a better link: https://github.com/rails/rails/blob/15720df1808b249bae91e5600d6f0676990a7de0/activesupport/lib/active_support/values/time_zone.rb#L285

#8 Updated by Nobuyoshi Nakada over 1 year ago

Yes, it's a very old bug.
Optional arguments have same semantics as assignments.

#9 Updated by Tomoyuki Chikanaga about 1 year ago

Hi,

I think this change should not be backported to stable branch.
I agree about that it is a long standing bug. But it lives too long to change a behavior during stable releases.
I'm going to fill 'DONTNEED' in 'Backport' field if there's no objection.

#10 Updated by Tomoyuki Chikanaga about 1 year ago

  • Backport changed from 1.9.3: REQUIRED, 2.0.0: REQUIRED, 2.1: REQUIRED to 1.9.3: DONTNEED, 2.0.0: DONTNEED, 2.1: DONTNEED

No objection. I changed Backport field to DONTNEED.

#11 Updated by Nobuyoshi Nakada 12 months ago

  • Duplicated by Bug #10280: Regression while evaluating default argments of a method added

#12 Updated by Hiroshi SHIBATA 11 months ago

  • Related to Bug #10314: Default argument lookup fails in Ruby 2.2 for circular shadowed variable names added

#13 Updated by Nobuyoshi Nakada 5 months ago

  • Related to Bug #11074: Block with optional parameter of same closure variable name added

Also available in: Atom PDF