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

Ensure trait_exists() always returns bool #7554

Merged

Conversation

Ocramius
Copy link
Contributor

@Ocramius Ocramius commented Feb 1, 2022

Fixes #7478

As discussed in the upstream issue, trait_exists() always returns bool: while
it can return null when the arguments passed to it do not match (either no arguments, or
3 or more arguments), we do not support that scenario, as that already doesn't respect the
type signature of this function.

We cut to the point: always make it bool, which is the scenario that works under healthy
operational conditions.

Ref: Roave/BetterReflection#983 (comment)
Ref: https://psalm.dev/r/c41a43805d
Ref: #7478 (comment)
Ref: #7478 (comment)
Ref: https://3v4l.org/XpHmh

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/c41a43805d
<?php

function traitDoesIndeedExist(string $traitName): bool
{
    return trait_exists($traitName);
}
Psalm output (using commit 2e01e9b):

ERROR: NullableReturnStatement - 5:12 - The declared return type 'bool' for traitDoesIndeedExist is not nullable, but the function returns 'bool|null'

ERROR: InvalidNullableReturnType - 3:51 - The declared return type 'bool' for traitDoesIndeedExist is not nullable, but 'bool|null' contains null

Fixes vimeo#7478

As discussed in the upstream issue, `trait_exists()` always returns `bool`: while
it can return `null` when the arguments passed to it do not match (either no arguments, or
3 or more arguments), we do not support that scenario, as that already doesn't respect the
type signature of this function.

We cut to the point: always make it `bool`, which is the scenario that works under healthy
operational conditions.

Ref: Roave/BetterReflection#983 (comment)
Ref: https://psalm.dev/r/c41a43805d
Ref: vimeo#7478 (comment)
Ref: vimeo#7478 (comment)
Ref: https://3v4l.org/XpHmh
@Ocramius Ocramius force-pushed the fix/#7478-trait_exists-always-returns-boolean branch from ea8cb6c to fabcda1 Compare February 1, 2022 15:52
@orklah orklah added the release:fix The PR will be included in 'Fixes' section of the release notes label Feb 1, 2022
@orklah orklah merged commit bd14653 into vimeo:4.x Feb 1, 2022
@orklah
Copy link
Collaborator

orklah commented Feb 1, 2022

Thanks!

@Ocramius Ocramius deleted the fix/#7478-trait_exists-always-returns-boolean branch February 1, 2022 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:fix The PR will be included in 'Fixes' section of the release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

trait_exists() returns nullable value, according to psalm stubs
2 participants