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

fix bcmath types. #2217

Closed
wants to merge 1 commit into from
Closed

Conversation

Warxcell
Copy link

@Warxcell Warxcell commented Feb 2, 2023

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

Verified

This commit was signed with the committer’s verified signature.
thg2k Giovanni Giacobbi
@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.

None yet

3 participants