Skip to content

Conversation

@SteenSchutt
Copy link
Contributor

WPConfigTransformer::get_value() never returns an array, but array is the return type being hinted in the associated docblock. I suggest changing it to string|null. I'm not aware of a situation in which it can return anything else. Feel free to correct me if I'm wrong, but through testing with int, float, string, array, stdClass and undefined variables I haven't managed to get anything else out of it.

@SteenSchutt SteenSchutt requested a review from a team as a code owner April 26, 2023 14:08
* @param string $name Config name.
*
* @return array
* @return string|null
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* @return string|null
* @return mixed

Maybe this would be a little safer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was thinking the same originally, but hinting nullable string makes it clear that the method does not cast the value to it's type, and maybe to some extent that undefined variables make it return null 🙂

Copy link
Member

Choose a reason for hiding this comment

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

Works for me 👍

@danielbachhuber danielbachhuber added this to the 1.3.3 milestone Apr 26, 2023
@danielbachhuber danielbachhuber self-requested a review April 26, 2023 19:01
@danielbachhuber danielbachhuber merged commit b1a6a01 into wp-cli:main Apr 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants