Repository navigation
Ability to Specify Variable as Array/String via @var to Help Static Analysis (Level 7) #1345
Description
Appears that one can't use @var docblock to specify native array or string for L7. It appears the docblock doesn't work when the value is for string|null or array|null. It doesn't parse docblock override and/also doesn't walk up into upper scope to check precondition.
Code snippet
https://phpstan.org/r/36394a0d564b152157b9e1567654e865
<?php declare(strict_types = 1);
class HelloWorld
{
/**
* To array function.
*
* @param array|null $columns For compatibility with PHP5 overloaded function
* this variable can't use the [ ?array $columns = null ] syntax; however,
* that doesn't matter because PHPStan properly picks up docblock of array|null
*/
public function filter($columns = null): array
{
$set = ['abc' => 123, 'def' => 456 ];
$checkColumns = is_array($columns); // $columns !== null (Same result)
$result = [];
foreach ($set as $k => $v) {
// ------------ Following annotation doesn't work for array|string types ----------
/** @var array $columns */
if ($checkColumns && in_array($k, $columns) === false) {
continue;
}
$result[$k] = $v;
}
return $result;
}
}
Expected output
I expect this to resolve/pass with no errors
- Automatically without having to add a docblock
/** @var array $columns */, or - I get that some cases are complicated ... the @var docblock works apparently for classes but doesn't seem to work for
stringorarray. Is there a reason I can't overload the type?
Activity
[-]Ability to Specify Variable as Array/String via @var to Help Static Anaysis[/-][+]Ability to Specify Variable as Array/String via @var to Help Static Analysis (Level 7)[/+]ondrejmirtes commentedon Aug 8, 2018
Hi,
I'm not sure what this issue is about, seems like you're talking about several threads and I need to weave them together 😊 But I'll try.
PHPStan doesn't know that
$columnsis an array when$checkColumnsis checked fortrue. This is really hard to implement and you can usually help static analyser with understanding the code. You can find similar issues here: https://github.com/phpstan/phpstan/milestone/12 (I group this feature requests by the name "Dependent types". You can use theis_arraycall directly: https://phpstan.org/r/a10fbdf2792269b3cde6f5c2d20105adInline
@varannotations are supported: above variable assignments, and above foreach for both key and value variables. It's in your best interest to declare variable type as soon as possible, so it does not make much sense to do it later after the variable is created. That's why you should generally document them properly in parameter typehints and phpDocs.virgofx commentedon Aug 8, 2018
@ondrejmirtes The example in the issue a simple dummed-down case to highlight the issue; however, assume the logic is intensive and the looping is large. In that case I don't want the code to run a function every time to return back a boolean. I want to cache that value outside the loop.
I get it that it's hard to walk the static analysis path (which would be option #1 above solution). Totally okay with PHPStan not picking up 100% of everything ..... however
Would it be possible to just allow the docblock to be specified in this case ?
/** @var array $columns */And have that work? I feel like this would be beneficial to allow and fix a lot of edge cases ( I have another 3-4 issues similar to this I notice where the native types couldn't be overloaded (string/array/int)) .. If I could simply specify via DocBlock ... I would be satisfied.ondrejmirtes commentedon Aug 8, 2018
@virgofx You can do this:
Or use a lower level than 7 ;)
virgofx commentedon Aug 8, 2018
Would it be possible to do it without re-assigning the variable (seems like bad code practice just to satisfy static analysis) ... e.g. could it be added as a feature request? That would be awesome ..... 👍