Skip to content

Commit

Permalink
Fix #6066 - introduce more robust system for capturing template const…
Browse files Browse the repository at this point in the history 
…raints (#6072)

* Fix #6066 - add better system for capturing template constraints

* Fix comment
  • Loading branch information
@muglug
muglug authored Jul 11, 2021
1 parent 83bf9b8 commit acc7ee2
Show file tree
Hide file tree
Showing 24 changed files with 659 additions and 143 deletions.
5 changes: 4 additions & 1 deletion src/Psalm/Internal/Analyzer/ClassAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -754,7 +754,10 @@ public static function addContextProperties(
$pt_name = array_keys($parent_storage->template_types)[$pt_offset];
if (isset($template_standins->lower_bounds[$pt_name][$parent_class])) {
$lower_bounds[$pt_name][$parent_class] =
$template_standins->lower_bounds[$pt_name][$parent_class]->type;
TemplateStandinTypeReplacer::getMostSpecificTypeFromBounds(
$template_standins->lower_bounds[$pt_name][$parent_class],
$codebase
);
}
}
}
Expand Down
14 changes: 8 additions & 6 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -367,17 +367,19 @@ private static function checkFunctionLikeTypeMatches(
[$template_type->param_name]
[$template_type->defining_class]
)) {
$template_result->lower_bounds[$template_type->param_name][$template_type->defining_class]
= new TemplateBound(
$template_result->lower_bounds[$template_type->param_name][$template_type->defining_class] = [
new TemplateBound(
clone $template_result->upper_bounds
[$template_type->param_name]
[$template_type->defining_class]->type
);
)
];
} else {
$template_result->lower_bounds[$template_type->param_name][$template_type->defining_class]
= new TemplateBound(
$template_result->lower_bounds[$template_type->param_name][$template_type->defining_class] = [
new TemplateBound(
clone $template_type->as
);
)
];
}
}
}
Expand Down
16 changes: 11 additions & 5 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentsAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
use function count;
use function in_array;
use function is_string;
use function reset;
use function strpos;
use function strtolower;

