codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.31k stars 1.89k forks source link

Dev: replace type `mixed` with more specific types #6310

Open kenjis opened 2 years ago

kenjis commented 2 years ago

Type mixed means object|resource|array|string|int|float|bool|null. https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.mixed

The current code base has a lot of type mixed in PHPDocs, But I think all of them are not correct.

And mixed prevents PHPStan level from increasing.

PHPStan’s rule level 6 isn’t satisfied with implicit mixed https://phpstan.org/writing-php-code/phpdoc-types#mixed

We need to replace type mixed with more specific types.

paulbalandan commented 2 years ago

I think we should, in part, allow explicit mixed types if the parameter/return can be of any type and we have no way of telling what can be the possible types at runtime.

However, for those annotated as mixed but the actual types can be deduced to be a subset only of object|resource|array|string|int|float|bool|null then that is what we should flag as erroneous. The annotation should indicate specifically the actual types, not mixed.

kenjis commented 2 years ago

@paulbalandan What's you say is correct, but I think it is difficult to do.

Currently, there are too many mixed, so I think it would be easier to understand the approach of banning and eliminating mixed, and then changing to mixed if mixed is appropriate.

paulbalandan commented 2 years ago

You're right. It will be a pain to track all existing uses of mixed in the framework. So it'll be easier to prevent additional uses of mixed moving forward.

kenjis commented 1 year ago

We still have a lot of @return mixed and @param mixed. Contributions are welcome!

ping-yee commented 1 year ago

I already sent few PR, review please.

neznaika0 commented 1 year ago
    /**
     * Outputs a string to the cli on it's own line.
     *
     * @return void
     */
    public static function write(string $text = '', ?string $foreground = null, ?string $background = null)

Is it really important to use Docbook or do I need to add a clarification : void ? Because now the methods have the only meaning : void looks right. But you accept comments where DocBlock

kenjis commented 1 year ago

@neznaika0 This issue is about @mixed doc comment.

Basically, we cannot change the method signature if the method can be overridden (by a user). Because it is a breaking change, and we permit breaking changes only in major version up.

kenjis commented 1 year ago
AbstractLogger.php
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
BaseBuilder.php
     * @param mixed               $value
     * @param mixed               $value
     * @param mixed               $value
     * @param mixed               $value
     * @param mixed               $value
     * @param mixed               $value  Field value, if $key is a single field
     * @param mixed $key
     * @param mixed $where
     * @param mixed $selectOverride
     * @param mixed $value
     * @param mixed $value
BaseConnection.php
     * @return mixed
BaseHandler.php
     * @phpstan-param Closure(): mixed $callback
BaseHandler.php
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
BaseUtils.php
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
BlobValue.php
     * @param mixed $encoding
     * @param mixed $encoding
Builder.php
     * @param mixed $where
     * @return mixed
Builder.php
     * @return mixed
     * @return mixed
     * @return mixed
     * @param mixed $where
     * @return mixed
Builder.php
     * @return mixed
     * @param mixed $where
     * @return mixed
CacheInterface.php
     *                          Returns null if the item does not exist, otherwise array<string, mixed>
CallFinder.php
     * @param mixed $function
     * @param mixed $token
CIUnitTestCase.php
     * @var mixed
     * @param mixed $actual
     * @param mixed $expected
     * @param mixed $actual
CLIRequest.php
     * @param mixed             $flags
ConditionalTrait.php
     * @template TWhen of mixed
     * @phpstan-param callable(self, TWhen): mixed  $callback
     * @phpstan-param (callable(self): mixed)|null  $defaultCallback
     * @template TWhenNot of mixed
     * @phpstan-param callable(self, TWhenNot): mixed $callback
     * @phpstan-param (callable(self): mixed)|null    $defaultCallback
ConfigFromArrayTrait.php
     * @param array<string, mixed> $config
controller.tpl.php
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
     * @param mixed $id
     * @return mixed
     * @return mixed
     * @return mixed
     * @param mixed $id
     * @return mixed
     * @param mixed $id
     * @return mixed
     * @param mixed $id
     * @return mixed
     * @param mixed $id
     * @return mixed
ControllerTester.php
     * @return mixed
     * @param mixed $appConfig
     * @return mixed
     * @param mixed $request
     * @return mixed
     * @param mixed $response
     * @return mixed
     * @param mixed $logger
     * @return mixed
     * @return mixed
     * @return mixed
ControllerTestTrait.php
     * @param mixed $appConfig
     * @param mixed $request
     * @param mixed $logger
Cookie.php
     * @return array<string, mixed> The old defaults array. Useful for resetting.
DOMDocumentPlugin.php
 * way to see inside the DOMNode without print_r, and the only way to see mixed
Email.php
     * @param mixed $type
ErrorlogHandler.php
     * @param mixed[] $config
Events.php
     * @param mixed  $arguments
FeatureTestCase.php
     * @param mixed $body
FileHandler.php
     * @return array<string, mixed>|false
     * @phpstan-return array{data: mixed, ttl: int, time: int}|false
filter.tpl.php
     * @return mixed
     * @return mixed
Forge.php
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
Forge.php
     * @return mixed
GDHandler.php
     * @return mixed
GeneratorTrait.php
     * @return mixed
ImageHandlerInterface.php
     * @return mixed
init_helpers.php
     * @param mixed ...$args
     * @param mixed ...$args
Iterator.php
     * @phpstan-param Closure(): mixed $closure
Kint.php
     * @var mixed Kint mode
     * @param mixed &$var Data to dump
     * @param mixed ...$args
LoggerInterface.php
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed[] $context
     * @param mixed   $level
     * @param mixed[] $context
LoggerTrait.php
     * @param mixed  $level
MigrateStatus.php
     * @param array<string, mixed> $params
