Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(re-)implement object-shape assertions #9656

Merged
merged 1 commit into from
Apr 16, 2023
Merged

(re-)implement object-shape assertions #9656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
101 changes: 70 additions & 31 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Fetch/AtomicPropertyFetchAnalyzer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,19 +125,39 @@ public static function analyze(
$statements_analyzer->node_data->setType(
$stmt,
Type::combineUnionTypes(
$lhs_type_part->properties[$prop_name],
TypeExpander::expandUnion(
$statements_analyzer->getCodebase(),
$lhs_type_part->properties[$prop_name],
null,
null,
null,
true,
true,
false,
true,
true,
true,
),
$stmt_type,
),
);

return;
}

$intersection_types = [];
if (!$lhs_type_part instanceof TObject) {
$intersection_types = $lhs_type_part->getIntersectionTypes();
}

// stdClass and SimpleXMLElement are special cases where we cannot infer the return types
// but we don't want to throw an error
// Hack has a similar issue: https://github.com/facebook/hhvm/issues/5164
if ($lhs_type_part instanceof TObject
|| in_array(strtolower($lhs_type_part->value), Config::getInstance()->getUniversalObjectCrates(), true)
|| (
in_array(strtolower($lhs_type_part->value), Config::getInstance()->getUniversalObjectCrates(), true)
&& $intersection_types === []
)
) {
$statements_analyzer->node_data->setType($stmt, Type::getMixed());

Expand All @@ -149,8 +169,6 @@ public static function analyze(
return;
}

$intersection_types = $lhs_type_part->getIntersectionTypes() ?: [];

$fq_class_name = $lhs_type_part->value;

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

if (!$naive_property_exists
&& $class_storage->namedMixins
) {
foreach ($class_storage->namedMixins as $mixin) {
$new_property_id = $mixin->value . '::$' . $prop_name;
if (!$naive_property_exists) {
if ($class_storage->namedMixins) {
foreach ($class_storage->namedMixins as $mixin) {
$new_property_id = $mixin->value . '::$' . $prop_name;

try {
$new_class_storage = $codebase->classlike_storage_provider->get($mixin->value);
} catch (InvalidArgumentException $e) {
$new_class_storage = null;
}
try {
$new_class_storage = $codebase->classlike_storage_provider->get($mixin->value);
} catch (InvalidArgumentException $e) {
$new_class_storage = null;
}

if ($new_class_storage
&& ($codebase->properties->propertyExists(
$new_property_id,
!$in_assignment,
$statements_analyzer,
$context,
$codebase->collect_locations
? new CodeLocation($statements_analyzer->getSource(), $stmt)
: null,
)
|| isset($new_class_storage->pseudo_property_get_types['$' . $prop_name]))
) {
$fq_class_name = $mixin->value;
$lhs_type_part = $mixin;
$class_storage = $new_class_storage;

if (!isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) {
$naive_property_exists = true;
}

if ($new_class_storage
&& ($codebase->properties->propertyExists(
$new_property_id,
!$in_assignment,
$property_id = $new_property_id;
}
}
} elseif ($intersection_types !== [] && !$class_storage->final) {
foreach ($intersection_types as $intersection_type) {
self::analyze(
$statements_analyzer,
$stmt,
$context,
$codebase->collect_locations
? new CodeLocation($statements_analyzer->getSource(), $stmt)
: null,
)
|| isset($new_class_storage->pseudo_property_get_types['$' . $prop_name]))
) {
$fq_class_name = $mixin->value;
$lhs_type_part = $mixin;
$class_storage = $new_class_storage;
$in_assignment,
$var_id,
$stmt_var_id,
$stmt_var_type,
$intersection_type,
$prop_name,
$has_valid_fetch_type,
$invalid_fetch_types,
$is_static_access,
);

if (!isset($new_class_storage->pseudo_property_get_types['$' . $prop_name])) {
$naive_property_exists = true;
if ($has_valid_fetch_type) {
return;
}

$property_id = $new_property_id;
}
}
}
Expand Down
49 changes: 46 additions & 3 deletions src/Psalm/Internal/Type/SimpleAssertionReconciler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
use Psalm\Storage\Assertion\NonEmptyCountable;
use Psalm\Storage\Assertion\Truthy;
use Psalm\Type;
use Psalm\Type\Atomic;
use Psalm\Type\Atomic\Scalar;
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Atomic\TArrayKey;
Expand Down Expand Up @@ -292,7 +293,9 @@ public static function reconcile(

if ($assertion_type instanceof TObject) {
return self::reconcileObject(
$codebase,
$assertion,
$assertion_type,
$existing_var_type,
$key,
$negated,
Expand Down Expand Up @@ -1580,7 +1583,9 @@ private static function reconcileNumeric(
* @param Reconciler::RECONCILIATION_* $failed_reconciliation
*/
private static function reconcileObject(
Codebase $codebase,
Assertion $assertion,
TObject $assertion_type,
Union $existing_var_type,
?string $key,
bool $negated,
Expand All @@ -1600,7 +1605,17 @@ private static function reconcileObject(
$redundant = true;

foreach ($existing_var_atomic_types as $type) {
if ($type->isObjectType()) {
if (Type::isIntersectionType($assertion_type)
&& self::areIntersectionTypesAllowed($codebase, $type)
) {
$object_types[] = $type->addIntersectionType($assertion_type);
$redundant = false;
} elseif ($type instanceof TNamedObject
&& $codebase->classlike_storage_provider->has($type->value)
&& $codebase->classlike_storage_provider->get($type->value)->final
) {
$redundant = false;
} elseif ($type->isObjectType()) {
$object_types[] = $type;
} elseif ($type instanceof TCallable) {
$callable_object = new TCallableObject($type->from_docblock, $type);
Expand All @@ -1614,16 +1629,26 @@ private static function reconcileObject(
$redundant = false;
} elseif ($type instanceof TTemplateParam) {
if ($type->as->hasObject() || $type->as->hasMixed()) {
$type = $type->replaceAs(self::reconcileObject(
/**
* @psalm-suppress PossiblyInvalidArgument This looks wrong, psalm assumes that $assertion_type
* can contain TNamedObject due to the reconciliation above
* regarding {@see Type::isIntersectionType}. Due to the
* native argument type `TObject`, the variable object will
* never be `TNamedObject`.
*/
$reconciled_type = self::reconcileObject(
$codebase,
$assertion,
$assertion_type,
$type->as,
null,
false,
null,
$suppressed_issues,
$failed_reconciliation,
$is_equality,
));
);
$type = $type->replaceAs($reconciled_type);

$object_types[] = $type;
}
Expand Down Expand Up @@ -2920,4 +2945,22 @@ private static function reconcileClassConstant(

return TypeCombiner::combine(array_values($matched_class_constant_types), $codebase);
}

/**
* @psalm-assert-if-true TCallableObject|TObjectWithProperties|TNamedObject $type
*/
private static function areIntersectionTypesAllowed(Codebase $codebase, Atomic $type): bool
{
if ($type instanceof TObjectWithProperties || $type instanceof TCallableObject) {
return true;
}

if (!$type instanceof TNamedObject || !$codebase->classlike_storage_provider->has($type->value)) {
return false;
}

$class_storage = $codebase->classlike_storage_provider->get($type->value);

return !$class_storage->final;
}
}
19 changes: 14 additions & 5 deletions src/Psalm/Type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use Psalm\Type\Atomic\TArray;
use Psalm\Type\Atomic\TArrayKey;
use Psalm\Type\Atomic\TBool;
use Psalm\Type\Atomic\TCallableObject;
use Psalm\Type\Atomic\TClassString;
use Psalm\Type\Atomic\TClosure;
use Psalm\Type\Atomic\TFalse;
Expand Down Expand Up @@ -962,10 +963,18 @@ private static function mayHaveIntersection(Atomic $type, Codebase $codebase): b

private static function hasIntersection(Atomic $type): bool
{
return ($type instanceof TIterable
|| $type instanceof TNamedObject
|| $type instanceof TTemplateParam
|| $type instanceof TObjectWithProperties
) && $type->extra_types;
return self::isIntersectionType($type) && $type->extra_types;
}

/**
* @psalm-assert-if-true TNamedObject|TTemplateParam|TIterable|TObjectWithProperties|TCallableObject $type
*/
public static function isIntersectionType(Atomic $type): bool
{
return $type instanceof TNamedObject
|| $type instanceof TTemplateParam
|| $type instanceof TIterable
|| $type instanceof TObjectWithProperties
|| $type instanceof TCallableObject;
}
}
38 changes: 38 additions & 0 deletions tests/Template/FunctionTemplateAssertTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -916,6 +916,27 @@ function acceptsArray(array $_list): void {}
$numbersT->assert($mixed);
acceptsArray($mixed);',
],
'assertObjectShape' => [
'code' => '<?php
final class Foo
{
public const STATUS_OK = "ok";
public const STATUS_FAIL = "fail";
}

$foo = new stdClass();

/** @psalm-assert object{status: Foo::STATUS_*} $bar */
function assertObjectShape(object $bar): void {
}

assertObjectShape($foo);
$status = $foo->status;
',
'assertions' => [
'$status===' => "'fail'|'ok'",
],
],
];
}

Expand Down Expand Up @@ -1196,6 +1217,23 @@ function fromArray(array $data) : void {
}',
'error_message' => 'InvalidDocblock',
],
'assertObjectShapeOnFinalClass' => [
'code' => '<?php
final class Foo
{
}

$foo = new Foo();

/** @psalm-assert object{status: string} $bar */
function assertObjectShape(object $bar): void {
}

assertObjectShape($foo);
$status = $foo->status;
',
'error_message' => 'Type Foo for $foo is never',
],
];
}
}