Fix type signature for ActiveRecord::Base.validate (to better support official Ruby documentation)
#859
Conversation
…mbining-validation-conditions
In the official documentation, you can have `if: [Proc.new { |c| c.market.retail? }, :desktop?],` so, RBS signatures should reflect that
|
@yuzisee Thanks for your contribution! Please follow the instructions below for each change. Available commandsYou can use the following commands by commenting on this PR.
|
…ntax in the official Rails documentation
yuzisee
changed the title
ActiveRecord::Base.validate (to match official Ruby documentation)ActiveRecord::Base.validate (to better support official Ruby documentation)
…thod
Error: activerecord.rb:1:6: [error] Cannot find implementation of method `::TestCallbackObject#local_condition2`
│ Diagnostic ID: Ruby::MethodDefinitionMissing
│
└ class TestCallbackObject < ActiveRecord::Base
~~~~~~~~~~~~~~~~~~
Error: activerecord.rb:1:6: [error] Cannot find implementation of method `::TestCallbackObject#local_condition3`
│ Diagnostic ID: Ruby::MethodDefinitionMissing
│
└ class TestCallbackObject < ActiveRecord::Base
~~~~~~~~~~~~~~~~~~
Detected 3 problems from 1 file
…an `^(T) [self: T] -> boolish` Is this a **steep 1.10** vs. **steep 1.5** thing?
| [true, false].sample | ||
| end | ||
|
|
||
| validate :custom_validation, on: :update, unless: [:local_condition1, proc { |this_rec| this_rec.local_condition2 }, ->(my_rec) { my_rec.local_condition3 }] |
There was a problem hiding this comment.
Re: #859 (review)
LGTM. Could you check the errors in CI? I'll merge this after CI passed.
Thanks! The steep check is succeeding, but steep stats is complaining:
⋮
No type error detected. 🫖
2 method calls with untyped receiver detected from activerecord.rb. Please assign the expected type to the receivers
⋮
But…
-
if
->(my_rec) { my_rec.local_condition3 }is matching^(T) [self: T] -> boolish- ...then my_rec shouldn't be considered TypeInference::MethodCall::Untyped right?
- (since
[self: T]is a https://github.com/ruby/rbs/blob/master/docs/syntax.md#self-type-binding it should know the receiver type)
-
if
proc { |this_rec| this_rec.local_condition2 }matchesProc- then there's no way to set a type for this_rec right?
- (for example, we can do 312d68d but it defeats the purpose of the test)
There was a problem hiding this comment.
@tk0miya do you think I should try to use https://github.com/soutaro/steep/blob/master/manual/annotations.md here? I'm not sure how else to set the type of the receiver inside the unless: block while still keeping the syntax from the original Rails documentation.
According to https://guides.rubyonrails.org/v6.0/active_record_validations.html#combining-validation-conditions
the official documentation says you can have
if: [Proc.new { |c| c.market.retail? }, :desktop?],so, RBS signatures should reflect that.