-
Notifications
You must be signed in to change notification settings - Fork 504
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
Implement TypeSpecifierContext->getReturnType() #3881
base: 2.1.x
Are you sure you want to change the base?
Conversation
/** @var self[] */ | ||
private static array $registry; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop the caching, as context now contains an additional Type (state)
on my machine this did not make a difference in performance
&& in_array(strtolower((string) $expr->right->name), ['preg_match', 'strlen', 'mb_strlen'], true) | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
atm I whitelisted the cases which I dropped above to keep my sanity.
I feel we can drop it after we removed the hard-coded case list and moved the logic into extensions
the case failling in webmozart-assert is effectively
not sure what todo with it atm. edit: fixed with 19749e8 |
debugging some more, I think more precisely the problem is here:
it dumps edit: fixed with 19749e8 |
$returnType = $context->getReturnType(); | ||
if ($returnType instanceof NeverType) { | ||
return $this->typeSpecifier->create($args[0]->value, $returnType, $context->negate(), $scope); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might move this into TypeSpecifier instead.
when $callType instanceof NeverType
turn all non-by-ref-args into *NEVER*
.
not sure its required though.
This pull request has been marked as ready for review. |
//cc @herndlm |
daf1b02
to
99f1b0a
Compare
} | ||
$newScope = $scope->filterBySpecifiedTypes($result); | ||
$callType = $newScope->getType($expr->right); | ||
$newContext = $context->newWithReturnType($callType); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the future we could use this new api also for ==
and ===
not just <
I think.
trying a minimal approach to see whether the idea works as a whole
for expression like
if (doFoo() > 0)
we want to pass the context specific return-type information into type-specifying extensions so they can decide not just on true/truethy/false/falsy but also on "remaining return type values" (e.g. preg_match returns
0|1|false
)this should help to reduce the number of hardcoded hacks within the TypeSpecifier und move over some cases into extensions (which also allows 3rd party extensions todo similar things, as they can't hard-code hack the TypeSpecifier)
ondrej quoted: