Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Improve error message when the first argument of __state_state isn'…#5463

Closed
carusogabriel wants to merge 1 commit into
php:masterphp/php-src:masterfrom
carusogabriel:php8/improve-error-messageCopy head branch name to clipboard
Closed

Improve error message when the first argument of __state_state isn'…#5463
carusogabriel wants to merge 1 commit into
php:masterphp/php-src:masterfrom
carusogabriel:php8/improve-error-messageCopy head branch name to clipboard

Conversation

@carusogabriel
Copy link
Copy Markdown
Contributor

@carusogabriel carusogabriel commented Apr 26, 2020

…t typed as array

Currently, the error message shown to the user is:

Argument 1 passed to A::__set_state() must be of the type string, array given

Which isn't necessary that. The problem is before that: the type that is actually wrong. With this patch,
the error message now is:

Type of first argument of Foo::__set_state() must be array

This is proposed separately from #4177 as I believe is an improvement, not a BC.

…t typed as array

Currently, the error message shown to the user is:

```
Argument 1 passed to A::__set_state() must be of the type string, array given
```

Which isn't necessary that. The problem is before that: the type that is actually wrong. With this patch,
the error message now is:

```
Type of first argument of Foo::__set_state() must be array
```
Comment thread Zend/zend_API.c
&& ZEND_TYPE_IS_SET(fptr->common.arg_info[0].type)
&& ZEND_TYPE_FULL_MASK(fptr->common.arg_info[1].type) != MAY_BE_ARRAY
) {
zend_error(error_type, "Type of first argument of %s::__set_state() must be array", ZSTR_VAL(ce->name));
Copy link
Copy Markdown
Member

@kocsismate kocsismate Apr 26, 2020

Choose a reason for hiding this comment

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

Can you please use the %s::__set_state(): Argument #1 ($%s) must be of type array, %s given form?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll adopt it in #4177, thanks!

@nikic
Copy link
Copy Markdown
Member

nikic commented Apr 26, 2020

I don't understand why this is separate from #4177.

@carusogabriel carusogabriel deleted the php8/improve-error-message branch April 26, 2020 12:50
@carusogabriel
Copy link
Copy Markdown
Contributor Author

@nikic Yeah, looks like this will need to go there. I'll keep #5462 separate as I believe it has the same intentions as #5441.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.