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

Catch missing classlike exceptions during scanning #10720

Merged
merged 2 commits into from
Feb 22, 2024
Merged

Catch missing classlike exceptions during scanning #10720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7bd35be
Catch missing classlike exceptions during scanning
weirdan Feb 19, 2024
4b827d3
Add more tests for native intersection types in class properties
ohader Feb 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 30 additions & 26 deletions src/Psalm/Type.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -888,33 +888,37 @@ private static function intersectAtomicTypes(
}

if (null === $intersection_atomic) {
if (AtomicTypeComparator::isContainedBy(
$codebase,
$type_2_atomic,
$type_1_atomic,
$allow_interface_equality,
$allow_float_int_equality,
)) {
$intersection_atomic = $type_2_atomic;
$wider_type = $type_1_atomic;
$intersection_performed = true;
} elseif (AtomicTypeComparator::isContainedBy(
$codebase,
$type_1_atomic,
$type_2_atomic,
$allow_interface_equality,
$allow_float_int_equality,
)) {
$intersection_atomic = $type_1_atomic;
$wider_type = $type_2_atomic;
$intersection_performed = true;
}
try {
if (AtomicTypeComparator::isContainedBy(
$codebase,
$type_2_atomic,
$type_1_atomic,
$allow_interface_equality,
$allow_float_int_equality,
)) {
$intersection_atomic = $type_2_atomic;
$wider_type = $type_1_atomic;
$intersection_performed = true;
} elseif (AtomicTypeComparator::isContainedBy(
$codebase,
$type_1_atomic,
$type_2_atomic,
$allow_interface_equality,
$allow_float_int_equality,
)) {
$intersection_atomic = $type_1_atomic;
$wider_type = $type_2_atomic;
$intersection_performed = true;
}

if ($intersection_atomic
&& !self::hasIntersection($type_1_atomic)
&& !self::hasIntersection($type_2_atomic)
) {
return $intersection_atomic;
if ($intersection_atomic
&& !self::hasIntersection($type_1_atomic)
&& !self::hasIntersection($type_2_atomic)
) {
return $intersection_atomic;
}
} catch (InvalidArgumentException $e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Muting this exception would not address the underlying issue: The class-like storage items (e.g. for \JsonSerializable or \Stringable and any other external interface) not being available (yet) at that time during the scanning process..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The question is, do we get broken data (e.g. Countable&Iterator turning into Countable), or do we just not get as tight an intersection as possible (e.g. MySessionHandler&SessionHandlerInterface not simplified to MySessionHandler). If it's the latter, it's probably not the end of the world and would be handled during the analysis phase at the cost of some wasted CPU cycles.

// Ignore non-existing classes during initial scan
}
}

Expand Down
74 changes: 74 additions & 0 deletions tests/NativeIntersectionsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,64 @@ function test(A&B $in): void {
'ignored_issues' => [],
'php_version' => '8.1',
],
'nativeTypeIntersectionAsClassProperty' => [
'code' => '<?php
interface A {}
interface B {}
class C implements A, B {}
class D {
private A&B $intersection;
public function __construct()
{
$this->intersection = new C();
}
}
',
'assertions' => [],
'ignored_issues' => [],
'php_version' => '8.1',
],
'nativeTypeIntersectionAsClassPropertyUsingProcessedInterfaces' => [
'code' => '<?php
interface A {}
interface B {}
class AB implements A, B {}
class C {
private A&B $other;
public function __construct()
{
$this->other = new AB();
}
}
',
'assertions' => [],
'ignored_issues' => [],
'php_version' => '8.1',
],
'nativeTypeIntersectionAsClassPropertyUsingUnprocessedInterfaces' => [
'code' => '<?php
class StringableJson implements \Stringable, \JsonSerializable {
public function jsonSerialize(): array
{
return [];
}
public function __toString(): string
{
return json_encode($this);
}
}
class C {
private \Stringable&\JsonSerializable $other;
public function __construct()
{
$this->other = new StringableJson();
}
}
',
'assertions' => [],
'ignored_issues' => [],
'php_version' => '8.1',
],
];
}

Expand Down Expand Up @@ -136,6 +194,22 @@ function foo (A&B $test): A&B {
'ignored_issues' => [],
'php_version' => '8.0',
],
'nativeTypeIntersectionAsClassPropertyUsingUnknownInterfaces' => [
'code' => '<?php
class C {
private \Example\Unknown\A&\Example\Unknown\B $other;
public function __construct()
{
$this->other = new \Example\Unknown\AB();
}
}
',
// @todo decide whether a fall-back should be implemented, that allows to by-pass this failure (opt-in config)
// `UndefinedClass - src/somefile.php:3:33 - Class, interface or enum named Example\Unknown\B does not exist`
'error_message' => 'UndefinedClass',
'ignored_issues' => [],
'php_version' => '8.1',
],
];
}
}