Skip to content

Conversation

@Seldaek
Copy link
Member

@Seldaek Seldaek commented Jul 11, 2024

@staabm

I think these should be doable in a similar way you did in this PR

I am not sure how to do the non-null part. Maybe it's doable by modifying the result of $this->regexShapeMatcher->matchType() to ensure string|null becomes string.. but I suck at phpstan internals :D

If you have a sec to help any time it'd be great, I believe the tests are correct.

@Seldaek Seldaek force-pushed the strict-groups-support branch from 4b2e643 to 9918fa8 Compare July 11, 2024 13:32
@staabm
Copy link
Contributor

staabm commented Jul 11, 2024

wdyt about

$ git diff
diff --git a/src/PHPStan/PregMatchTypeSpecifyingExtension.php b/src/PHPStan/PregMatchTypeSpecifyingExtension.php
index b83ed12..7ee7ec5 100644
--- a/src/PHPStan/PregMatchTypeSpecifyingExtension.php
+++ b/src/PHPStan/PregMatchTypeSpecifyingExtension.php
@@ -11,8 +11,11 @@
 use PHPStan\Analyser\TypeSpecifierContext;
 use PHPStan\Reflection\MethodReflection;
 use PHPStan\TrinaryLogic;
+use PHPStan\Type\Constant\ConstantArrayType;
 use PHPStan\Type\Php\RegexArrayShapeMatcher;
 use PHPStan\Type\StaticMethodTypeSpecifyingExtension;
+use PHPStan\Type\TypeCombinator;
+use PHPStan\Type\Type;

 final class PregMatchTypeSpecifyingExtension implements StaticMethodTypeSpecifyingExtension, TypeSpecifierAwareExtension
 {
@@ -70,6 +73,22 @@ public function specifyTypes(MethodReflection $methodReflection, StaticCall $nod
             return new SpecifiedTypes();
         }

+        if (
+            in_array($methodReflection->getName(), ['matchStrictGroups', 'isMatchStrictGroups'], true)
+            && $matchedType instanceof ConstantArrayType
+        ) {
+            $matchedType = new ConstantArrayType(
+                $matchedType->getKeyTypes(),
+                array_map(static function (Type $valueType): Type {
+                    return TypeCombinator::removeNull($valueType);
+
+                }, $matchedType->getValueTypes()),
+                [0],
+                [],
+                $matchedType->isList()
+            );
+        }
+
         $overwrite = false;
         if ($context->false()) {
             $overwrite = true;

I am not 100% sure, I got the difference of 'matchStrictGroups' vs. 'isMatchStrictGroups'.

the above patch removes all nullable types and also turns all groups into mandatory groups.

@staabm
Copy link
Contributor

staabm commented Jul 11, 2024

I am not 100% sure, I got the difference of 'matchStrictGroups' vs. 'isMatchStrictGroups'.

I wonder whether this even means $matches can never be the empty-array?

@Seldaek
Copy link
Member Author

Seldaek commented Jul 11, 2024

Yeah that looks great thanks!

I wonder whether this even means $matches can never be the empty-array?

Hmm if it matches it cannot, but it is still allowed to NOT match. It only throws if it matches but some groups are null, thus improving the type safety within the code handling the matches as we know we don't have to check for nulls.. Now with better support for optional groups etc in PHPStan this whole thing might be less needed or not at all anymore tbh. I'll have to see.

@Seldaek Seldaek merged commit 59a4eec into composer:2.x Jul 11, 2024
@Seldaek Seldaek deleted the strict-groups-support branch July 11, 2024 14:25
@Seldaek
Copy link
Member Author

Seldaek commented Jul 11, 2024

BTW there was a deprecation warning on $matchedType instanceof ConstantArrayType which I hacked around, possibly wrong :)

@Seldaek
Copy link
Member Author

Seldaek commented Jul 11, 2024

Woops, that does not go so well on the composer repo:

     Internal error: Internal error. while analysing file /var/www/composer/src/Composer/Repository/Vcs/GitHubDriver.php
     Post the following stack trace to https://github.com/phpstan/phpstan/issues/new?template=Bug_report.yaml:
     ## phar:///var/www/composer/vendor/phpstan/phpstan/phpstan.phar/src/Type/Constant/ConstantArrayTypeBuilder.php(150)
     #0 phar:///var/www/composer/vendor/phpstan/phpstan/phpstan.phar/src/Type/Constant/ConstantArrayType.php(576):
     PHPStan\Type\Constant\ConstantArrayTypeBuilder->setOffsetValueType()
     #1 phar:///var/www/composer/vendor/phpstan/phpstan/phpstan.phar/src/Type/TypeCombinator.php(915):
     PHPStan\Type\Constant\ConstantArrayType->setOffsetValueType()

@staabm
Copy link
Contributor

staabm commented Jul 11, 2024

how to reproduce?

@Seldaek
Copy link
Member Author

Seldaek commented Jul 11, 2024

in the composer project:

composer require composer/pcre:2.x-dev

And add - ../vendor/composer/pcre/extension.neon to phpstan/config.neon

Then composer phpstan crashes

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

Successfully merging this pull request may close these issues.

2 participants