Skip to content

Commit

Permalink
Fix #1674 - treat intersections more equally regardless of order
Browse files Browse the repository at this point in the history
  • Loading branch information
@muglug
muglug committed May 24, 2019
1 parent a43e4d8 commit 3e2b716
Show file tree
Hide file tree
Showing 4 changed files with 268 additions and 73 deletions.
162 changes: 89 additions & 73 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/MethodCallAnalyzer.php
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -400,7 +400,8 @@ private static function analyzeAtomicCall(
&$invalid_method_call_types, &$invalid_method_call_types,
&$existent_method_ids, &$existent_method_ids,
&$non_existent_class_method_ids, &$non_existent_class_method_ids,
&$non_existent_interface_method_ids &$non_existent_interface_method_ids,
bool &$check_visibility = true
) { ) {
$config = $codebase->config; $config = $codebase->config;


Expand Down Expand Up @@ -520,8 +521,6 @@ private static function analyzeAtomicCall(


$fq_class_name = $lhs_type_part->value; $fq_class_name = $lhs_type_part->value;


$intersection_types = $lhs_type_part->getIntersectionTypes();

$is_mock = ExpressionAnalyzer::isMock($fq_class_name); $is_mock = ExpressionAnalyzer::isMock($fq_class_name);


$has_mock = $has_mock || $is_mock; $has_mock = $has_mock || $is_mock;
Expand Down Expand Up @@ -568,6 +567,54 @@ private static function analyzeAtomicCall(
return false; return false;
} }


$class_storage = $codebase->classlike_storage_provider->get($fq_class_name);

$check_visibility = $check_visibility && !$class_storage->override_method_visibility;

$intersection_types = $lhs_type_part->getIntersectionTypes();

$all_intersection_return_type = null;
$all_intersection_existent_method_ids = [];

if ($intersection_types) {
foreach ($intersection_types as $intersection_type) {
$i_non_existent_class_method_ids = [];
$i_non_existent_interface_method_ids = [];

$intersection_return_type = null;

self::analyzeAtomicCall(
$statements_analyzer,
$stmt,
$codebase,
$context,
$intersection_type,
$lhs_var_id,
$intersection_return_type,
$returns_by_ref,
$has_mock,
$has_valid_method_call_type,
$has_mixed_method_call,
$invalid_method_call_types,
$all_intersection_existent_method_ids,
$i_non_existent_class_method_ids,
$i_non_existent_interface_method_ids,
$check_visibility
);

if ($intersection_return_type) {
if (!$all_intersection_return_type || $all_intersection_return_type->isMixed()) {
$all_intersection_return_type = $intersection_return_type;
} else {
$all_intersection_return_type = Type::intersectUnionTypes(
$all_intersection_return_type,
$intersection_return_type
);
}
}
}
}

if (!$stmt->name instanceof PhpParser\Node\Identifier) { if (!$stmt->name instanceof PhpParser\Node\Identifier) {
if (!$context->ignore_variable_method) { if (!$context->ignore_variable_method) {
$codebase->analyzer->addMixedMemberName( $codebase->analyzer->addMixedMemberName(
Expand Down Expand Up @@ -603,8 +650,6 @@ private static function analyzeAtomicCall(
$statements_analyzer->getSource() $statements_analyzer->getSource()
) )
) { ) {
$class_storage = $codebase->classlike_storage_provider->get($fq_class_name);

$interface_has_method = false; $interface_has_method = false;


if ($class_storage->abstract && $class_storage->class_implements) { if ($class_storage->abstract && $class_storage->class_implements) {
Expand Down Expand Up @@ -668,6 +713,13 @@ private static function analyzeAtomicCall(
$fq_class_name $fq_class_name
); );


if ($all_intersection_return_type) {
$return_type_candidate = Type::intersectUnionTypes(
$all_intersection_return_type,
$return_type_candidate
);
}

if (!$return_type) { if (!$return_type) {
$return_type = $return_type_candidate; $return_type = $return_type_candidate;
} else { } else {
Expand Down Expand Up @@ -733,80 +785,12 @@ function (PhpParser\Node\Arg $arg) {
$fq_class_name = $context->self; $fq_class_name = $context->self;
} }


$check_visibility = true;

if ($intersection_types) {
foreach ($intersection_types as $intersection_type) {
if ($intersection_type instanceof TNamedObject
&& $codebase->interfaceExists($intersection_type->value)
) {
$interface_storage = $codebase->classlike_storage_provider->get($intersection_type->value);

$check_visibility = $check_visibility && !$interface_storage->override_method_visibility;
}
}
}

$is_interface = false; $is_interface = false;


if ($codebase->interfaceExists($fq_class_name)) { if ($codebase->interfaceExists($fq_class_name)) {
$is_interface = true; $is_interface = true;
} }


if ($intersection_types && !$codebase->methodExists($method_id)) {
if ($is_interface) {
$interface_storage = $codebase->classlike_storage_provider->get($fq_class_name);

$check_visibility = $check_visibility && !$interface_storage->override_method_visibility;
}

foreach ($intersection_types as $intersection_type) {
if ($intersection_type instanceof Type\Atomic\TTemplateParam) {
if (!$intersection_type->as->isMixed()
&& !$intersection_type->as->hasObject()
) {
$intersection_type = array_values(
$intersection_type->as->getTypes()
)[0];

if (!$intersection_type instanceof TNamedObject) {
throw new \UnexpectedValueException(
'Shouldn’t get a non-object generic param here'
);
}

$intersection_type->from_docblock = true;
} else {
continue;
}
}

$method_id = $intersection_type->value . '::' . $method_name_lc;
$fq_class_name = $intersection_type->value;

$does_class_exist = ClassLikeAnalyzer::checkFullyQualifiedClassLikeName(
$statements_analyzer,
$fq_class_name,
new CodeLocation($source, $stmt->var),
$statements_analyzer->getSuppressedIssues(),
false,
false,
true,
$intersection_type->from_docblock
);

if (!$does_class_exist) {
return false;
}

if ($codebase->methodExists($method_id)) {
$is_interface = $codebase->interfaceExists($fq_class_name);

break;
}
}
}

$source_method_id = $source instanceof FunctionLikeAnalyzer $source_method_id = $source instanceof FunctionLikeAnalyzer
? $source->getMethodId() ? $source->getMethodId()
: null; : null;
Expand Down Expand Up @@ -854,6 +838,13 @@ function (PhpParser\Node\Arg $arg) {
if ($pseudo_method_storage->return_type) { if ($pseudo_method_storage->return_type) {
$return_type_candidate = clone $pseudo_method_storage->return_type; $return_type_candidate = clone $pseudo_method_storage->return_type;


if ($all_intersection_return_type) {
$return_type_candidate = Type::intersectUnionTypes(
$all_intersection_return_type,
$return_type_candidate
);
}

if (!$return_type) { if (!$return_type) {
$return_type = $return_type_candidate; $return_type = $return_type_candidate;
} else { } else {
Expand All @@ -869,6 +860,18 @@ function (PhpParser\Node\Arg $arg) {
} }
} }


if ($all_intersection_return_type && $all_intersection_existent_method_ids) {
$existent_method_ids = array_merge($existent_method_ids, $all_intersection_existent_method_ids);

if (!$return_type) {
$return_type = $all_intersection_return_type;
} else {
$return_type = Type::combineUnionTypes($all_intersection_return_type, $return_type);
}

return;
}

if ($is_interface) { if ($is_interface) {
$non_existent_interface_method_ids[] = $intersection_method_id ?: $method_id; $non_existent_interface_method_ids[] = $intersection_method_id ?: $method_id;
} else { } else {
Expand Down Expand Up @@ -1193,11 +1196,24 @@ function (Assertion $assertion) use ($class_template_params) : Assertion {
} }


if ($return_type_candidate) { if ($return_type_candidate) {
if ($all_intersection_return_type) {
$return_type_candidate = Type::intersectUnionTypes(
$all_intersection_return_type,
$return_type_candidate
);
}

if (!$return_type) { if (!$return_type) {
$return_type = $return_type_candidate; $return_type = $return_type_candidate;
} else { } else {
$return_type = Type::combineUnionTypes($return_type_candidate, $return_type); $return_type = Type::combineUnionTypes($return_type_candidate, $return_type);
} }
} elseif ($all_intersection_return_type) {
if (!$return_type) {
$return_type = $all_intersection_return_type;
} else {
$return_type = Type::combineUnionTypes($all_intersection_return_type, $return_type);
}
} else { } else {
$return_type = Type::getMixed(); $return_type = Type::getMixed();
} }
Expand Down
101 changes: 101 additions & 0 deletions src/Psalm/Type.php
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -1306,6 +1306,107 @@ public static function combineUnionTypes(
return $combined_type; return $combined_type;
} }


/**
* Combines two union types into one via an intersection
*
* @param Union $type_1
* @param Union $type_2
*
* @return Union
*/
public static function intersectUnionTypes(
Union $type_1,
Union $type_2
) {
if ($type_1->isMixed() && $type_2->isMixed()) {
$combined_type = Type::getMixed();
} else {
$both_failed_reconciliation = false;

if ($type_1->failed_reconciliation) {
if ($type_2->failed_reconciliation) {
$both_failed_reconciliation = true;
} else {
return $type_2;
}
} elseif ($type_2->failed_reconciliation) {
return $type_1;
}

if ($type_1->isMixed() && !$type_2->isMixed()) {
$combined_type = clone $type_2;
} elseif (!$type_1->isMixed() && $type_2->isMixed()) {
$combined_type = clone $type_1;
} else {
$combined_type = clone $type_1;

foreach ($combined_type->getTypes() as $type_1_atomic) {
foreach ($type_2->getTypes() as $type_2_atomic) {
if (($type_1_atomic instanceof TIterable
|| $type_1_atomic instanceof TNamedObject
|| $type_1_atomic instanceof TTemplateParam)
&& ($type_2_atomic instanceof TIterable
|| $type_2_atomic instanceof TNamedObject
|| $type_2_atomic instanceof TTemplateParam)
) {
if (!$type_1_atomic->extra_types) {
$type_1_atomic->extra_types = [];
}

$type_2_atomic_clone = clone $type_2_atomic;

$type_2_atomic_clone->extra_types = [];

$type_1_atomic->extra_types[] = $type_2_atomic_clone;

$type_2_atomic_intersection_types = $type_2_atomic->getIntersectionTypes();

if ($type_2_atomic_intersection_types) {
foreach ($type_2_atomic_intersection_types as $type_2_intersection_type) {
$type_1_atomic->extra_types[] = clone $type_2_intersection_type;
}
}
}
}
}
}

if (!$type_1->initialized && !$type_2->initialized) {
$combined_type->initialized = false;
}

if ($type_1->possibly_undefined_from_try && $type_2->possibly_undefined_from_try) {
$combined_type->possibly_undefined_from_try = true;
}

if ($type_1->from_docblock && $type_2->from_docblock) {
$combined_type->from_docblock = true;
}

if ($type_1->from_calculation && $type_2->from_calculation) {
$combined_type->from_calculation = true;
}

if ($type_1->ignore_nullable_issues && $type_2->ignore_nullable_issues) {
$combined_type->ignore_nullable_issues = true;
}

if ($type_1->ignore_falsable_issues && $type_2->ignore_falsable_issues) {
$combined_type->ignore_falsable_issues = true;
}

if ($both_failed_reconciliation) {
$combined_type->failed_reconciliation = true;
}
}

if ($type_1->possibly_undefined && $type_2->possibly_undefined) {
$combined_type->possibly_undefined = true;
}

return $combined_type;
}

public static function clearCache() : void public static function clearCache() : void
{ {
self::$memoized_tokens = []; self::$memoized_tokens = [];
Expand Down
26 changes: 26 additions & 0 deletions tests/InterfaceTest.php
View file
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -504,6 +504,12 @@ function takeI(I $i) : void {
if ($i instanceof A) { if ($i instanceof A) {
$i->foo(); $i->foo();
} }
}
function takeA(A $a) : void {
if ($a instanceof I) {
$a->foo();
}
}', }',
], ],
'docblockParamInheritance' => [ 'docblockParamInheritance' => [
Expand Down Expand Up @@ -555,6 +561,26 @@ function f(I $c): void {
$c->current(); $c->current();
}' }'
], ],
'intersectMixedTypes' => [
'<?php
interface IFoo {
function foo();
}
interface IBar {
function foo() : string;
}
/** @param IFoo&IBar $i */
function iFooFirst($i) : string {
return $i->foo();
}
/** @param IBar&IFoo $i */
function iBarFirst($i) : string {
return $i->foo();
}',
],
]; ];
} }


Expand Down
Loading

0 comments on commit 3e2b716

Please sign in to comment.