This project is archived and is in readonly mode.
Zero-arity conditional validation procs broken by AS::NewCallbacks
Reported by Sam Pohlenz | September 18th, 2009 @ 05:59 AM
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.
Comments and changes to this ticket
-
Sam Pohlenz September 21st, 2009 @ 02:01 AM
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 == 0with
return method_name if filter.arity <= 0however 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.
-
CancelProfileIsBroken September 25th, 2009 @ 12:00 PM
- Tag changed from “activemodel, activerecord, activesupport, patch” to “activemodel, activerecord, activesupport, bugmash, patch”
-
Rizwan Reza January 21st, 2010 @ 06:56 AM
- 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?
-
-
Rizwan Reza January 21st, 2010 @ 09:30 AM
- State changed from “open” to “resolved”
-
Rizwan Reza January 21st, 2010 @ 09:30 AM
- no changes were found...
Create your profile
Help contribute to this project by taking a few moments to create your personal profile. Create your profile »
<h2 style="font-size: 14px">Tickets have moved to Github</h2>
The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>
Sam Pohlenz