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

Variadic params should always be optional and fix redundant optional variadic annotation #10777

Open
wants to merge 15 commits into
base: 5.x
Choose a base branch
from
Open

Variadic params should always be optional and fix redundant optional variadic annotation #10777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
b63fd97
variadic params should be handled as optional by default
kkmuffme Mar 3, 2024
f7d8a16
fix wrong variadic param types since PHP 8+
kkmuffme Mar 3, 2024
b788ee6
don't allow named arg calls for PHP < 8 and report error early before…
kkmuffme Mar 4, 2024
e2cda27
PHP internal functions with variadic do not support named argument ca…
kkmuffme Mar 4, 2024
c48c97a
fix wrong version used for string unpack in some places since it's on…
kkmuffme Mar 4, 2024
4c2f324
fix inconsistent TooMany/TooFew reporting for fallback params used co…
kkmuffme Mar 4, 2024
b47e90b
improve unpacked args
kkmuffme Mar 4, 2024
b1ddf8d
Fix unpacked array|int not reporting errors
kkmuffme Mar 12, 2024
f2e8d6a
report error for would-be fatal when using positional arg after unpack
kkmuffme Mar 12, 2024
89f895f
error for non-literal arrays possibly positional or duplicate named
kkmuffme Mar 12, 2024
d57ab4f
fix tests string unpack only from PHP 8.1 and similar PHP version res…
kkmuffme Mar 12, 2024
46dd8e4
code style
kkmuffme Mar 12, 2024
7e7d965
add php_version for UnusedCodeTest like it exists for other tests alr…
kkmuffme Mar 12, 2024
1a715f9
fix bug with class-string-map
kkmuffme Mar 12, 2024
03bde5e
fix shepherd reported errors
kkmuffme Mar 12, 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
2 changes: 1 addition & 1 deletion examples/TemplateChecker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function analyze(?Context $file_context = null, ?Context $global_context
throw new InvalidArgumentException('Could not interpret doc comment correctly');
}

/** @psalm-suppress ArgumentTypeCoercion */
/** @psalm-suppress ArgumentTypeCoercion, TooFewArguments */
$method_id = new MethodIdentifier(...explode('::', $matches[1]));

$this_params = $this->checkMethod($method_id, $first_stmt, $codebase);
Expand Down
2 changes: 1 addition & 1 deletion examples/plugins/FunctionCasingChecker.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public static function afterMethodCallAnalysis(AfterMethodCallAnalysisEvent $eve
}

