Fix PHPStan extension for matchAllStrictGroups #38
Conversation
| @@ -82,19 +82,9 @@ public function specifyTypes(MethodReflection $methodReflection, StaticCall $nod | |||
| } | |||
|
|
|||
| if ( | |||
| in_array($methodReflection->getName(), ['matchStrictGroups', 'isMatchStrictGroups'], true) | |||
| && count($matchedType->getConstantArrays()) === 1 | |||
| in_array($methodReflection->getName(), ['matchStrictGroups', 'isMatchStrictGroups', 'matchAllStrictGroups', 'isMatchAllStrictGroups'], true) | |||
There was a problem hiding this comment.
this IF case was missing 'matchAllStrictGroups', 'isMatchAllStrictGroups'
There was a problem hiding this comment.
Yeah I tried that without success tho. But now with the traverser looks much better, thanks!
| @@ -34,4 +39,32 @@ static public function getType(?Arg $flagsArg, Scope $scope): ?Type | |||
| } | |||
| return TypeCombinator::union(...$internalFlagsTypes); | |||
| } | |||
|
|
|||
| static public function removeNullFromMatches(Type $matchesType): Type | |||
There was a problem hiding this comment.
moved this out into the utility class to ease re-use without duplication
| @@ -44,6 +44,10 @@ public function testRule(): void | |||
| 'The isMatchStrictGroups call is potentially unsafe as $matches\' type could not be inferred.', | |||
| 86, | |||
| ], | |||
| [ | |||
| 'The isMatchAllStrictGroups call is unsafe as match groups "test", "1" are optional and may be null.', | |||
There was a problem hiding this comment.
I am not 100% sure this error is expected, as I don't got the use-case of the rule
There was a problem hiding this comment.
This new warning is correct/fine.
There was a problem hiding this comment.
The point of StrictGroups methods is, the method returns only non-null matches, but if one is null it throws, so it isn't safe to use with nullable match groups, which is what this warns about. In the test though you want nullable match groups to ensure they appear not-null in $matches.
|
//cc @Seldaek |
|
Great thanks! |
closes #34