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

Bleeding edge - Precise array shape for preg_replace_callback() $matches #3281

Merged
merged 4 commits into from
Aug 3, 2024

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

//cc @Seldaek @mvorisek

@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
447 of 455 checks passed
@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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants