Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Narrow property types in validated objects #369

Open
ondrejmirtes opened this issue Oct 29, 2023 · 4 comments
Open

Narrow property types in validated objects #369

ondrejmirtes opened this issue Oct 29, 2023 · 4 comments

Comments

@ondrejmirtes
Copy link
Member

/cc @DaveLiddament

When an object is validated with Symfony Validator, we could narrow down its property types based on constraints.

My idea is:

  • Create class ValidatedObjectType extends ObjectType
  • This class could override ObjectType::getProperty(), read the validator attributes, and narrow the property type in reflection returned from parent. We'd probably create new ValidatedObjectPropertyReflection, inject the original object, and only override the type getters.
  • Also override what ValidatedObjectType::isSuperTypeOf and acceptsWithReason answer.
  • In PHPDocs we could typehint something like symfony-validated<UserDTO> and return ValidatedObjectType thanks to https://phpstan.org/developing-extensions/custom-phpdoc-types.

The only missing part is to narrow the object type to a validated one after calling the validator:

    $errors = $validator->validate($author);
    if (count($errors) === 0) { /* here we have a validated object */ }

This interface isn't really static analysis friendly. It'd be a job for a type-specifying extension to do that, but something like $validator->isValid($author) would be much easier to work with.

@RobertMe
Copy link
Contributor

I've been doing sone work on this and based on the ticket description was pretty easy to get something working in two hours 🎉

@ondrejmirtes could you elaborate on what you mean with this:

Also override what ValidatedObjectType::isSuperTypeOf and acceptsWithReason answer.

Furthermore I'm wondering what should happen when assigning properties on a validated object. I guess that using the same narrowed type for PropertyReflection::getWritableType() would disallow assigning any type which is actually assignable but might not pass validation? So IMO ideally assigning to any (validated) property should change the object type back (i.e. change the ValidatedObjectType into an ObjectType again).

Furthermore I have a technical question. Is there an easy method to parse the attributes arguments? For example NotBlank has an allowNull argument. So I need to parse the argument expressions on the attribute. Which results in some more complex code on it's own. And then, there are three methods to set this option, #[NotBlank(allowNull: true)], #[NotBlank(['allowNull' => true])] and #[NotBlank(options: ['allowNull' => true])], which makes the code even worse. So if there is some method which you can give the Expr of the argument which will resolve it into some PHP value already that would be nice. As now I have to resolve the ConstFetch for true/false, walk over the Array_->items, check every ArrayItem->key whether it's a String_, ....

@ondrejmirtes
Copy link
Member Author

I'd say that a validated object should stay validated, so it should only accept the narrowed type. I'd say it's a rare scenario anyway.

So if there is some method which you can give the Expr

When you ask for attributes with ClassReflection::getNativeReflection()->getProperty($name)->getAttributes(), you're gonna end up with this object: https://apiref.phpstan.org/1.11.x/PHPStan.BetterReflection.Reflection.Adapter.ReflectionAttribute.html

You can see it has getArgumentsExpressions() method.

You can use this class to turn expressions into Types: https://apiref.phpstan.org/1.11.x/PHPStan.Reflection.InitializerExprTypeResolver.html

There's a feature request for a nicer API: phpstan/phpstan#10443

@ruudk
Copy link
Contributor

ruudk commented Sep 18, 2024

I've been doing sone work on this and based on the ticket description was pretty easy to get something working in two hours 🎉

@RobertMe Will you continue with this or can you share what you have created in those 2 hours? That would allow others to continue with it 😃

@RobertMe
Copy link
Contributor

@ruudk sadly I didn't manage to really finish it. And in the end it ended up more like being a proof of concept, which wasn't really future proof. This as I just started parsing the attributes. But this has a lot of obvious limitations. For one it thus only supports the attributes, while Symfony also supports the constraints being defined in classic (PHPDoc) annotations, yaml / XML / ... config files, etc.. Furthermore I opted to not instantiate the attributes (to not depend on Symfony / the code) which is kinda bonkers to do on its own. As it requires a lot more of complex parsing (identifying options either by the named argument of the attribute or the position).
Furthermore there is also the "groups" option, i.e. "apply constraint only when validating group ..." which would make the code even harder.

At some later point I did find out that Symfony has some factory / registry / ... like class which can be used to get all the constraints for a class (/container?), which would be ideal to use for this as well. But it obviously would require some new "bootstrap" file (as is for example required for console application support, but also for phpstan-doctrine) to get the service from the container. But... the service is private (by default). But sadly I can't remember which service it was. And also didn't retry to build a solution based on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants