Fix `typecastValue` and `typecastAttributes` methods in `AttributeTypecastBehavior`
AttributeTypecastBehavior::typecastValue():$typecallback can be stringAttributeTypecastBehavior::typecastAttributes: typecast only active attributes
| Q | A |
|---|---|
| Is bugfix? | ✔️ |
| New feature? | ❌ |
| Breaks BC? | ✔️/❌ |
| Fixed issues |
@rob006 smiles not informative, can you explain your position?
Your PR description is laconic and does not explain what use case you're trying to fix. And I see big potential for BC breaks and footguns (is_callable() is just infinite source of problems, and it is also quite slow).
@rob006
Your PR description is laconic
it's because my English not so good :(
AttributeTypecastBehavior::typecastValue(): $type callback can be string
fixes error at using stringable callbacks
AttributeTypecastBehavior::typecastAttributes: typecast only active attributes
filters passed attributes to skip type casting unactive attributes
And I see big potential for BC breaks and footguns (is_callable() is just infinite source of problems, and it is also quite slow
is_callable() better than is_scalar(), because covers string callbacks
filters passed attributes to skip type casting unactive attributes
Which obviously breaks BC, since you even needed to adjust unit tests.
is_callable()better thanis_scalar(), because covers string callbacks
So if someone has integer() function it will be called instead of built-in typecast. If we want to support string callbacks, built-in typecasts should have precedence and call_user_func() should be used for everything else.
@rob006
Which obviously breaks BC
type casting unactive attributes is bug
since you even needed to adjust unit tests
I'm just fixed incomplete test model - active attribute must be rules()
If we want to support string callbacks, built-in typecasts should have precedence and call_user_func() should be used for everything else.
agree, fixed
type casting unactive attributes is bug
Why? This "bug" was covered by tests.
agree, fixed
There is still problematic is_callable().
@rob006
This "bug" was covered by tests.
as you can see, changes too covered, I'm just add missed rule
There is still problematic is_callable().
what's problem?
Closing since that's very likely result in a big BC break.