Code analysis for Laravel 9 branch

[bennothommo]

Thought I would try installing Larastan/PHPStan and run some code analysis to look for some invalid PHP docblocks and dead code and the like. So far, it's picked up about 1100 errors - not sure if it's a bridge too far to cross, but interesting nonetheless.

We're testing against Level 5 so far, which seems to be a good compromise between ensuring all docblocks and signatures are correct whilst not having PHPStan being pedantic about everything.

Modules

• Argon
• Auth
• Config
• Cookie
• Database
• Events
• Exception
• Extension
• Filesystem
• Flash
• Foundation
• Halcyon
• Html
• Http
• Mail
• Network
• Parse
• Redis
• Router
• Scaffold
• Support
• Translation
• Validation

Breaking changes introduced

We will need to discuss these before merging.

Database

• To prevent unsafe model instantiating, the model constructors are now forced to only allow a single $attributes parameter via an interface (\Winter\Storm\Database\ModelInterface and \Winter\Storm\Halcyon\ModelInterface). This will ensure that calls like Model::make() or Model::create() will execute correctly. It is possible that some people might have used additional parameters for their model constructors - these will no longer work and must be moved to another method.
• Due to the above, Pivot model construction has been rewritten. The constructor used to allow 4 parameters but now only allows the one $attributes parameter as per the Model class. Construction now happens more closely to Laravel's format of calling a Pivot::fromAttributes() static method. If you previously used new Pivot() to create a pivot model, switch to using Pivot::fromAttributes() instead.
• The MorphToMany class now extends the MorphToMany class from Laravel, as opposed to the BelongsToMany class in Winter. This prevents repeated code in the Winter MorphToMany class and maintains covariance with Laravel. This will mean that it will no longer inherit from the Winter BelongsToMany class. To allow for this, we have converted most of the overridden BelongsToMany functionality in Winter into a trait (Concerns\BelongsOrMorphsToMany). The BelongsToMany class now also uses this trait.
• The relation traits found in src/Database/Relations have been moved to src/Database/Relations/Concerns, in order to keep just the actual relation classes within this directory. In the unlikely event that you are using a relation trait directly, please rename the trait classes to the following:
  • Winter\Storm\Database\Relations\AttachOneOrMany to Winter\Storm\Database\Relations\Concerns\AttachOneOrMany
  • Winter\Storm\Database\Relations\DeferOneOrMany to Winter\Storm\Database\Relations\Concerns\DeferOneOrMany
  • Winter\Storm\Database\Relations\DefinedConstraints to Winter\Storm\Database\Relations\Concerns\DefinedConstraints
  • Winter\Storm\Database\Relations\HasOneOrMany to Winter\Storm\Database\Relations\Concerns\HasOneOrMany
  • Winter\Storm\Database\Relations\MorphOneOrMany to Winter\Storm\Database\Relations\Concerns\MorphOneOrMany

Extension

• Previously, the Winter\Storm\Extension\Extendable::extendClassWith() method returned the current class if the extension name provided was an empty string. This appears to be a code smell, so this has been changed to throw an Exception.

Filesystem

• The Filesystem::symbolizePath method's $default parameter now accepts a string, bool or null. Only in the case of null will the method return the original path - the given $default will be used in all other cases. This is the same as the original functionality, but we're documenting it in case you have customised this method in some fashion.
• The Filesystem::isPathSymbol method previously returned the path symbol used, as a string, if one was found and false if not found. However, the docblock stipulated, as well as the method name itself implied, that this is a boolean check function, so we have converted this to a straight boolean response - true if a path symbol is used, otherwise false.
• Many classes within the Filesystem namespace have had type hinting and return types added to enforce functionality and clarify documentation. If you extend any of these classes in your plugins, you may need to update your method signatures.

Foundation

• The Application class callback methods before and after were documented as void methods, but returned a value. These no longer return a value.

Halcyon

• To prevent unsafe model instantiating, the model constructors are now forced to only allow a single $attributes parameter via an interface (\Winter\Storm\Database\ModelInterface and \Winter\Storm\Halcyon\ModelInterface). This will ensure that calls like Model::make() or Model::create() will execute correctly. It is possible that some people might have used additional parameters for their model constructors - these will no longer work and must be moved to another method.
• The Halcyon Builder insert method now returns an int representing the created model's filesize, not a bool.
• The Halcyon Builder insert method requires the $values parameter to actually contain variables and will throw an Exception if an empty array is provided. Previously, this was silently discarded and returned true (although this would not actually save the file).

HTML

• The Winter\Storm\Html\FormBuilder class has had type hinting and return types added to enforce functionality and clarify documentation. If you extend this class in your plugin, you may need to make changes to your method signatures if you overwrite the base functionality.

Parse

• The Bracket class constructor is now final to prevent unsafe static calls to the parse static method.
• The Bracket::parseString() method previously returned false if a string was not provided for parsing, or the string was empty after trimming. Since the signature calls for a string, we won't check for this anymore. If the string is empty, an empty string will be returned.
• The markdown.beforeParse event in the Markdown class sent a single MarkdownData instance as a parameter to the listeners. It now sends an array with the MarkdownData instance as the only value, to meet the signature requirements for events.
• The Syntax\FieldParser class constructor has been made final to prevent unsafe static calls to the parse statuc method.
• The $template parameter for the Syntax\FieldParser class constructor was originally optional. Since there is no purpose for this, and no way to populate the template after the fact, this has been made required.
• The Syntax\Parser class constructor has been made final to prevent unsafe static calss to the parse static method.
• The $template parameter for the Syntax\Parser class constructor was originally optional. Since there is no purpose for this, and no way to populate the template after the fact, this has been made required.

Testing

• The Winter\Storm\Support\Testing\Fakes\MailFake::queue() method has had its signature re-arranged to remain compatible with Laravel's MailFake class, with the $queue parameter being moved to the second parameter. The new signature is ($view, $queue, $data, $callback).