Skip to content

Fix #77079 odbc_fetch_object has incorrect type signature #3645

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.

Sign up for GitHub

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

Jump to bottom
Closed
ylixir wants to merge 1 commit into from
Closed

Fix #77079 odbc_fetch_object has incorrect type signature #3645

ylixir wants to merge 1 commit into from

Conversation

@ylixir
Copy link

@ylixir ylixir commented Oct 29, 2018 •
edited by petk
Loading

@petk petk added the Bug label Oct 29, 2018
cmb69
cmb69 approved these changes Oct 29, 2018
View reviewed changes
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

carusogabriel
carusogabriel approved these changes Oct 29, 2018
View reviewed changes
Copy link
Contributor

@carusogabriel carusogabriel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation for this function already uses the resource type, go it should be ready to go.

@petk
Copy link
Member

petk commented Oct 29, 2018

Looks ok then. This one goes to PHP-7.1+ then?

@cmb69
Copy link
Member

cmb69 commented Oct 29, 2018

This one goes to PHP-7.1+ then?

If there are indeed static analyzers which use the protos, as the OP claims, this should be treated as bugfix and applied to PHP-7.1+ accordingly.

@petk
Copy link
Member

petk commented Oct 29, 2018

Applied via 8775bea Thank you.

@petk petk closed this Oct 29, 2018
@ylixir
Copy link
Author

ylixir commented Oct 29, 2018

This I think has already been closed, but for posterity, here is a screenshot of the static analyzer we use finding the issue in circle-ci:
image

@sj-i
Copy link
Contributor

sj-i commented Oct 30, 2018

Actually the bug is duplicated in psalm and phan, and the direct cause of this problem is in its repository.
I've send PRs on both psalm and phan.
vimeo/psalm#1043
phan/phan#2094

@petk
Copy link
Member

petk commented Oct 30, 2018

Thanks @sj-i for sending fixes also there. Yes, this makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmb69 cmb69 cmb69 approved these changes

+1 more reviewer

@carusogabriel carusogabriel carusogabriel approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Bug

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@ylixir @petk @cmb69 @sj-i @carusogabriel