try {
/** @psalm-suppress ArgumentTypeCoercion */
/** @psalm-suppress ArgumentTypeCoercion, TooFewArguments */
$method_id = new MethodIdentifier(...explode('::', $declaring_method_id));
$function_storage = $codebase->methods->getStorage($method_id);

Expand Down
12 changes: 6 additions & 6 deletions src/Psalm/Codebase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ public function getFunctionStorageForSymbol(string $file_path, string $symbol):
{
if (strpos($symbol, '::')) {
$symbol = substr($symbol, 0, -2);
/** @psalm-suppress ArgumentTypeCoercion */
/** @psalm-suppress ArgumentTypeCoercion, TooFewArguments */
$method_id = new MethodIdentifier(...explode('::', $symbol));

$declaring_method_id = $this->methods->getDeclaringMethodId($method_id);
Expand Down Expand Up @@ -1007,7 +1007,7 @@ public function getMarkupContentForSymbolByReference(
if (strpos($reference->symbol, '()')) {
$symbol = substr($reference->symbol, 0, -2);

/** @psalm-suppress ArgumentTypeCoercion */
/** @psalm-suppress ArgumentTypeCoercion, TooFewArguments */
$method_id = new MethodIdentifier(...explode('::', $symbol));

$declaring_method_id = $this->methods->getDeclaringMethodId(
Expand Down Expand Up @@ -1216,7 +1216,7 @@ public function getSymbolInformation(string $file_path, string $symbol): ?array
if (strpos($symbol, '()')) {
$symbol = substr($symbol, 0, -2);

/** @psalm-suppress ArgumentTypeCoercion */
/** @psalm-suppress ArgumentTypeCoercion, TooFewArguments */
$method_id = new MethodIdentifier(...explode('::', $symbol));

$declaring_method_id = $this->methods->getDeclaringMethodId($method_id);
Expand Down Expand Up @@ -1359,7 +1359,7 @@ public function getSymbolLocation(string $file_path, string $symbol): ?CodeLocat
if (strpos($symbol, '()')) {
$symbol = substr($symbol, 0, -2);

/** @psalm-suppress ArgumentTypeCoercion */
/** @psalm-suppress ArgumentTypeCoercion, TooFewArguments */
$method_id = new MethodIdentifier(...explode('::', $symbol));

$declaring_method_id = $this->methods->getDeclaringMethodId($method_id);
Expand Down Expand Up @@ -1445,7 +1445,7 @@ public function getSymbolLocationByReference(Reference $reference): ?CodeLocatio
if (strpos($reference->symbol, '()')) {
$symbol = substr($reference->symbol, 0, -2);

/** @psalm-suppress ArgumentTypeCoercion */
/** @psalm-suppress ArgumentTypeCoercion, TooFewArguments */
$method_id = new MethodIdentifier(
...explode('::', $symbol),
);
Expand Down Expand Up @@ -1656,7 +1656,7 @@ public function getSignatureInformation(
$signature_label = '';
$signature_documentation = null;
if (strpos($function_symbol, '::') !== false) {
/** @psalm-suppress ArgumentTypeCoercion */
/** @psalm-suppress ArgumentTypeCoercion, TooFewArguments */
$method_id = new MethodIdentifier(...explode('::', $function_symbol));

$declaring_method_id = $this->methods->getDeclaringMethodId($method_id);
Expand Down
1 change: 1 addition & 0 deletions src/Psalm/Config/Creator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ public static function getLevel(array $issues, int $counted_types): int
return array_keys($issues_at_level)[0] + 1;
}

/** @psalm-suppress TooFewArguments */
return max(...array_keys($issues_at_level)) + 1;
}

Expand Down
2 changes: 1 addition & 1 deletion src/Psalm/Internal/Analyzer/FunctionLikeAnalyzer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1147,7 +1147,7 @@ private function processParams(
$var_type = $param_type;

if ($function_param->is_variadic) {
if ($storage->allow_named_arg_calls) {
if ($codebase->analysis_php_version_id >= 8_00_00 && $storage->allow_named_arg_calls) {
$var_type = new Union([
new TArray([Type::getArrayKey(), $param_type]),
], [
Expand Down
210 changes: 106 additions & 104 deletions src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,24 @@ private static function checkFunctionLikeTypeMatches(
bool $specialize_taint,
bool $in_call_map
): ?bool {
if ($codebase->analysis_php_version_id < 8_00_00 || !$codebase->config->allow_named_arg_calls) {
$allow_named_args = false;
} elseif ($function_param->is_variadic && $in_call_map === true) {
$allow_named_args = false;
}

// before we possibly return early
if (!$allow_named_args && $arg->name !== null) {
IssueBuffer::maybeAdd(
new NamedArgumentNotAllowed(
'Method ' . $cased_method_id. ' called with named argument ' . $arg->name->name,
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}

if (!$function_param->type) {
if (!$codebase->infer_types_from_usage && !$statements_analyzer->data_flow_graph) {
return null;
Expand Down Expand Up @@ -471,154 +489,138 @@ private static function checkFunctionLikeTypeMatches(
return null;
}

$arg_value_type_from_array = null;
if ($arg_value_type->hasArray()) {
$unpacked_atomic_array = $arg_value_type->getArray();
$arg_key_allowed = true;
}

if ($unpacked_atomic_array instanceof TKeyedArray) {
if (!$allow_named_args && !$unpacked_atomic_array->getGenericKeyType()->isInt()) {
$arg_key_allowed = false;
}
$is_array = $arg_value_type->isArray();

$non_iterable = false;
$invalid_key = false;
$invalid_string_key = false;
$possibly_matches = false;
foreach ($arg_value_type->getAtomicTypes() as $atomic_type) {
if (!$atomic_type->isIterable($codebase) && !$atomic_type instanceof TClassStringMap) {
$non_iterable = true;
continue;
}

if ($atomic_type instanceof TKeyedArray) {
$key_type = $atomic_type->getGenericKeyType();
if ($function_param->is_variadic) {
$arg_value_type = $unpacked_atomic_array->getGenericValueType();
} elseif ($codebase->analysis_php_version_id >= 8_00_00
&& $allow_named_args
&& isset($unpacked_atomic_array->properties[$function_param->name])
$arg_value_type_from_array = $atomic_type->getGenericValueType();
} elseif ($codebase->analysis_php_version_id >= 8_01_00
&& $allow_named_args
&& isset($atomic_type->properties[$function_param->name])
) {
$arg_value_type = $unpacked_atomic_array->properties[$function_param->name];
} elseif ($unpacked_atomic_array->is_list
&& isset($unpacked_atomic_array->properties[$unpacked_argument_offset])
$arg_value_type_from_array = $atomic_type->properties[$function_param->name];
} elseif ($atomic_type->is_list
&& isset($atomic_type->properties[$unpacked_argument_offset])
) {
$arg_value_type = $unpacked_atomic_array->properties[$unpacked_argument_offset];
} elseif ($unpacked_atomic_array->fallback_params) {
$arg_value_type = $unpacked_atomic_array->fallback_params[1];
$arg_value_type_from_array = $atomic_type->properties[$unpacked_argument_offset];
} elseif ($atomic_type->fallback_params) {
$arg_value_type_from_array = $atomic_type->fallback_params[1];
} elseif ($function_param->is_optional && $function_param->default_type) {
if ($function_param->default_type instanceof Union) {
$arg_value_type = $function_param->default_type;
$arg_value_type_from_array = $function_param->default_type;
} else {
$arg_value_type_atomic = ConstantTypeResolver::resolve(
$codebase->classlikes,
$function_param->default_type,
$statements_analyzer,
);

$arg_value_type = new Union([$arg_value_type_atomic]);
$arg_value_type_from_array = new Union([$arg_value_type_atomic]);
}
} else {
$arg_value_type = Type::getMixed();
$arg_value_type_from_array = Type::getMixed();
}
} elseif ($unpacked_atomic_array instanceof TClassStringMap) {
$arg_value_type = Type::getMixed();
} elseif ($atomic_type instanceof TClassStringMap) {
$key_type = Type::getNonFalsyString();
$arg_value_type_from_array = $atomic_type->value_param;
} elseif ($atomic_type instanceof TArray) {
$key_type = $atomic_type->type_params[0];
$arg_value_type_from_array = $atomic_type->type_params[1];
} else {
if (!$allow_named_args && !$unpacked_atomic_array->type_params[0]->isInt()) {
$arg_key_allowed = false;
}
$arg_value_type = $unpacked_atomic_array->type_params[1];
$key_type = $codebase->getKeyValueParamsForTraversableObject($atomic_type)[0];
}

if (!$arg_key_allowed) {
IssueBuffer::maybeAdd(
new NamedArgumentNotAllowed(
'Method ' . $cased_method_id
. ' called with named unpacked array ' . $unpacked_atomic_array->getId()
. ' (array with string keys)',
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
if (!UnionTypeComparator::isContainedBy(
$codebase,
$key_type,
Type::getArrayKey(),
)) {
$invalid_key = true;

continue;
}
} else {
$non_iterable = false;
$invalid_key = false;
$invalid_string_key = false;
$possibly_matches = false;
foreach ($arg_value_type->getAtomicTypes() as $atomic_type) {
if (!$atomic_type->isIterable($codebase)) {
$non_iterable = true;
} else {
$key_type = $codebase->getKeyValueParamsForTraversableObject($atomic_type)[0];
if (!UnionTypeComparator::isContainedBy(
$codebase,
$key_type,
Type::getArrayKey(),
)) {
$invalid_key = true;

continue;
}
if (($codebase->analysis_php_version_id < 8_00_00 || !$allow_named_args)
&& !$key_type->isInt()
) {
$invalid_string_key = true;
// string unpacking is supported from PHP 8.1
if (($codebase->analysis_php_version_id < 8_01_00 || !$allow_named_args)
&& !$key_type->isInt()
) {
$invalid_string_key = true;

continue;
}
$possibly_matches = true;
}
continue;
}
$possibly_matches = true;
}

$issue_type = $possibly_matches ? PossiblyInvalidArgument::class : InvalidArgument::class;
if ($non_iterable) {
$issue_type = $possibly_matches ? PossiblyInvalidArgument::class : InvalidArgument::class;
if ($non_iterable) {
IssueBuffer::maybeAdd(
new $issue_type(
'Tried to unpack non-iterable ' . $arg_value_type->getId(),
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
if ($invalid_key) {
IssueBuffer::maybeAdd(
new $issue_type(
'Method ' . $cased_method_id
. ' called with unpacked iterable ' . $arg_value_type->getId()
. ' with invalid key (must be '
. ($codebase->analysis_php_version_id < 8_01_00 ? 'int' : 'int|string') . ')',
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
if ($invalid_string_key) {
if ($codebase->analysis_php_version_id < 8_01_00) {
IssueBuffer::maybeAdd(
new $issue_type(
'Tried to unpack non-iterable ' . $arg_value_type->getId(),
'String keys not supported in unpacked arguments',
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
if ($invalid_key) {
} else {
IssueBuffer::maybeAdd(
new $issue_type(
new NamedArgumentNotAllowed(
'Method ' . $cased_method_id
. ' called with unpacked iterable ' . $arg_value_type->getId()
. ' with invalid key (must be '
. ($codebase->analysis_php_version_id < 8_00_00 ? 'int' : 'int|string') . ')',
. ' called with named unpacked iterable ' . $arg_value_type->getId()
. ' (iterable with string keys)',
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
if ($invalid_string_key) {
if ($codebase->analysis_php_version_id < 8_00_00) {
IssueBuffer::maybeAdd(
new $issue_type(
'String keys not supported in unpacked arguments',
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
} else {
IssueBuffer::maybeAdd(
new NamedArgumentNotAllowed(
'Method ' . $cased_method_id
. ' called with named unpacked iterable ' . $arg_value_type->getId()
. ' (iterable with string keys)',
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);
}
}
}

if (!$is_array) {
return null;
}
} else {
if (!$allow_named_args && $arg->name !== null) {
IssueBuffer::maybeAdd(
new NamedArgumentNotAllowed(
'Method ' . $cased_method_id. ' called with named argument ' . $arg->name->name,
new CodeLocation($statements_analyzer->getSource(), $arg->value),
$cased_method_id,
),
$statements_analyzer->getSuppressedIssues(),
);

if ($arg_value_type_from_array) {
$arg_value_type = $arg_value_type_from_array;
}
}

Expand Down Expand Up @@ -828,7 +830,7 @@ public static function verifyType(
$codebase->analyzer->incrementNonMixedCount($statements_analyzer->getFilePath());
}

if ($function_param->by_ref || $function_param->is_optional) {
if ($function_param->by_ref || ($function_param->is_optional && !$function_param->is_variadic)) {
//if the param is optional or a ref, we'll allow the input to be possibly_undefined
$param_type = $param_type->setPossiblyUndefined(true);
}
Expand Down
Loading
Loading