Skip to content

Fix ContainerInterface::has() returning always-true for known services#987

Merged
mglaman merged 2 commits intomainfrom
fix/668-container-has-always-true
Apr 24, 2026
Merged

Fix ContainerInterface::has() returning always-true for known services#987
mglaman merged 2 commits intomainfrom
fix/668-container-has-always-true

Conversation

@mglaman
Copy link
Copy Markdown
Owner

@mglaman mglaman commented Apr 23, 2026

Summary

Fixes #668

  • Adds drupal.bleedingEdge.containerHasAlwaysTrue parameter (default true in 2.0.x for backwards compat)
  • When false (set in bleedingEdge.neon), has() returns bool for found services instead of ConstantBooleanType(true), eliminating false-positive "always true" errors that could cause developers to remove legitimate conditional service guards
  • Unknown services still return ConstantBooleanType(false) in both modes
  • Will become the default behavior in 2.1.0 by flipping the default in extension.neon

Test 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 analyze
  • php vendor/bin/phpcs

🤖 Generated with Claude Code

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>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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 (default true), and wires it into the container has() dynamic return type extension.
  • Sets the flag to false in bleedingEdge.neon so bleeding-edge mode returns bool (not constant true) 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.

Comment thread bleedingEdge.neon
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mglaman mglaman merged commit c954592 into main Apr 24, 2026
23 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ContainerInterace::has should never return "always true"

2 participants