phpstan/phpstan-symfony

Type refinement for options in form types

stof opened this issue · 2 comments

stof commented

the $options argument of the various methods in FormTypeInterface and FormTypeExtensionInterface are typed as array<string, mixed> for now, because there is no other way to type them in phpdoc. However, the actual type is a configurable array shape, depending on the configuration of the OptionsResolver.
Having a more precise type would make it easier to write form types when using the level 9 of phpstan (where mixed requires explicit type checking to use them) and would allow more precise analysis at lower levels (as phpstan would report invalid usages of values taken from options instead of allowing anything because they are mixed)

Calls done on the OptionsResolver in the configureOptions method should be analyzed to build the array shape:

  • setDefined marks a key (or multiple ones) as known (but the key might be missing)
  • setRequired will mark a key (or multiple ones) as known and the key is guaranteed to be present
  • setDefault will mark the key as known and the key is guaranteed to be present
  • setDefaults is equivalent to calling setDefault for each key/value pair of the argument
  • setAllowedTypes defines the allowed types for that option. When multiple values are passed in the array, this allows a union of those types. Supported types are built-in type names, object types (class names or interface names) or generic array types (defines with a [] suffix after a supported type). Passing a string as second argument is equivalent to passing an array containing that string as single value.
  • addAllowedTypes allows to expand the list of allowed types by adding more types in the union (if no allowed types have been configured yet for that option, either through setAllowedTypes or addAllowedTypes, it is equivalent to setAllowedTypes)
  • setAllowedValues allows to define the list of allowed values for a property (passing anything else than an array as second argument is treated like passing a single-value array):
    • either a Closure (not other types of callables) taking the value as argument and returning a boolean to decide whether it is allowed (which would probably mean that phpstan has to skip the refinement based on allowed values)
    • or a value which will be compared with ===. This might lead to refining the option to a union of string literals for instance.
  • setDefault (and setDefaults) support defining a nested option by using a Closure with a first argument using Symfony\Component\OptionsResolver\OptionsResolver (exactly) as argument type. the type of the value will depend on the array shape for the OptionsResolver configured in that Closure:
    • if setPrototype(true) was called in the configuration, the nested option contains an array of values matching the array shape
    • otherwise, the nested option will match the array shape

For options that are defined without defining allowed types or allowed values (or defining callback-based validation of allowed values that cannot be analyzed), the array shape should used mixed as the type for them.

A basic version of the analysis could report only options defined by a form type for itself. However, this might return false positive errors for some valid code relying on parent options. A full analysis would have to consider the type hierarchy:

  • in a type extension, merge the array shape configuration with the shape of the extended types (based on class names returned in getExtendedTypes). If an extension extends multiple types, this would lead to a union type of a shape for each type
  • in a form type, merge the array shape configuration with the shape or the parent defined in getParent() (if not null)

Note: the array shape should be an unsealed one (if PHPStan ever implements the distinction between sealed and unsealed shapes that Psalm implements) because all form types of the type hierarchy receive the full array of options (so when calling methods on a parent type, it is likely to contain some extra options known only by the child type or a type extension).

As I am currently struggling with that exact topic, here is a short & simple reproducer:

final class SomeDateHandlerForm extends AbstractType {
  public function buildForm(FormBuilderInterface $builder, array $options): void {
        $date = $options['year'] . '-01-01';
        // ... 
    }

    public function configureOptions(OptionsResolver $resolver): void {
        $resolver->setDefaults(['year' => date('Y')]);
        $resolver->setAllowedTypes('year', ['string']);
    }
}

This will report Binary operation "." between mixed and '-01-01' results in an error. even though the OptionsResolver makes sure that only a string is given in $options['year']. Currently I have to either silence the line or write type checking logic, which is not necessary due to the validation inside OptionsResolver.