Skip to content

Commit c954592

Browse files
mglamanclaude
andauthored
Fix ContainerInterface::has() returning always-true for known services (#987)
* 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 <noreply@anthropic.com> * Document containerHasAlwaysTrue bleedingEdge toggle in README Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d2c6d6b commit c954592

8 files changed

Lines changed: 85 additions & 9 deletions

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ What it currently enables:
183183
- `cacheableDependencyRule` — flags `addCacheableDependency()` calls with non-cacheable arguments
184184
- `loggerFromFactoryPropertyAssignmentRule` — flags logger channels assigned to properties in classes using `DependencySerializationTrait`
185185
- `entityStorageDirectInjectionRule` — flags direct injection of entity storage into a constructor; inject `EntityTypeManagerInterface` and call `getStorage()` instead
186+
- `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
186187

187188
> [!NOTE]
188189
> `checkDeprecatedHooksInApiFiles` is deprecated. Use `checkCoreDeprecatedHooksInApiFiles` and `checkContribDeprecatedHooksInApiFiles` instead.

bleedingEdge.neon

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ parameters:
44
checkDeprecatedHooksInApiFiles: false
55
checkCoreDeprecatedHooksInApiFiles: true
66
checkContribDeprecatedHooksInApiFiles: true
7+
containerHasAlwaysTrue: false
78
rules:
89
testClassSuffixNameRule: true
910
dependencySerializationTraitPropertyRule: true

extension.neon

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ parameters:
2121
checkDeprecatedHooksInApiFiles: false
2222
checkCoreDeprecatedHooksInApiFiles: false
2323
checkContribDeprecatedHooksInApiFiles: false
24+
containerHasAlwaysTrue: true
2425
extensions:
2526
entityFieldsViaMagicReflection: true
2627
entityFieldMethodsViaMagicReflection: true
@@ -259,6 +260,7 @@ parametersSchema:
259260
checkDeprecatedHooksInApiFiles: boolean()
260261
checkCoreDeprecatedHooksInApiFiles: boolean()
261262
checkContribDeprecatedHooksInApiFiles: boolean()
263+
containerHasAlwaysTrue: boolean()
262264
])
263265
extensions: structure([
264266
entityFieldsViaMagicReflection: boolean()
@@ -325,6 +327,8 @@ services:
325327
-
326328
class: mglaman\PHPStanDrupal\Type\ContainerDynamicReturnTypeExtension
327329
tags: [phpstan.broker.dynamicMethodReturnTypeExtension]
330+
arguments:
331+
containerHasAlwaysTrue: %drupal.bleedingEdge.containerHasAlwaysTrue%
328332
-
329333
class: mglaman\PHPStanDrupal\Type\DrupalClassResolverDynamicReturnTypeExtension
330334
tags: [phpstan.broker.dynamicMethodReturnTypeExtension]

src/Type/ContainerDynamicReturnTypeExtension.php

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PHPStan\Analyser\Scope;
88
use PHPStan\Reflection\MethodReflection;
99
use PHPStan\Reflection\ParametersAcceptorSelector;
10+
use PHPStan\Type\BooleanType;
1011
use PHPStan\Type\Constant\ConstantBooleanType;
1112
use PHPStan\Type\DynamicMethodReturnTypeExtension;
1213
use PHPStan\Type\NullType;
@@ -18,14 +19,11 @@
1819

1920
class ContainerDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension
2021
{
21-
/**
22-
* @var ServiceMap
23-
*/
24-
private ServiceMap $serviceMap;
2522

26-
public function __construct(ServiceMap $serviceMap)
27-
{
28-
$this->serviceMap = $serviceMap;
23+
public function __construct(
24+
private ServiceMap $serviceMap,
25+
private bool $containerHasAlwaysTrue,
26+
) {
2927
}
3028

3129
public function getClass(): string
@@ -62,7 +60,11 @@ public function getTypeFromMethodCall(
6260
foreach ($argType->getConstantStrings() as $constantStringType) {
6361
$serviceId = $constantStringType->getValue();
6462
$service = $this->serviceMap->getService($serviceId);
65-
$types[] = new ConstantBooleanType($service !== null);
63+
if (!$this->containerHasAlwaysTrue && $service !== null) {
64+
$types[] = new BooleanType();
65+
} else {
66+
$types[] = new ConstantBooleanType($service !== null);
67+
}
6668
}
6769

6870
return TypeCombinator::union(...$types);
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
parameters:
2+
drupal:
3+
entityMapping:
4+
content_entity_using_custom_storage:
5+
class: Drupal\phpstan_fixtures\Entity\ContentEntityUsingCustomStorage
6+
storage: Drupal\phpstan_fixtures\CustomContentEntityStorage
7+
config_entity_using_default_storage:
8+
class: Drupal\phpstan_fixtures\Entity\ConfigEntityUsingDefaultStorage
9+
storage: Drupal\Core\Config\Entity\ConfigEntityStorage
10+
config_entity_using_custom_storage:
11+
class: Drupal\phpstan_fixtures\Entity\ConfigEntityUsingCustomStorage
12+
storage: Drupal\phpstan_fixtures\CustomConfigEntityStorage
13+
content_entity_using_default_storage:
14+
class: Drupal\phpstan_fixtures\Entity\ContentEntityUsingDefaultStorage
15+
includes:
16+
- ../../../extension.neon
17+
- ../../../rules.neon
18+
- ../../../vendor/phpstan/phpstan-deprecation-rules/rules.neon
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace mglaman\PHPStanDrupal\Tests\Type;
6+
7+
use PHPStan\Testing\TypeInferenceTestCase;
8+
9+
final class DrupalContainerDynamicReturnTypeLegacyTest extends TypeInferenceTestCase
10+
{
11+
12+
public static function getAdditionalConfigFiles(): array
13+
{
14+
return array_merge(parent::getAdditionalConfigFiles(), [
15+
__DIR__ . '/../../fixtures/config/phpunit-drupal-phpstan-no-bleedingedge.neon',
16+
]);
17+
}
18+
19+
public static function dataFileAsserts(): iterable
20+
{
21+
yield from self::gatherAssertTypes(__DIR__ . '/data/container-legacy-has.php');
22+
}
23+
24+
/**
25+
* @dataProvider dataFileAsserts
26+
* @param string $assertType
27+
* @param string $file
28+
* @param mixed ...$args
29+
*/
30+
public function testFileAsserts(
31+
string $assertType,
32+
string $file,
33+
...$args
34+
): void {
35+
$this->assertFileAsserts($assertType, $file, ...$args);
36+
}
37+
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
namespace DrupalContainerStaticLegacy;
4+
5+
use Drupal\service_map\MyService;
6+
use function PHPStan\Testing\assertType;
7+
8+
function test(): void {
9+
$container = \Drupal::getContainer();
10+
11+
assertType('true', $container->has('service_map.my_service'));
12+
assertType('false', $container->has('unknown_service'));
13+
}

tests/src/Type/data/container.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ function test(): void {
1212
$container = \Drupal::getContainer();
1313

1414
assertType(MyService::class, $container->get('service_map.my_service'));
15-
assertType('true', $container->has('service_map.my_service'));
15+
assertType('bool', $container->has('service_map.my_service'));
1616
assertType('false', $container->has('unknown_service'));
1717
assertType(MyService::class, $container->get('service_map.concrete_service'));
1818
assertType(MyService::class, $container->get('service_map.concrete_service_with_a_parent_which_has_a_parent'));

0 commit comments

Comments
 (0)