Skip to content

fix bcmath types.#2217

Closed
Warxcell wants to merge 1 commit intophpstan:1.10.xfrom
Warxcell:fix_bcmath_types
Closed

fix bcmath types.#2217
Warxcell wants to merge 1 commit intophpstan:1.10.xfrom
Warxcell:fix_bcmath_types

Conversation

@Warxcell
Copy link

@Warxcell Warxcell commented Feb 2, 2023

Reference in psalm: https://psalm.dev/r/3786d05a6c

@ondrejmirtes
Copy link
Member

I'm hesitating to merge this for same reason as here: #2163 (comment)

With this update, users need to update their PHPDocs with numeric-string...

@orklah
Copy link
Contributor

orklah commented Feb 6, 2023

If it can help with deciding, bcadd(and others) is documented as taking numeric-strings in Psalm for almost 3 years now: vimeo/psalm#3189

As far as I know, no one complained about that. Though I admit it may be biased because users likely to come and create an issue are users that generally will want more strict checks,

@Warxcell
Copy link
Author

Warxcell commented Feb 6, 2023

I'm hesitating to merge this for same reason as here: #2163 (comment)

With this update, users need to update their PHPDocs with numeric-string...

but that's a good thing, right? I mean: if they accept user string - the method that returns that string will be just string - so phpstan will force them to take that string through is_numeric check, otherwise they will get an exception in runtime - which is basically what static analysers are trying to prevent.

image

@ondrejmirtes
Copy link
Member

Commited as 3fb919a

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