Skip to content

Conversation

@felixarntz
Copy link
Member

No description provided.

@felixarntz felixarntz added this to the 0.4.0 milestone Dec 11, 2025
@felixarntz felixarntz added the [Type] Bug An existing feature does not function as intended label Dec 11, 2025
@github-actions
Copy link

github-actions bot commented Dec 11, 2025

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: felixarntz <flixos90@git.wordpress.org>
Co-authored-by: JasonTheAdams <jason_the_adams@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@felixarntz
Copy link
Member Author

@JasonTheAdams There's some weird PHPStan issue when I change it, see https://github.com/WordPress/php-ai-client/actions/runs/20122278394/job/57744921021?pr=147: I don't see why it complains about the value being a string here. As far as I can tell, the type annotations for the underlying class properties correctly use class-string<ProviderInterface>. Any idea what's going on? 🤔

@JasonTheAdams
Copy link
Member

@felixarntz check out 30b3ffd

So the issue is that we're have a union string|class-string type being returned, and PHPStan isn't particularly good at following type narrowing by tracking the isset() logic the way we were going about it. So PHPStan was never confident when it logically should have been a class-string.

Fortunately, PHPStan has @phpstan-assert-if-true tags that we can use here which helps guide the static analysis system. Happily, it also makes our checks a bit more declarative. This now works! 😄

Copy link
Member

@JasonTheAdams JasonTheAdams left a comment

Choose a reason for hiding this comment

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

Approving this, but you should as well, @felixarntz, since I ended up changing more code than you. Hahah!

Copy link
Member Author

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

@JasonTheAdams Awesome! Really appreciate learning more gotchas about PHPStan from you :)

@felixarntz felixarntz merged commit 05bb759 into trunk Dec 11, 2025
6 checks passed
@felixarntz felixarntz deleted the fix/provider-class-name-return-type branch December 11, 2025 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants