Phan comes with a few options, disabled by default, to make type analysis much stricter. From the manual:
- strict_method_checking
- strict_object_checking
- strict_param_checking
- strict_property_checking
- strict_return_checking
We could consider enabling this to make our code more robust and prevent errors such as those we're seeing with the PHP 8.1 migration (T379874). Unfortunately, MediaWiki code is considerably far from this level of strictness, and for each of the issue types above we have hundreds of issues in MW core alone. Some possible solutions and patterns that stand out:
- Documenting array shapes more thoroughly (phan can also enforce this with UnknownElementTypePlugin, but it is admittedly quite annoying for arrays of scalars).
- Being stricter in documentation. For example, query methods in IDatabase can return false if there's an error and the internal QUERY_SILENCE_ERRORS flag is set, but we typically ignore this fact in 99.9% of our code. This would not pass strict checks.
- A super common pattern: methods that return a nullable type (e.g., Title|null) where some callers use the return value without checking for null. If it can be null, it should be handled (potentially with an assertion). If it (generally) can't, either make the return value not nullable, or replace nullability with a checked exception (or a separate LBYL method + an unchecked exception).
- Class properties with nullable type where the property is not initialized in the constructor, but rather in a separate initialization method that needs to be called before using the property. I find these confusing regardless of tools. Could use a getter with an assertion.