From 1c9e65bc8b16b40ff6ae5ebd1b839f1285e97a71 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Apr 2026 10:29:24 -0500 Subject: [PATCH 1/2] Fix ContainerInterface::has() returning always-true for known services 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 --- bleedingEdge.neon | 1 + extension.neon | 4 ++ .../ContainerDynamicReturnTypeExtension.php | 18 +++++---- ...hpunit-drupal-phpstan-no-bleedingedge.neon | 18 +++++++++ ...alContainerDynamicReturnTypeLegacyTest.php | 37 +++++++++++++++++++ tests/src/Type/data/container-legacy-has.php | 13 +++++++ tests/src/Type/data/container.php | 2 +- 7 files changed, 84 insertions(+), 9 deletions(-) create mode 100644 tests/fixtures/config/phpunit-drupal-phpstan-no-bleedingedge.neon create mode 100644 tests/src/Type/DrupalContainerDynamicReturnTypeLegacyTest.php create mode 100644 tests/src/Type/data/container-legacy-has.php diff --git a/bleedingEdge.neon b/bleedingEdge.neon index e1fbc853..0bf5b90b 100644 --- a/bleedingEdge.neon +++ b/bleedingEdge.neon @@ -4,6 +4,7 @@ parameters: checkDeprecatedHooksInApiFiles: false checkCoreDeprecatedHooksInApiFiles: true checkContribDeprecatedHooksInApiFiles: true + containerHasAlwaysTrue: false rules: testClassSuffixNameRule: true dependencySerializationTraitPropertyRule: true diff --git a/extension.neon b/extension.neon index c636a641..9a6c18ba 100644 --- a/extension.neon +++ b/extension.neon @@ -21,6 +21,7 @@ parameters: checkDeprecatedHooksInApiFiles: false checkCoreDeprecatedHooksInApiFiles: false checkContribDeprecatedHooksInApiFiles: false + containerHasAlwaysTrue: true extensions: entityFieldsViaMagicReflection: true entityFieldMethodsViaMagicReflection: true @@ -259,6 +260,7 @@ parametersSchema: checkDeprecatedHooksInApiFiles: boolean() checkCoreDeprecatedHooksInApiFiles: boolean() checkContribDeprecatedHooksInApiFiles: boolean() + containerHasAlwaysTrue: boolean() ]) extensions: structure([ entityFieldsViaMagicReflection: boolean() @@ -325,6 +327,8 @@ services: - class: mglaman\PHPStanDrupal\Type\ContainerDynamicReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] + arguments: + containerHasAlwaysTrue: %drupal.bleedingEdge.containerHasAlwaysTrue% - class: mglaman\PHPStanDrupal\Type\DrupalClassResolverDynamicReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] diff --git a/src/Type/ContainerDynamicReturnTypeExtension.php b/src/Type/ContainerDynamicReturnTypeExtension.php index 5cb87c74..3a071ed7 100644 --- a/src/Type/ContainerDynamicReturnTypeExtension.php +++ b/src/Type/ContainerDynamicReturnTypeExtension.php @@ -7,6 +7,7 @@ use PHPStan\Analyser\Scope; use PHPStan\Reflection\MethodReflection; use PHPStan\Reflection\ParametersAcceptorSelector; +use PHPStan\Type\BooleanType; use PHPStan\Type\Constant\ConstantBooleanType; use PHPStan\Type\DynamicMethodReturnTypeExtension; use PHPStan\Type\NullType; @@ -18,14 +19,11 @@ class ContainerDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension { - /** - * @var ServiceMap - */ - private ServiceMap $serviceMap; - public function __construct(ServiceMap $serviceMap) - { - $this->serviceMap = $serviceMap; + public function __construct( + private ServiceMap $serviceMap, + private bool $containerHasAlwaysTrue, + ) { } public function getClass(): string @@ -62,7 +60,11 @@ public function getTypeFromMethodCall( foreach ($argType->getConstantStrings() as $constantStringType) { $serviceId = $constantStringType->getValue(); $service = $this->serviceMap->getService($serviceId); - $types[] = new ConstantBooleanType($service !== null); + if (!$this->containerHasAlwaysTrue && $service !== null) { + $types[] = new BooleanType(); + } else { + $types[] = new ConstantBooleanType($service !== null); + } } return TypeCombinator::union(...$types); diff --git a/tests/fixtures/config/phpunit-drupal-phpstan-no-bleedingedge.neon b/tests/fixtures/config/phpunit-drupal-phpstan-no-bleedingedge.neon new file mode 100644 index 00000000..d9510dc5 --- /dev/null +++ b/tests/fixtures/config/phpunit-drupal-phpstan-no-bleedingedge.neon @@ -0,0 +1,18 @@ +parameters: + drupal: + entityMapping: + content_entity_using_custom_storage: + class: Drupal\phpstan_fixtures\Entity\ContentEntityUsingCustomStorage + storage: Drupal\phpstan_fixtures\CustomContentEntityStorage + config_entity_using_default_storage: + class: Drupal\phpstan_fixtures\Entity\ConfigEntityUsingDefaultStorage + storage: Drupal\Core\Config\Entity\ConfigEntityStorage + config_entity_using_custom_storage: + class: Drupal\phpstan_fixtures\Entity\ConfigEntityUsingCustomStorage + storage: Drupal\phpstan_fixtures\CustomConfigEntityStorage + content_entity_using_default_storage: + class: Drupal\phpstan_fixtures\Entity\ContentEntityUsingDefaultStorage +includes: + - ../../../extension.neon + - ../../../rules.neon + - ../../../vendor/phpstan/phpstan-deprecation-rules/rules.neon diff --git a/tests/src/Type/DrupalContainerDynamicReturnTypeLegacyTest.php b/tests/src/Type/DrupalContainerDynamicReturnTypeLegacyTest.php new file mode 100644 index 00000000..2bb68928 --- /dev/null +++ b/tests/src/Type/DrupalContainerDynamicReturnTypeLegacyTest.php @@ -0,0 +1,37 @@ +assertFileAsserts($assertType, $file, ...$args); + } +} diff --git a/tests/src/Type/data/container-legacy-has.php b/tests/src/Type/data/container-legacy-has.php new file mode 100644 index 00000000..00934abf --- /dev/null +++ b/tests/src/Type/data/container-legacy-has.php @@ -0,0 +1,13 @@ +has('service_map.my_service')); + assertType('false', $container->has('unknown_service')); +} diff --git a/tests/src/Type/data/container.php b/tests/src/Type/data/container.php index 9b4e052d..f59e3cf1 100644 --- a/tests/src/Type/data/container.php +++ b/tests/src/Type/data/container.php @@ -12,7 +12,7 @@ function test(): void { $container = \Drupal::getContainer(); assertType(MyService::class, $container->get('service_map.my_service')); - assertType('true', $container->has('service_map.my_service')); + assertType('bool', $container->has('service_map.my_service')); assertType('false', $container->has('unknown_service')); assertType(MyService::class, $container->get('service_map.concrete_service')); assertType(MyService::class, $container->get('service_map.concrete_service_with_a_parent_which_has_a_parent')); From 4bb807a364fecda637752320e8bf0277bfe209f2 Mon Sep 17 00:00:00 2001 From: Matt Glaman Date: Thu, 23 Apr 2026 11:03:36 -0500 Subject: [PATCH 2/2] Document containerHasAlwaysTrue bleedingEdge toggle in README Co-Authored-By: Claude Sonnet 4.6 --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index a27e7c07..752e3235 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,7 @@ What it currently enables: - `cacheableDependencyRule` — flags `addCacheableDependency()` calls with non-cacheable arguments - `loggerFromFactoryPropertyAssignmentRule` — flags logger channels assigned to properties in classes using `DependencySerializationTrait` - `entityStorageDirectInjectionRule` — flags direct injection of entity storage into a constructor; inject `EntityTypeManagerInterface` and call `getStorage()` instead +- `containerHasAlwaysTrue: false` — `ContainerInterface::has()` returns `bool` instead of always-`true` for known services, preventing false positives that may cause developers to remove legitimate conditional service guards > [!NOTE] > `checkDeprecatedHooksInApiFiles` is deprecated. Use `checkCoreDeprecatedHooksInApiFiles` and `checkContribDeprecatedHooksInApiFiles` instead.