Skip to content

[Capability] Remove CompletionProvider's dead providerClass argument - #499

Merged
chr-hertel merged 2 commits into
mainfrom
fix-completion-provider-class
Sep 7, 2026
Merged

[Capability] Remove CompletionProvider's dead providerClass argument#499
chr-hertel merged 2 commits into
mainfrom
fix-completion-provider-class

Conversation

@chr-hertel

@chr-hertel chr-hertel commented Sep 5, 2026

Copy link
Copy Markdown
Member

#[CompletionProvider(providerClass: Foo::class)] has never worked in any release. The constructor's exactly-one-source check was array_filter([$provider, $values, $enum])$providerClass was never counted, so the form always threw Only one of provider, values, or enum can be set. during newInstance(). Discovery catches that and drops the element, so the whole prompt/template silently disappeared. Discoverer::getCompletionProviders() then stored $attributeInstance->provider in the providerClass branch, always null there.

So rather than making it work: provider: already accepts a class-string<ProviderInterface> and resolves it through the container identically, which makes providerClass a redundant spelling that nothing can depend on — every version that shipped it threw on construction. Removing it also unlocks the positional form #[CompletionProvider(Foo::class)], which until now hit providerClass and threw.

[BC Break] in the changelog, though the practical risk is nil: no working code could have set the property.

Covered from both ends. DiscoveryTest pins the positional class-string surviving discovery unresolved, and a new integration test drives a real client against a real server over stdio — the fixture's provider takes a scalar the auto-wiring container cannot supply, so it can only come from Container::set(), and completions arriving at all prove the container was asked to build it.

Found by Copilot's review on #492.

@chr-hertel chr-hertel added Server Issues & PRs related to the Server component bug Something isn't working labels Sep 5, 2026
It always threw on construction, so it never worked in any release. `provider:`
takes the same class-string and is now the first positional argument.
@chr-hertel
chr-hertel force-pushed the fix-completion-provider-class branch from 9d1136c to 3c8c7bb Compare September 7, 2026 20:38
@chr-hertel chr-hertel changed the title [Capability] Make CompletionProvider's providerClass usable [Capability] Remove CompletionProvider's dead providerClass argument Sep 7, 2026
Comment thread CHANGELOG.md Outdated
@chr-hertel chr-hertel added the breaking change Breaking the Backwards Compatibility Promise label Sep 7, 2026
@chr-hertel
chr-hertel merged commit f39625b into main Sep 7, 2026
29 checks passed
@chr-hertel
chr-hertel deleted the fix-completion-provider-class branch September 7, 2026 21:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Breaking the Backwards Compatibility Promise bug Something isn't working Server Issues & PRs related to the Server component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant