Skip to content

Commit

Permalink
Merge pull request #10370 from kkmuffme/fix-misc-callable-bugs
Browse files Browse the repository at this point in the history 
Fix misc callable bugs
  • Loading branch information
@orklah
orklah authored Nov 22, 2023
2 parents 07acefd + 00bed51 commit 579cc08
Show file tree
Hide file tree
Showing 35 changed files with 1,332 additions and 222 deletions.
2 changes: 1 addition & 1 deletion dictionaries/CallMap.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@
'array_diff_ukey\'1' => ['array', 'array'=>'array', 'rest'=>'array', 'arr3'=>'array', 'arg4'=>'array|callable(mixed,mixed):int', '...rest='=>'array|callable(mixed,mixed):int'],
'array_fill' => ['array<int,mixed>', 'start_index'=>'int', 'count'=>'int', 'value'=>'mixed'],
'array_fill_keys' => ['array', 'keys'=>'array', 'value'=>'mixed'],
'array_filter' => ['array', 'array'=>'array', 'callback='=>'callable(mixed,mixed=):scalar|null', 'mode='=>'int'],
'array_filter' => ['array', 'array'=>'array', 'callback='=>'callable(mixed,array-key=):mixed|null', 'mode='=>'int'],
'array_flip' => ['array<string|int>', 'array'=>'array<string|int>'],
'array_intersect' => ['array', 'array'=>'array', '...arrays='=>'array'],
'array_intersect_assoc' => ['array', 'array'=>'array', '...arrays='=>'array'],
Expand Down
4 changes: 2 additions & 2 deletions dictionaries/CallMap_80_delta.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -553,8 +553,8 @@
'new' => ['array', 'array'=>'array', '...arrays='=>'array'],
],
'array_filter' => [
'old' => ['array', 'array'=>'array', 'callback='=>'callable(mixed,mixed=):scalar', 'mode='=>'int'],
'new' => ['array', 'array'=>'array', 'callback='=>'callable(mixed,mixed=):scalar|null', 'mode='=>'int'],
'old' => ['array', 'array'=>'array', 'callback='=>'callable(mixed,array-key=):mixed', 'mode='=>'int'],
'new' => ['array', 'array'=>'array', 'callback='=>'callable(mixed,array-key=):mixed|null', 'mode='=>'int'],
],
'array_key_exists' => [
'old' => ['bool', 'key'=>'string|int', 'array'=>'array|object'],
Expand Down
2 changes: 1 addition & 1 deletion dictionaries/CallMap_historical.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9288,7 +9288,7 @@
'array_diff_ukey\'1' => ['array', 'array'=>'array', 'rest'=>'array', 'arr3'=>'array', 'arg4'=>'array|callable(mixed,mixed):int', '...rest='=>'array|callable(mixed,mixed):int'],
'array_fill' => ['array<int,mixed>', 'start_index'=>'int', 'count'=>'int', 'value'=>'mixed'],
'array_fill_keys' => ['array', 'keys'=>'array', 'value'=>'mixed'],
'array_filter' => ['array', 'array'=>'array', 'callback='=>'callable(mixed,mixed=):scalar', 'mode='=>'int'],
'array_filter' => ['array', 'array'=>'array', 'callback='=>'callable(mixed,array-key=):mixed', 'mode='=>'int'],
'array_flip' => ['array<string|int>', 'array'=>'array<string|int>'],
'array_intersect' => ['array', 'array'=>'array', '...arrays'=>'array'],
'array_intersect_assoc' => ['array', 'array'=>'array', '...arrays'=>'array'],
Expand Down
61 changes: 61 additions & 0 deletions src/Psalm/Config.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Psalm\Internal\Analyzer\ClassLikeAnalyzer;
use Psalm\Internal\Analyzer\FileAnalyzer;
use Psalm\Internal\Analyzer\ProjectAnalyzer;
use Psalm\Internal\CliUtils;
use Psalm\Internal\Composer;
use Psalm\Internal\EventDispatcher;
use Psalm\Internal\IncludeCollector;
Expand Down Expand Up @@ -1298,6 +1299,66 @@ private static function fromXmlAndPaths(
$config->project_files = ProjectFileFilter::loadFromXMLElement($config_xml->projectFiles, $base_dir, true);
}

// any paths passed via CLI should be added to the projectFiles
// as they're getting analyzed like if they are part of the project
// ProjectAnalyzer::getInstance()->check_paths_files is not populated at this point in time
$paths_to_check = CliUtils::getPathsToCheck(null);
if ($paths_to_check !== null) {
$paths_to_add_to_project_files = array();
foreach ($paths_to_check as $path) {
// if we have an .xml arg here, the files passed are invalid
// valid cases (in which we don't want to add CLI passed files to projectFiles though)
// are e.g. if running phpunit tests for psalm itself
if (substr($path, -4) === '.xml') {
$paths_to_add_to_project_files = array();
break;
}

// we need an absolute path for checks
if ($path[0] !== '/' && DIRECTORY_SEPARATOR === '/') {
$prospective_path = $base_dir . DIRECTORY_SEPARATOR . $path;
} else {
$prospective_path = $path;
}

// will report an error when config is loaded anyway
if (!file_exists($prospective_path)) {
continue;
}

if ($config->isInProjectDirs($prospective_path)) {
continue;
}

$paths_to_add_to_project_files[] = $prospective_path;
}

if ($paths_to_add_to_project_files !== array() && !isset($config_xml->projectFiles)) {
if ($config_xml === null) {
$config_xml = new SimpleXMLElement('<psalm/>');
}
$config_xml->addChild('projectFiles');
}

if ($paths_to_add_to_project_files !== array() && isset($config_xml->projectFiles)) {
foreach ($paths_to_add_to_project_files as $path) {
if (is_dir($path)) {
$child = $config_xml->projectFiles->addChild('directory');
} else {
$child = $config_xml->projectFiles->addChild('file');
}

$child->addAttribute('name', $path);
}

$config->project_files = ProjectFileFilter::loadFromXMLElement(
$config_xml->projectFiles,
$base_dir,
true,
);
}
}

if (isset($config_xml->extraFiles)) {
$config->extra_files = ProjectFileFilter::loadFromXMLElement($config_xml->extraFiles, $base_dir, true);
}
Expand Down
111 changes: 83 additions & 28 deletions src/Psalm/Internal/Analyzer/MethodComparator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use Psalm\CodeLocation;
use Psalm\Codebase;
use Psalm\Config;
use Psalm\Internal\Codebase\InternalCallMapHandler;
use Psalm\Internal\FileManipulation\FileManipulationBuffer;
use Psalm\Internal\MethodIdentifier;
use Psalm\Internal\PhpVisitor\ParamReplacementVisitor;
Expand Down Expand Up @@ -37,6 +38,7 @@
use Psalm\Type\Union;

use function array_filter;
use function count;
use function in_array;
use function strpos;
use function strtolower;
Expand Down Expand Up @@ -116,21 +118,21 @@ public static function compare(
);
}

