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. 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'));