Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

Zero-arity conditional validation procs broken by AS::NewCallbacks

#3230

Using a zero-arity proc/lambda as an :if or :unless option for validations, breaks with the new callback code. This potentially affects a lot of legacy code.

Consider the following code:

class User
  include ActiveModel::Validations
  attr_accessor :name
  validates_presence_of :name, :if => Proc.new { true }
end

>> User.new.valid?
ArgumentError: tried to create Proc object without a block
    from /Users/sam/new-rails-2/vendor/rails/activeresource/lib/../../activesupport/lib/active_support/new_callbacks.rb:396:in `new'
    from /Users/sam/new-rails-2/vendor/rails/activeresource/lib/../../activesupport/lib/active_support/new_callbacks.rb:396:in `_run_validate_callbacks'
    from /Users/sam/new-rails-2/vendor/rails/activeresource/lib/../../activemodel/lib/active_model/validations.rb:94:in `valid?'
    from (irb):2

In AS::NewCallbacks::_compile_filter, replacing

method_name << (filter.arity == 1 ? "(self)" : " self, Proc.new ")

with:

method_name << (filter.arity == 1 ? "(self)" : " self, Proc.new {} ")

appears to fix the problem and passes all tests, but I'm not sure what exactly the second proc argument does there.

The attached patch includes failing tests for ActiveModel.

Reported by Sam Pohlenz · September 18th, 2009 @ 05:59 AM

State: resolved
Milestone: none
Assigned to: nobody
Importance: none

Activity

  1. Sam Pohlenz
    Sam Pohlenz

    After some more digging, it seems the problem is due to a variation between Ruby 1.8 and 1.9.

    In 1.8.6 and 1.8.7, Proc.new {}.arity == -1, whereas in 1.9, Proc.new {}.arity == 0.

    A possible fix for this then, is to replace in ActiveSupport::NewCallbacks::_compile_filter:

              return method_name if filter.arity == 0
    

    with

              return method_name if filter.arity <= 0
    

    however this will break support for splatted proc arguments.

    Whilst the problem lies in ActiveSupport::NewCallbacks, I have not yet been able to create a test case that isolates the problem.

    September 21st, 2009 @ 02:01 AM

  2. CancelProfileIsBroken
    CancelProfileIsBroken
    • Tag changed from activemodel, activerecord, activesupport, patch to activemodel, activerecord, activesupport, bugmash, patch

    September 25th, 2009 @ 12:00 PM

  3. Rizwan Reza
    Rizwan Reza
    • Tag changed from activemodel, activerecord, activesupport, bugmash, patch to activemodel, activerecord, activesupport
    • State changed from new to open

    Were you able to get to the problem Sam? Is this still applicable?

    January 21st, 2010 @ 06:56 AM

  4. Sam Pohlenz
    Sam Pohlenz

    This appears to now be fixed. Thanks.

    January 21st, 2010 @ 09:28 AM

  5. Rizwan Reza
    Rizwan Reza
    • State changed from open to resolved

    January 21st, 2010 @ 09:30 AM

  6. Rizwan Reza
    Rizwan Reza
    • No changes were found…

    January 21st, 2010 @ 09:30 AM