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

Dev: replace type mixed with more specific types #6310

Open
kenjis opened this issue Jul 29, 2022 · 11 comments
Open

Dev: replace type mixed with more specific types #6310

kenjis opened this issue Jul 29, 2022 · 11 comments
Labels
dev good first issue Good for newcomers help wanted More help is needed for the proper resolution of an issue or pull request

Comments

@kenjis
Copy link
Member

kenjis commented Jul 29, 2022

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.

@kenjis kenjis added the dev label Jul 29, 2022
@kenjis kenjis changed the title Dev: replace type mixed with more specific types Dev: replace type mixed with more specific types Jul 29, 2022
@paulbalandan
Copy link
Member

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
Copy link
Member Author

kenjis commented Aug 2, 2022

@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
Copy link
Member

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 kenjis added the help wanted More help is needed for the proper resolution of an issue or pull request label Sep 12, 2022
@kenjis kenjis mentioned this issue Dec 8, 2022
3 tasks
@ping-yee
Copy link
Contributor

I already sent few PR, review please.

@neznaika0
Copy link
Contributor

    /**
     * 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
Copy link
Member Author

kenjis commented Aug 2, 2023

@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
Copy link
Member Author

kenjis commented Aug 27, 2023

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
Copy link

paolooo commented Mar 31, 2024

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
Copy link
Member Author

kenjis commented Mar 31, 2024

@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
Copy link

paolooo commented Mar 31, 2024

Thank you, @kenjis, for providing clarification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dev good first issue Good for newcomers help wanted More help is needed for the proper resolution of an issue or pull request
Projects
None yet
Development

No branches or pull requests

5 participants