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

Array cast should emit an error #190

Open
mvorisek opened this issue Sep 7, 2022 · 4 comments
Open

Array cast should emit an error #190

mvorisek opened this issue Sep 7, 2022 · 4 comments

Comments

@mvorisek
Copy link

mvorisek commented Sep 7, 2022

Feature Request

Array cast is redundant on array type. (redundant cast emit a phpstan error already)

For scalar type values it wraps the value into an array - inconsistent with casting an array, array is not wrapped again.

For null it produces empty array - can be a mistake in code, scalar type value is wrapped in an array and never empty.

For objects - get_object_vars should be used.

Given these reasons, I would expect phpstan to emit an error if an (array) cast is found.

demo: https://3v4l.org/2INk8

playground: https://phpstan.org/r/e03daaef-49f6-4c30-b029-24bed8f50048 (line 10 should emit an error)

@VincentLanglet
Copy link
Contributor

For scalar type values it wraps the value into an array - inconsistent with casting an array, array is not wrapped again.

I often see the following method

/**
 * @param string|string[] $messages
 */
public function doThings($messages)
{
    $messages = (array) $messages;
    foreach ($messages as $message) {
        // ...
    }
}

Writing

$messages = (array) $messages;

is shorter than

if (!is_array($messages) {
     $messages = [$messages];
}

and casting to an array is exactly what we want to do, since we don't want to wrap again an existing array.

So I do not consider an (array) cast as an error, even in strict rules.

@mvorisek
Copy link
Author

that is true, but if an object is passed, the array cast is almost always wrong, so the variant with if should be preffered

also null is typically not wanted to be casted, again, the variant with if should be preffered

@VincentLanglet
Copy link
Contributor

The phpdoc is asking for string or string[]. If you are passing null or an object you already have an error from phpstan-core. You also can use string|array as a native param typehint if you use php 8.

@mvorisek
Copy link
Author

as long as the input is restricted by hard type and not-null/non-array-accepting phpdoc type

https://phpstan.org/r/d9d9c0ce-e0b6-4e4c-9ebd-92636312f21c

in both cases, I would expect a phpstan error with this feature request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants