Skip to content

Conversation

staabm
Copy link
Contributor

@staabm staabm commented Aug 2, 2024

will do preg_replace_callback_array in a separate PR

@staabm
Copy link
Contributor Author

staabm commented Aug 2, 2024

@staabm staabm marked this pull request as ready for review August 2, 2024 08:33
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@staabm staabm changed the title Bleeding edge - Precise array shape for preg_match_callback() $matches Bleeding edge - Precise array shape for preg_replace_callback() $matches Aug 2, 2024
@ondrejmirtes ondrejmirtes merged commit 8fe28fa into phpstan:1.11.x Aug 3, 2024
@ondrejmirtes
Copy link
Member

Thank you.

@staabm staabm deleted the preg-cb branch August 3, 2024 06:58
@ondrejmirtes
Copy link
Member

Could/should this be improved? https://phpstan.org/r/de0085e1-29c0-4346-849a-79948ec1c385

It's code from phpdoc-parser and copied from PHP-Parser I think, of course I don't understand it at all.

@Seldaek
Copy link
Contributor

Seldaek commented Aug 3, 2024

Sounds feasible in theory to me, as you have a branch of the match group 1 which is "u", but that'd require a way to have partially static strings where you can look up index 0 and if index0 is known then we return it if not then it's just string or ''. That part you have to say if feasible.

@staabm
Copy link
Contributor Author

staabm commented Aug 3, 2024

I don't understand the regex either but I hope I got you right:

we would need dependent types, because $matches[2] only always exists after if ($str[0] === 'u') - right?
I think this kind of dependent type is not even possible in the plain PHPStan type-system (a type depending on the offset of a string)?

@ondrejmirtes
Copy link
Member

Not dependent types, but something that is called tagged unions. Those are supported: https://phpstan.org/r/90cb0d94-b149-4b37-8c20-e5f7d984a160

Do you think the type here can be expressed like that?

@staabm
Copy link
Contributor Author

staabm commented Aug 3, 2024

oh I see. I think we do similar things already in

$onlyOptionalTopLevelGroup = $this->getOnlyOptionalTopLevelGroup($groupList);
$onlyTopLevelAlternationId = $this->getOnlyTopLevelAlternationId($groupList);
if (
!$matchesAll
&& $wasMatched->yes()
&& $onlyOptionalTopLevelGroup !== null
) {
// if only one top level capturing optional group exists
// we build a more precise constant union of a empty-match and a match with the group
$onlyOptionalTopLevelGroup->forceNonOptional();
$combiType = $this->buildArrayType(
$groupList,
$wasMatched,
$trailingOptionals,
$flags ?? 0,
$markVerbs,
$matchesAll,
);
if (!$this->containsUnmatchedAsNull($flags ?? 0, $matchesAll)) {
$combiType = TypeCombinator::union(
new ConstantArrayType([new ConstantIntegerType(0)], [new StringType()], [0], [], true),
$combiType,
);
}
return $combiType;
} elseif (
!$matchesAll
&& $wasMatched->yes()
&& $onlyTopLevelAlternationId !== null
&& array_key_exists($onlyTopLevelAlternationId, $groupCombinations)
) {
$combiTypes = [];
$isOptionalAlternation = false;
foreach ($groupCombinations[$onlyTopLevelAlternationId] as $groupCombo) {
$comboList = $groupList;
$beforeCurrentCombo = true;
foreach ($comboList as $groupId => $group) {
if (in_array($groupId, $groupCombo, true)) {
$isOptionalAlternation = $group->inOptionalAlternation();
$group->forceNonOptional();
$beforeCurrentCombo = false;
} elseif ($beforeCurrentCombo && !$group->resetsGroupCounter()) {
$group->forceNonOptional();
} elseif ($group->getAlternationId() === $onlyTopLevelAlternationId && !$this->containsUnmatchedAsNull($flags ?? 0, $matchesAll)) {
unset($comboList[$groupId]);
}
}
$combiType = $this->buildArrayType(
$comboList,
$wasMatched,
$trailingOptionals,
$flags ?? 0,
$markVerbs,
$matchesAll,
);
$combiTypes[] = $combiType;
foreach ($groupCombo as $groupId) {
$group = $comboList[$groupId];
$group->restoreNonOptional();
}
}
if ($isOptionalAlternation && !$this->containsUnmatchedAsNull($flags ?? 0, $matchesAll)) {
$combiTypes[] = new ConstantArrayType([new ConstantIntegerType(0)], [new StringType()], [0], [], true);
}
return TypeCombinator::union(...$combiTypes);
}

I will have a look

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants