Skip to content

Ensure trait_exists() always returns bool#7554

Merged
orklah merged 1 commit intovimeo:4.xfrom
Ocramius:fix/#7478-trait_exists-always-returns-boolean
Feb 1, 2022
Merged

Ensure trait_exists() always returns bool#7554
orklah merged 1 commit intovimeo:4.xfrom
Ocramius:fix/#7478-trait_exists-always-returns-boolean

Conversation

@Ocramius
Copy link
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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