Expand Down Expand Up @@ -327,10 +328,13 @@ private static function handleClosureArg(

$replace_template_result = new \Psalm\Internal\Type\TemplateResult(
array_map(
function ($template_map) {
function ($template_map) use ($codebase) {
return array_map(
function ($bound) {
return $bound->type;
function ($lower_bounds) use ($codebase) {
return \Psalm\Internal\Type\TemplateStandinTypeReplacer::getMostSpecificTypeFromBounds(
$lower_bounds,
$codebase
);
},
$template_map
);
Expand Down Expand Up @@ -563,8 +567,10 @@ public static function checkArgumentsMatch(

if ($class_template_result) {
foreach ($class_template_result->lower_bounds as $template_name => $type_map) {
foreach ($type_map as $class => $bound) {
$class_generic_params[$template_name][$class] = clone $bound->type;
foreach ($type_map as $class => $lower_bounds) {
if (count($lower_bounds) === 1) {
$class_generic_params[$template_name][$class] = clone reset($lower_bounds)->type;
}
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,8 @@ public static function analyze(
$statements_analyzer->node_data->setIfTrueAssertions(
$stmt,
array_map(
function (Assertion $assertion) use ($inferred_lower_bounds) : Assertion {
return $assertion->getUntemplatedCopy($inferred_lower_bounds ?: [], null);
function (Assertion $assertion) use ($inferred_lower_bounds, $codebase) : Assertion {
return $assertion->getUntemplatedCopy($inferred_lower_bounds ?: [], null, $codebase);
},
$function_call_info->function_storage->if_true_assertions
)
Expand All @@ -295,8 +295,8 @@ function (Assertion $assertion) use ($inferred_lower_bounds) : Assertion {
$statements_analyzer->node_data->setIfFalseAssertions(
$stmt,
array_map(
function (Assertion $assertion) use ($inferred_lower_bounds) : Assertion {
return $assertion->getUntemplatedCopy($inferred_lower_bounds ?: [], null);
function (Assertion $assertion) use ($inferred_lower_bounds, $codebase) : Assertion {
return $assertion->getUntemplatedCopy($inferred_lower_bounds ?: [], null, $codebase);
},
$function_call_info->function_storage->if_false_assertions
)
Expand Down
24 changes: 15 additions & 9 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/FunctionCallReturnTypeFetcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,21 +67,27 @@ public static function fetch(
if (!isset($template_result->lower_bounds[$template_name])) {
if ($template_name === 'TFunctionArgCount') {
$template_result->lower_bounds[$template_name] = [
'fn-' . $function_id => new TemplateBound(
Type::getInt(false, count($stmt->args))
)
'fn-' . $function_id => [
new TemplateBound(
Type::getInt(false, count($stmt->args))
)
]
];
} elseif ($template_name === 'TPhpMajorVersion') {
$template_result->lower_bounds[$template_name] = [
'fn-' . $function_id => new TemplateBound(
Type::getInt(false, $codebase->php_major_version)
)
'fn-' . $function_id => [
new TemplateBound(
Type::getInt(false, $codebase->php_major_version)
)
]
];
} else {
$template_result->lower_bounds[$template_name] = [
'fn-' . $function_id => new TemplateBound(
Type::getEmpty()
)
'fn-' . $function_id => [
new TemplateBound(
Type::getEmpty()
)
]
];
}
}
Expand Down
12 changes: 8 additions & 4 deletions .../Internal/Analyzer/Statements/Expression/Call/Method/ExistingAtomicMethodCallAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -322,11 +322,13 @@ public static function analyze(
array_map(
function (Assertion $assertion) use (
$class_template_params,
$lhs_var_id
$lhs_var_id,
$codebase
) : Assertion {
return $assertion->getUntemplatedCopy(
$class_template_params ?: [],
$lhs_var_id
$lhs_var_id,
$codebase
);
},
$method_storage->if_true_assertions
Expand All @@ -340,11 +342,13 @@ function (Assertion $assertion) use (
array_map(
function (Assertion $assertion) use (
$class_template_params,
$lhs_var_id
$lhs_var_id,
$codebase
) : Assertion {
return $assertion->getUntemplatedCopy(
$class_template_params ?: [],
$lhs_var_id
$lhs_var_id,
$codebase
);
},
$method_storage->if_false_assertions
Expand Down
20 changes: 13 additions & 7 deletions ...Psalm/Internal/Analyzer/Statements/Expression/Call/Method/MethodCallReturnTypeFetcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,19 +534,25 @@ private static function replaceTemplateTypes(
) {
if ($template_type->param_name === 'TFunctionArgCount') {
$template_result->lower_bounds[$template_type->param_name] = [
'fn-' . strtolower((string) $method_id) => new TemplateBound(
Type::getInt(false, $arg_count)
)
'fn-' . strtolower((string) $method_id) => [
new TemplateBound(
Type::getInt(false, $arg_count)
)
]
];
} elseif ($template_type->param_name === 'TPhpMajorVersion') {
$template_result->lower_bounds[$template_type->param_name] = [
'fn-' . strtolower((string) $method_id) => new TemplateBound(
Type::getInt(false, $codebase->php_major_version)
)
'fn-' . strtolower((string) $method_id) => [
new TemplateBound(
Type::getInt(false, $codebase->php_major_version)
)
]
];
} else {
$template_result->lower_bounds[$template_type->param_name] = [
($template_type->defining_class) => new TemplateBound(Type::getEmpty())
($template_type->defining_class) => [
new TemplateBound(Type::getEmpty())
]
];
}
}
Expand Down
24 changes: 15 additions & 9 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/NewAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
use Psalm\Internal\Analyzer\StatementsAnalyzer;
use Psalm\Internal\Codebase\TaintFlowGraph;
use Psalm\Internal\DataFlow\DataFlowNode;
use Psalm\Internal\Type\TemplateStandinTypeReplacer;
use Psalm\Issue\AbstractInstantiation;
use Psalm\Issue\DeprecatedClass;
use Psalm\Issue\ImpureMethodCall;
Expand Down Expand Up @@ -439,8 +440,8 @@ private static function analyzeNamedConstructor(
$statements_analyzer->node_data->setIfTrueAssertions(
$stmt,
\array_map(
function ($assertion) use ($generic_params) {
return $assertion->getUntemplatedCopy($generic_params, null);
function ($assertion) use ($generic_params, $codebase) {
return $assertion->getUntemplatedCopy($generic_params, null, $codebase);
},
$method_storage->if_true_assertions
)
Expand All @@ -451,8 +452,8 @@ function ($assertion) use ($generic_params) {
$statements_analyzer->node_data->setIfFalseAssertions(
$stmt,
\array_map(
function ($assertion) use ($generic_params) {
return $assertion->getUntemplatedCopy($generic_params, null);
function ($assertion) use ($generic_params, $codebase) {
return $assertion->getUntemplatedCopy($generic_params, null, $codebase);
},
$method_storage->if_false_assertions
)
Expand All @@ -465,18 +466,23 @@ function ($assertion) use ($generic_params) {
if ($storage->template_types) {
foreach ($storage->template_types as $template_name => $base_type) {
if (isset($template_result->lower_bounds[$template_name][$fq_class_name])) {
$generic_param_type
= $template_result->lower_bounds[$template_name][$fq_class_name]->type;
$generic_param_type = TemplateStandinTypeReplacer::getMostSpecificTypeFromBounds(
$template_result->lower_bounds[$template_name][$fq_class_name],
$codebase
);
} elseif ($storage->template_extended_params && $template_result->lower_bounds) {
$generic_param_type = self::getGenericParamForOffset(
$fq_class_name,
$template_name,
$storage->template_extended_params,
array_map(
function ($type_map) {
function ($type_map) use ($codebase) {
return array_map(
function ($bound) {
return $bound->type;
function ($bounds) use ($codebase) {
return TemplateStandinTypeReplacer::getMostSpecificTypeFromBounds(
$bounds,
$codebase
);
},
$type_map
);
Expand Down
28 changes: 17 additions & 11 deletions ...nal/Analyzer/Statements/Expression/Call/StaticMethod/ExistingAtomicStaticCallAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ public static function analyze(
$statements_analyzer->node_data->setIfTrueAssertions(
$stmt,
array_map(
function (Assertion $assertion) use ($generic_params) : Assertion {
return $assertion->getUntemplatedCopy($generic_params, null);
function (Assertion $assertion) use ($generic_params, $codebase) : Assertion {
return $assertion->getUntemplatedCopy($generic_params, null, $codebase);
},
$method_storage->if_true_assertions
)
Expand All @@ -330,8 +330,8 @@ function (Assertion $assertion) use ($generic_params) : Assertion {
$statements_analyzer->node_data->setIfFalseAssertions(
$stmt,
array_map(
function (Assertion $assertion) use ($generic_params) : Assertion {
return $assertion->getUntemplatedCopy($generic_params, null);
function (Assertion $assertion) use ($generic_params, $codebase) : Assertion {
return $assertion->getUntemplatedCopy($generic_params, null, $codebase);
},
$method_storage->if_false_assertions
)
Expand Down Expand Up @@ -488,19 +488,25 @@ private static function getMethodReturnType(
)) {
if ($template_type->param_name === 'TFunctionArgCount') {
$template_result->lower_bounds[$template_type->param_name] = [
'fn-' . strtolower((string)$method_id) => new TemplateBound(
Type::getInt(false, count($stmt->args))
)
'fn-' . strtolower((string)$method_id) => [
new TemplateBound(
Type::getInt(false, count($stmt->args))
)
]
];
} elseif ($template_type->param_name === 'TPhpMajorVersion') {
$template_result->lower_bounds[$template_type->param_name] = [
'fn-' . strtolower((string)$method_id) => new TemplateBound(
Type::getInt(false, $codebase->php_major_version)
)
'fn-' . strtolower((string)$method_id) => [
new TemplateBound(
Type::getInt(false, $codebase->php_major_version)
)
]
];
} else {
$template_result->lower_bounds[$template_type->param_name] = [
($template_type->defining_class) => new TemplateBound(Type::getEmpty())
($template_type->defining_class) => [
new TemplateBound(Type::getEmpty())
]
];
}
}
Expand Down
Loading

7 comments on commit acc7ee2

@azjezz
Copy link
Contributor

@azjezz azjezz commented on acc7ee2 Jul 12, 2021

Choose a reason for hiding this comment

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

This PR broke PSL build :)

@Ocramius
Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, similarly can no longer work with azjezz/psl since this commit: will attempt to pin to 4.8.x for now.

@azjezz
Copy link
Contributor

@azjezz azjezz commented on acc7ee2 Aug 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@Ocramius i will try to tag a new version of PSL this week ( already fixed in the 1.8.x branch )

@Ocramius
Copy link
Contributor

Choose a reason for hiding this comment

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

@azjezz do you think this is on azjezz/psl-side? 🤔

@azjezz
Copy link
Contributor

@azjezz azjezz commented on acc7ee2 Aug 24, 2021

Choose a reason for hiding this comment

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

yes, a fix has already been merged in 1.8, i will add it to 1.7 ( azjezz/psl@3d4b50f + azjezz/psl@46391a4 )

@Ocramius
Copy link
Contributor

Choose a reason for hiding this comment

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

Amazing! For other readers: https://opencollective.com/php-standard-library :D

@azjezz
Copy link
Contributor

@azjezz azjezz commented on acc7ee2 Oct 3, 2021

Choose a reason for hiding this comment

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

actually, the fix for constructing psl type objects also broke Psl\Type\TypeInterface::matches validation ( see #6573 ).

Please sign in to comment.