[Validator] fix lazy property usage. #36627
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This attempts to fix a large regression introduced in #36343, which broke recursing values returned from
getter
Constraints, because they are now wrapped in in aLazyProperty
. TheLazyProperty
needs to be evaluated because some checks are done on the type of$value
, i.eis_array
etc... invalidateGenericNode
.I'm concerned that the original PR didn't really add sufficient test coverage for the introduction of
LazyProperty
, and I'm not 100% sure that I've caught all the cases where theinstanceof
check are needed in this PR.For the tests, I added the
@dataProvider getConstraintMethods
to every test that hit the problem area of code.The only issue is that my fixed has broken the test introduced in #36343,testGroupedMethodConstraintValidateInSequence
.I think I need @HeahDude to help me work through this. Maybe there is a more simple solution, one that doesn't require doinginstanceof LazyPropery
checks in multiple places, because this feels very brittle.EDIT: fixed that test.