MigrationRunner.php
     * @return mixed Current batch number on success, FALSE on failure or no migrations are found
MockCache.php
     * @return mixed
     * @return mixed
     * @param mixed  $value the data to save
     * @return array|null Returns null if the item does not exist, otherwise array<string, mixed>
     * @param mixed $value
MockConnection.php
     * @param mixed ...$binds
     * @return mixed
     * @return mixed
MockLanguage.php
     * @var mixed
MockResult.php
     * @return mixed
ModelFactory.php
     * @return mixed
NullLogger.php
     * @param mixed  $level
Parser.php
     * @param mixed &$var The input variable
     * @param mixed &$var The input variable
     * @param mixed &$var    variable
PluginInterface.php
     * @param mixed &$var
Plugins.php
     * @return mixed|string|URI
Query.php
     * @param mixed $binds
QueryInterface.php
     * @param mixed $binds
     * @return mixed
     * @return mixed
     * @return mixed
     * @return mixed
ReflectionHelper.php
     * @param mixed         $value    value
     * @return mixed value
RendererInterface.php
     * @param mixed  $value
RequestInterface.php
     * @return mixed
RequestTrait.php
     * @return mixed
     * @return mixed
     * @param mixed $value
RichRenderer.php
     * @param mixed $encoding
Seeder.php
     * @return mixed
SeeInDatabase.php
     * @param mixed $table
     * @param mixed $table
Table.php
     * @return mixed
     * @return mixed
     * @phpstan-return ($fields is array ? array : mixed)
     * @param mixed $keys
     * @return mixed
TestResponse.php
     * @param mixed $value
     * @return mixed|string
     * @param mixed  $params   Any method parameters
     * @return mixed
TextRenderer.php
     * @param mixed $encoding
Timer.php
     * @phpstan-param callable(): mixed $callable callable to be executed
Utils.php
     * @return mixed
Utils.php
     * @return mixed
Utils.php
     * @return mixed
Utils.php
     * @return mixed
Utils.php
     * @return mixed
Utils.php
     * @param mixed $encoding
View.php
     * @param mixed       $value
paolooo commented 5 months ago

Hello, I'm curious about how to display PHPStan errors for mixed types. I've attempted to run the following but there's no error.

$ composer sa
> Composer\Config::disableProcessTimeout
> bash -c "XDEBUG_MODE=off phpstan analyse"
Note: Using configuration file /Users/github/CodeIgniter4/phpstan.neon.dist.
 917/917 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%
 [OK] No errors
> rector process --dry-run
 [OK] Rector is done!

However, I can find several @return mixed types.

system/Database/Postgre/Builder.php:     * @return mixed
system/Database/Postgre/Builder.php:     * @return mixed
system/Database/Postgre/Builder.php:     * @return mixed
system/Database/Postgre/Builder.php:     * @return mixed
system/Database/BaseUtils.php:     * @return mixed
system/Database/BaseUtils.php:     * @return mixed
system/Database/SQLSRV/Builder.php:     * @return mixed
system/Database/SQLSRV/Builder.php:     * @return mixed
system/Database/SQLSRV/Forge.php:     * @return mixed
system/Database/ModelFactory.php:     * @return mixed
system/Database/OCI8/Builder.php:     * @return mixed
system/Database/BaseConnection.php:     * @return mixed
system/Database/SQLite3/Table.php:     * @return mixed
system/Database/SQLite3/Table.php:     * @return mixed
system/Database/QueryInterface.php:     * @return mixed
system/Database/QueryInterface.php:     * @return mixed
system/Database/QueryInterface.php:     * @return mixed
system/Database/QueryInterface.php:     * @return mixed
system/Test/TestResponse.php:     * @return mixed|string
system/Test/TestResponse.php:     * @return mixed
system/Test/ReflectionHelper.php:     * @return mixed value
system/Test/Mock/MockConnection.php:     * @return mixed
system/Test/Mock/MockConnection.php:     * @return mixed
system/Test/Mock/MockResult.php:     * @return mixed
system/Test/ControllerTester.php:     * @return mixed
system/Test/ControllerTester.php:     * @return mixed
system/Test/ControllerTester.php:     * @return mixed
system/Test/ControllerTester.php:     * @return mixed
system/Test/ControllerTester.php:     * @return mixed
system/Test/ControllerTester.php:     * @return mixed
system/Test/ControllerTester.php:     * @return mixed
system/Images/ImageHandlerInterface.php:     * @return mixed
system/Images/Handlers/GDHandler.php:     * @return mixed
system/Images/Handlers/BaseHandler.php:     * @return mixed
system/Images/Handlers/BaseHandler.php:     * @return mixed
system/Images/Handlers/BaseHandler.php:     * @return mixed
system/Images/Handlers/BaseHandler.php:     * @return mixed
system/CLI/GeneratorTrait.php:     * @return mixed
system/HTTP/RequestTrait.php:     * @return mixed
system/HTTP/RequestTrait.php:     * @return mixed
system/HTTP/RequestInterface.php:     * @return mixed
system/Debug/Timer.php:     * @return mixed
kenjis commented 5 months ago

@paolooo Explicit mixed is allowed in level 6 on PHPStan. So PHPStan does not report any error.

PHPStan has a concept of implicit and explicit mixed. Missing typehint is implicit mixed - no type was specified as a parameter type or a return type. Explicit mixed is written in the PHPDoc. PHPStan’s rule level 6 isn’t satisfied with implicit mixed, but an explicit one is sufficient. https://phpstan.org/writing-php-code/phpdoc-types#mixed

But

Rule level 9 is stricter about the mixed type. The only allowed operation you can do with it is to pass it to another mixed.

paolooo commented 5 months ago

Thank you, @kenjis, for providing clarification.