Fix ContainerInterface::has() returning always-true for known services#987
Merged
Fix ContainerInterface::has() returning always-true for known services#987
Conversation
Adds drupal.bleedingEdge.containerHasAlwaysTrue flag (default true for 2.0.x backwards compat). When false, has() returns BooleanType for found services instead of ConstantBooleanType(true), preventing false positives that lead developers to remove conditional service guards. Unknown services still return ConstantBooleanType(false) in both modes. bleedingEdge.neon sets containerHasAlwaysTrue: false; will become the default in 2.1.0. Fixes #668 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a configurable switch to control how PHPStan infers the return type of ContainerInterface::has() for known Drupal services, avoiding “always true” false-positives in bleeding-edge mode while keeping 2.0.x compatibility by default.
Changes:
- Introduces
drupal.bleedingEdge.containerHasAlwaysTrue(defaulttrue), and wires it into the containerhas()dynamic return type extension. - Sets the flag to
falseinbleedingEdge.neonso bleeding-edge mode returnsbool(not constanttrue) for known services. - Adds/updates type-inference tests and fixtures to cover both the bleeding-edge and legacy behaviors.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/src/Type/data/container.php | Updates assertion for known-service has() to expect bool under bleeding-edge config. |
| tests/src/Type/data/container-legacy-has.php | Adds legacy-mode fixture asserting known-service has() remains true. |
| tests/src/Type/DrupalContainerDynamicReturnTypeLegacyTest.php | Adds a dedicated test case running with a “no bleedingEdge” config. |
| tests/fixtures/config/phpunit-drupal-phpstan-no-bleedingedge.neon | Adds a test config that omits bleedingEdge.neon to validate legacy defaults. |
| src/Type/ContainerDynamicReturnTypeExtension.php | Implements the new flag: returns BooleanType for known services when enabled. |
| extension.neon | Adds the new parameter + schema entry, and injects it into the extension service. |
| bleedingEdge.neon | Flips the new flag to false in bleeding-edge mode. |
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #668
drupal.bleedingEdge.containerHasAlwaysTrueparameter (defaulttruein 2.0.x for backwards compat)false(set inbleedingEdge.neon),has()returnsboolfor found services instead ofConstantBooleanType(true), eliminating false-positive "always true" errors that could cause developers to remove legitimate conditional service guardsConstantBooleanType(false)in both modesextension.neonTest plan
DrupalContainerDynamicReturnTypeTest— asserts'bool'for known services (bleedingEdge active)DrupalContainerDynamicReturnTypeLegacyTest— asserts'true'for known services (no bleedingEdge, 2.0.x compat)php vendor/bin/phpunit --filter="DrupalContainerDynamicReturnType"php vendor/bin/phpstan analyzephp vendor/bin/phpcs🤖 Generated with Claude Code