Skip to content

floor() and ceil() never return false in PHP 8#954

Closed
jlherren wants to merge 1 commit intoJetBrains:masterfrom
jlherren:patch-1
Closed

floor() and ceil() never return false in PHP 8#954
jlherren wants to merge 1 commit intoJetBrains:masterfrom
jlherren:patch-1

Conversation

@jlherren
Copy link
Copy Markdown
Contributor

No description provided.

@orklah
Copy link
Copy Markdown
Contributor

orklah commented Nov 13, 2020

Seems like they could before PHP8:
https://3v4l.org/XWcND

@jlherren
Copy link
Copy Markdown
Contributor Author

Oh well... Can this be considered an unlikely edge case, which is reported by any decent IDE and static analyzer? Otherwise discard this PR.

@orklah
Copy link
Copy Markdown
Contributor

orklah commented Nov 13, 2020

I guess it's up to the maintainers to decide but it's a delicate subject. By removing this in the stubs, it's like saying that it can never happen. Which means every code that handle this false value, even for edge cases, will be flagged as an error in IDE.

However, Jetbrains are experimenting with attributes lately in the stubs. Maybe they could have a use to differentiate returns between php versions

@MaXal
Copy link
Copy Markdown
Collaborator

MaXal commented Nov 13, 2020

@jlherren Please use #[PhpStormStubsElementAvailable] attribute. See:

#[PhpStormStubsElementAvailable('8.0')]
for example.

@MaXal MaXal closed this Nov 13, 2020
@MaXal MaXal reopened this Nov 13, 2020
@jlherren jlherren changed the title floor() and ceil() never return false floor() and ceil() never return false in PHP 8 Nov 13, 2020
@MaXal
Copy link
Copy Markdown
Collaborator

MaXal commented Nov 18, 2020

We've addressed this situation using #[LanguageLevelTypeAware(["8.0" => "float"], default: "float|false")]

@MaXal MaXal closed this Nov 18, 2020
@jlherren jlherren deleted the patch-1 branch November 18, 2020 21:28
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.

3 participants