Skip to content

Commit a9bc87e

Browse files
orklahorklah
authored
Merge pull request #9656 from boesing/bugfix/issue-8981
(re-)implement object-shape assertions
2 parents 6c7db9e + 66afbb1 commit a9bc87e

File tree

4 files changed

+168
-39
lines changed

4 files changed

+168
-39
lines changed

‎src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php‎

Lines changed: 70 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -125,19 +125,39 @@ public static function analyze(
125125
$statements_analyzer->node_data->setType(
126126
$stmt,
127127
Type::combineUnionTypes(
128-
$lhs_type_part->properties[$prop_name],
128+
TypeExpander::expandUnion(
129+
$statements_analyzer->getCodebase(),
130+
$lhs_type_part->properties[$prop_name],
131+
null,
132+
null,
133+
null,
134+
true,
135+
true,
136+
false,
137+
true,
138+
true,
139+
true,
140+
),
129141
$stmt_type,
130142
),
131143
);
132144

133145
return;
134146
}
135147

148+
$intersection_types = [];
149+
if (!$lhs_type_part instanceof TObject) {
150+
$intersection_types = $lhs_type_part->getIntersectionTypes();
151+
}
152+
136153
// stdClass and SimpleXMLElement are special cases where we cannot infer the return types
137154
// but we don't want to throw an error
138155
// Hack has a similar issue: https://github.com/facebook/hhvm/issues/5164
139156
if ($lhs_type_part instanceof TObject
140-
|| in_array(strtolower($lhs_type_part->value), Config::getInstance()->getUniversalObjectCrates(), true)
157+
|| (
158+
in_array(strtolower($lhs_type_part->value), Config::getInstance()->getUniversalObjectCrates(), true)
159+
&& $intersection_types === []
160+
)
141161
) {
142162
$statements_analyzer->node_data->setType($stmt, Type::getMixed());
143163

@@ -149,8 +169,6 @@ public static function analyze(
149169
return;
150170
}
151171

152-
$intersection_types = $lhs_type_part->getIntersectionTypes() ?: [];
153-
154172
$fq_class_name = $lhs_type_part->value;
155173

156174
$override_property_visibility = false;
@@ -237,39 +255,60 @@ public static function analyze(
237255
// add method before changing fq_class_name
238256
$get_method_id = new MethodIdentifier($fq_class_name, '__get');
239257

240-
if (!$naive_property_exists
241-
&& $class_storage->namedMixins
242-
) {
243-
foreach ($class_storage->namedMixins as $mixin) {
244-
$new_property_id = $mixin->value . '::$' . $prop_name;
258+
if (!$naive_property_exists) {
259+
if ($class_storage->namedMixins) {
260+
foreach ($class_storage->namedMixins as $mixin) {
261+
$new_property_id = $mixin->value . '::$' . $prop_name;
245262

246-
try {
247-
$new_class_storage = $codebase->classlike_storage_provider->get($mixin->value);
248-
} catch (InvalidArgumentException $e) {
249-
$new_class_storage = null;
250-
}
263+
try {
264+
$new_class_storage = $codebase->classlike_storage_provider->get($mixin->value);
265+
} catch (InvalidArgumentException $e) {
266+
$new_class_storage = null;
267+
}
268+
269+
if ($new_class_storage
270+
&& ($codebase->properties->propertyExists(
271+
$new_property_id,
272+
!$in_assignment,
273+
$statements_analyzer,
274+
$context,
275+
$codebase->collect_locations
276+
? new CodeLocation($statements_analyzer->getSource(), $stmt)
277+
: null,
278+
)
279+
|| isset($new_class_storage->pseudo_property_get_types['$' . $prop_name]))
280+
) {
281+
$fq_class_name = $mixin->value;
282+
$lhs_type_part = $mixin;
283+
$class_storage = $new_class_storage;
284+
285+
if (!isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) {
286+
$naive_property_exists = true;
287+
}
251288

252-
if ($new_class_storage
253-
&& ($codebase->properties->propertyExists(
254-
$new_property_id,
255-
!$in_assignment,
289+
$property_id = $new_property_id;
290+
}
291+
}
292+
} elseif ($intersection_types !== [] && !$class_storage->final) {
293+
foreach ($intersection_types as $intersection_type) {
294+
self::analyze(
256295
$statements_analyzer,
296+
$stmt,
257297
$context,
258-
$codebase->collect_locations
259-
? new CodeLocation($statements_analyzer->getSource(), $stmt)
260-
: null,
261-
)
262-
|| isset($new_class_storage->pseudo_property_get_types['$' . $prop_name]))
263-
) {
264-
$fq_class_name = $mixin->value;
265-
$lhs_type_part = $mixin;
266-
$class_storage = $new_class_storage;
298+
$in_assignment,
299+
$var_id,
300+
$stmt_var_id,
301+
$stmt_var_type,
302+
$intersection_type,
303+
$prop_name,
304+
$has_valid_fetch_type,
305+
$invalid_fetch_types,
306+
$is_static_access,
307+
);
267308

268-
if (!isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) {
269-
$naive_property_exists = true;
309+
if ($has_valid_fetch_type) {
310+
return;
270311
}
271-
272-
$property_id = $new_property_id;
273312
}
274313
}
275314
}

‎src/Psalm/Internal/Type/SimpleAssertionReconciler.php‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
use Psalm\Storage\Assertion\NonEmptyCountable;
2929
use Psalm\Storage\Assertion\Truthy;
3030
use Psalm\Type;
31+
use Psalm\Type\Atomic;
3132
use Psalm\Type\Atomic\Scalar;
3233
use Psalm\Type\Atomic\TArray;
3334
use Psalm\Type\Atomic\TArrayKey;
@@ -292,7 +293,9 @@ public static function reconcile(
292293

293294
if ($assertion_type instanceof TObject) {
294295
return self::reconcileObject(
296+
$codebase,
295297
$assertion,
298+
$assertion_type,
296299
$existing_var_type,
297300
$key,
298301
$negated,
@@ -1580,7 +1583,9 @@ private static function reconcileNumeric(
15801583
* @param Reconciler::RECONCILIATION_* $failed_reconciliation
15811584
*/
15821585
private static function reconcileObject(
1586+
Codebase $codebase,
15831587
Assertion $assertion,
1588+
TObject $assertion_type,
15841589
Union $existing_var_type,
15851590
?string $key,
15861591
bool $negated,
@@ -1600,7 +1605,17 @@ private static function reconcileObject(
16001605
$redundant = true;
16011606

16021607
foreach ($existing_var_atomic_types as $type) {
1603-
if ($type->isObjectType()) {
1608+
if (Type::isIntersectionType($assertion_type)
1609+
&& self::areIntersectionTypesAllowed($codebase, $type)
1610+
) {
1611+
$object_types[] = $type->addIntersectionType($assertion_type);
1612+
$redundant = false;
1613+
} elseif ($type instanceof TNamedObject
1614+
&& $codebase->classlike_storage_provider->has($type->value)
1615+
&& $codebase->classlike_storage_provider->get($type->value)->final
1616+
) {
1617+
$redundant = false;
1618+
} elseif ($type->isObjectType()) {
16041619
$object_types[] = $type;
16051620
} elseif ($type instanceof TCallable) {
16061621
$callable_object = new TCallableObject($type->from_docblock, $type);
@@ -1614,16 +1629,26 @@ private static function reconcileObject(
16141629
$redundant = false;
16151630
} elseif ($type instanceof TTemplateParam) {
16161631
if ($type->as->hasObject() || $type->as->hasMixed()) {
1617-
$type = $type->replaceAs(self::reconcileObject(
1632+
/**
1633+
* @psalm-suppress PossiblyInvalidArgument This looks wrong, psalm assumes that $assertion_type
1634+
* can contain TNamedObject due to the reconciliation above
1635+
* regarding {@see Type::isIntersectionType}. Due to the
1636+
* native argument type `TObject`, the variable object will
1637+
* never be `TNamedObject`.
1638+
*/
1639+
$reconciled_type = self::reconcileObject(
1640+
$codebase,
16181641
$assertion,
1642+
$assertion_type,
16191643
$type->as,
16201644
null,
16211645
false,
16221646
null,
16231647
$suppressed_issues,
16241648
$failed_reconciliation,
16251649
$is_equality,
1626-
));
1650+
);
1651+
$type = $type->replaceAs($reconciled_type);
16271652

16281653
$object_types[] = $type;
16291654
}
@@ -2920,4 +2945,22 @@ private static function reconcileClassConstant(
29202945

29212946
return TypeCombiner::combine(array_values($matched_class_constant_types), $codebase);
29222947
}
2948+
2949+
/**
2950+
* @psalm-assert-if-true TCallableObject|TObjectWithProperties|TNamedObject $type
2951+
*/
2952+
private static function areIntersectionTypesAllowed(Codebase $codebase, Atomic $type): bool
2953+
{
2954+
if ($type instanceof TObjectWithProperties || $type instanceof TCallableObject) {
2955+
return true;
2956+
}
2957+
2958+
if (!$type instanceof TNamedObject || !$codebase->classlike_storage_provider->has($type->value)) {
2959+
return false;
2960+
}
2961+
2962+
$class_storage = $codebase->classlike_storage_provider->get($type->value);
2963+
2964+
return !$class_storage->final;
2965+
}
29232966
}

‎src/Psalm/Type.php‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Psalm\Type\Atomic\TArray;
1616
use Psalm\Type\Atomic\TArrayKey;
1717
use Psalm\Type\Atomic\TBool;
18+
use Psalm\Type\Atomic\TCallableObject;
1819
use Psalm\Type\Atomic\TClassString;
1920
use Psalm\Type\Atomic\TClosure;
2021
use Psalm\Type\Atomic\TFalse;
@@ -962,10 +963,18 @@ private static function mayHaveIntersection(Atomic $type, Codebase $codebase): b
962963

963964
private static function hasIntersection(Atomic $type): bool
964965
{
965-
return ($type instanceof TIterable
966-
|| $type instanceof TNamedObject
967-
|| $type instanceof TTemplateParam
968-
|| $type instanceof TObjectWithProperties
969-
) && $type->extra_types;
966+
return self::isIntersectionType($type) && $type->extra_types;
967+
}
968+
969+
/**
970+
* @psalm-assert-if-true TNamedObject|TTemplateParam|TIterable|TObjectWithProperties|TCallableObject $type
971+
*/
972+
public static function isIntersectionType(Atomic $type): bool
973+
{
974+
return $type instanceof TNamedObject
975+
|| $type instanceof TTemplateParam
976+
|| $type instanceof TIterable
977+
|| $type instanceof TObjectWithProperties
978+
|| $type instanceof TCallableObject;
970979
}
971980
}

‎tests/Template/FunctionTemplateAssertTest.php‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -916,6 +916,27 @@ function acceptsArray(array $_list): void {}
916916
$numbersT->assert($mixed);
917917
acceptsArray($mixed);',
918918
],
919+
'assertObjectShape' => [
920+
'code' => '<?php
921+
final class Foo
922+
{
923+
public const STATUS_OK = "ok";
924+
public const STATUS_FAIL = "fail";
925+
}
926+
927+
$foo = new stdClass();
928+
929+
/** @psalm-assert object{status: Foo::STATUS_*} $bar */
930+
function assertObjectShape(object $bar): void {
931+
}
932+
933+
assertObjectShape($foo);
934+
$status = $foo->status;
935+
',
936+
'assertions' => [
937+
'$status===' => "'fail'|'ok'",
938+
],
939+
],
919940
];
920941
}
921942

@@ -1196,6 +1217,23 @@ function fromArray(array $data) : void {
11961217
}',
11971218
'error_message' => 'InvalidDocblock',
11981219
],
1220+
'assertObjectShapeOnFinalClass' => [
1221+
'code' => '<?php
1222+
final class Foo
1223+
{
1224+
}
1225+
1226+
$foo = new Foo();
1227+
1228+
/** @psalm-assert object{status: string} $bar */
1229+
function assertObjectShape(object $bar): void {
1230+
}
1231+
1232+
assertObjectShape($foo);
1233+
$status = $foo->status;
1234+
',
1235+
'error_message' => 'Type Foo for $foo is never',
1236+
],
11991237
];
12001238
}
12011239
}

0 commit comments

Comments
 (0)