// CallMapHandler needed due to https://github.com/vimeo/psalm/issues/10378
if (!$guide_classlike_storage->user_defined
&& $implementer_classlike_storage->user_defined
&& $codebase->analysis_php_version_id >= 8_01_00
&& ($guide_method_storage->return_type
&& (($guide_method_storage->return_type && InternalCallMapHandler::inCallMap($cased_guide_method_id))
|| $guide_method_storage->signature_return_type
)
&& !$implementer_method_storage->signature_return_type
) && !$implementer_method_storage->signature_return_type
&& !array_filter(
$implementer_method_storage->attributes,
static fn(AttributeStorage $s): bool => $s->fq_class_name === 'ReturnTypeWillChange',
)
) {
IssueBuffer::maybeAdd(
new MethodSignatureMustProvideReturnType(
'Method ' . $cased_implementer_method_id . ' must have a return type signature!',
'Method ' . $cased_implementer_method_id . ' must have a return type signature',
$implementer_method_storage->location ?: $code_location,
),
$suppressed_issues + $implementer_classlike_storage->suppressed_issues,
Expand Down Expand Up @@ -199,10 +201,9 @@ public static function compare(
);
}

if ($guide_classlike_storage->user_defined
&& ($guide_classlike_storage->is_interface
|| $guide_classlike_storage->preserve_constructor_signature
|| $implementer_method_storage->cased_name !== '__construct')
if (($guide_classlike_storage->is_interface
|| $guide_classlike_storage->preserve_constructor_signature
|| $implementer_method_storage->cased_name !== '__construct')
&& $implementer_method_storage->required_param_count > $guide_method_storage->required_param_count
) {
if ($implementer_method_storage->cased_name !== '__construct') {
Expand Down Expand Up @@ -361,10 +362,20 @@ private static function compareMethodParams(
CodeLocation $code_location,
array $suppressed_issues
): void {
// ignore errors from stubbed/out of project files
$config = Config::getInstance();
if (!$implementer_classlike_storage->user_defined
&& (!$implementer_param->location
|| !$config->isInProjectDirs(
$implementer_param->location->file_path,
)
)) {
return;
}

if ($prevent_method_signature_mismatch) {
if (!$guide_classlike_storage->user_defined
&& $guide_param->type
) {
&& $guide_param->type) {
$implementer_param_type = $implementer_param->signature_type;

$guide_param_signature_type = $guide_param->type;
Expand All @@ -386,8 +397,6 @@ private static function compareMethodParams(
&& !$guide_param->type->from_docblock
&& ($implementer_param_type || $guide_param_signature_type)
) {
$config = Config::getInstance();

if ($implementer_param_type
&& (!$guide_param_signature_type
|| strtolower($implementer_param_type->getId())
Expand Down Expand Up @@ -438,11 +447,8 @@ private static function compareMethodParams(
}
}

$config = Config::getInstance();

if ($guide_param->name !== $implementer_param->name
&& $guide_method_storage->allow_named_arg_calls
&& $guide_classlike_storage->user_defined
&& $implementer_classlike_storage->user_defined
&& $implementer_param->location
&& $guide_method_storage->cased_name
Expand All @@ -451,7 +457,10 @@ private static function compareMethodParams(
$implementer_param->location->file_path,
)
) {
if ($config->allow_named_arg_calls
if (!$guide_classlike_storage->user_defined && $i === 0 && count($guide_method_storage->params) < 2) {
// if it's third party defined and a single arg, renaming is unnecessary
// if we still want to psalter it, move this if and change the else below to elseif
} elseif ($config->allow_named_arg_calls
|| ($guide_classlike_storage->location
&& !$config->isInProjectDirs($guide_classlike_storage->location->file_path)
)
Expand Down Expand Up @@ -491,9 +500,7 @@ private static function compareMethodParams(
}
}

if ($guide_classlike_storage->user_defined
&& $implementer_param->signature_type
) {
if ($implementer_param->signature_type) {
self::compareMethodSignatureParams(
$codebase,
$i,
Expand Down Expand Up @@ -532,9 +539,7 @@ private static function compareMethodParams(
);
}

if ($guide_classlike_storage->user_defined && $implementer_param->by_ref !== $guide_param->by_ref) {
$config = Config::getInstance();

if ($implementer_param->by_ref !== $guide_param->by_ref) {
IssueBuffer::maybeAdd(
new MethodSignatureMismatch(
'Argument ' . ($i + 1) . ' of ' . $cased_implementer_method_id . ' is' .
Expand Down Expand Up @@ -585,6 +590,50 @@ private static function compareMethodSignatureParams(
)
: null;

// CallMapHandler needed due to https://github.com/vimeo/psalm/issues/10378
if (!$guide_param->signature_type
&& $guide_param->type
&& InternalCallMapHandler::inCallMap($cased_guide_method_id)) {
$guide_method_storage_param_type = TypeExpander::expandUnion(
$codebase,
$guide_param->type,
$guide_classlike_storage->is_trait && $guide_method_storage->abstract
? $implementer_classlike_storage->name
: $guide_classlike_storage->name,
$guide_classlike_storage->is_trait && $guide_method_storage->abstract
? $implementer_classlike_storage->name
: $guide_classlike_storage->name,
$guide_classlike_storage->is_trait && $guide_method_storage->abstract
? $implementer_classlike_storage->parent_class
: $guide_classlike_storage->parent_class,
);

$builder = $guide_method_storage_param_type->getBuilder();
foreach ($builder->getAtomicTypes() as $k => $t) {
if ($t instanceof TTemplateParam) {
$builder->removeType($k);

foreach ($t->as->getAtomicTypes() as $as_t) {
$builder->addType($as_t);
}
}
}

if ($builder->hasMixed()) {
foreach ($builder->getAtomicTypes() as $k => $_) {
if ($k !== 'mixed') {
$builder->removeType($k);
}
}
}
$guide_method_storage_param_type = $builder->freeze();
unset($builder);

if (!$guide_method_storage_param_type->hasMixed() || $codebase->analysis_php_version_id >= 8_00_00) {
$guide_param_signature_type = $guide_method_storage_param_type;
}
}

$implementer_param_signature_type = TypeExpander::expandUnion(
$codebase,
$implementer_param_signature_type,
Expand Down Expand Up @@ -896,12 +945,18 @@ private static function compareMethodSignatureReturnTypes(
: UnionTypeComparator::isContainedByInPhp($implementer_signature_return_type, $guide_signature_return_type);

if (!$is_contained_by) {
if ($codebase->analysis_php_version_id >= 8_00_00
|| $guide_classlike_storage->is_trait === $implementer_classlike_storage->is_trait
|| !in_array($guide_classlike_storage->name, $implementer_classlike_storage->used_traits)
|| $implementer_method_storage->defining_fqcln !== $implementer_classlike_storage->name
|| (!$implementer_method_storage->abstract
&& !$guide_method_storage->abstract)
if ($implementer_signature_return_type === null
&& array_filter(
$implementer_method_storage->attributes,
static fn(AttributeStorage $s): bool => $s->fq_class_name === 'ReturnTypeWillChange',
)) {
// no error if return type will change and no signature set at all
} elseif ($codebase->analysis_php_version_id >= 8_00_00
|| $guide_classlike_storage->is_trait === $implementer_classlike_storage->is_trait
|| !in_array($guide_classlike_storage->name, $implementer_classlike_storage->used_traits)
|| $implementer_method_storage->defining_fqcln !== $implementer_classlike_storage->name
|| (!$implementer_method_storage->abstract
&& !$guide_method_storage->abstract)
) {
IssueBuffer::maybeAdd(
new MethodSignatureMismatch(
Expand Down
Loading

0 comments on commit 579cc08

Please sign in to comment.