Skip to content

Commit

Permalink
Fix a false flag issue with InvalidConstantAssignmentValue being thro…
Browse files Browse the repository at this point in the history 
…wn for constants over a certain length. Usually happens with arrays or lists over 100+ entries in length.

Check if this type was defined via a dockblock or type hint otherwise the inferred type should always match the assigned type, and we don't even need to do additional checks
There is an issue with constants over a certain length where additional values are added to fallback_params in the assigned_type but not in const_storage_type which causes a false flag for this error to appear. Usually happens with arrays/lists.

Added two separate tests to cover both lists, and arrays to ensure this issue is fixed.
  • Loading branch information
@MelechMizrachi
MelechMizrachi committed Feb 23, 2024
1 parent 2b64280 commit f553392
Show file tree
Hide file tree
Showing 2 changed files with 248 additions and 4 deletions.
16 changes: 12 additions & 4 deletions src/Psalm/Internal/Analyzer/Statements/Expression/ClassConstAnalyzer.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -721,19 +721,27 @@ public static function analyzeAssignment(

// Check assigned type matches docblock type
if ($assigned_type = $statements_analyzer->node_data->getType($const->value)) {
if ($const_storage->type !== null
$const_storage_type = $const_storage->type;

if ($const_storage_type !== null
&& $const_storage->stmt_location !== null
&& $assigned_type !== $const_storage->type
&& $assigned_type !== $const_storage_type
// Check if this type was defined via a dockblock or type hint otherwise the inferred type
// should always match the assigned type and we don't even need to do additional checks
// There is an issue with constants over a certain length where additional values
// are added to fallback_params in the assigned_type but not in const_storage_type
// which causes a false flag for this error to appear. Usually happens with arrays
&& ($const_storage_type->from_docblock || $const_storage_type->from_property)
&& !UnionTypeComparator::isContainedBy(
$statements_analyzer->getCodebase(),
$assigned_type,
$const_storage->type,
$const_storage_type,
)
) {
IssueBuffer::maybeAdd(
new InvalidConstantAssignmentValue(
"{$class_storage->name}::{$const->name->name} with declared type "
. "{$const_storage->type->getId()} cannot be assigned type {$assigned_type->getId()}",
. "{$const_storage_type->getId()} cannot be assigned type {$assigned_type->getId()}",
$const_storage->stmt_location,
"{$class_storage->name}::{$const->name->name}",
),
Expand Down
236 changes: 236 additions & 0 deletions tests/ConstantTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,242 @@ public static function foo(int $i) : void {}
A::foo(2);
A::foo(3);',
],
'tooLongArrayInvalidConstantAssignmentValueFalsePositiveWithArray' => [
'code' => '<?php
class TestInvalidConstantAssignmentValueFalsePositiveWithArray {
const LOOKUP = [
"00" => null,
"01" => null,
"02" => null,
"03" => null,
"04" => null,
"05" => null,
"06" => null,
"07" => null,
"08" => null,
"09" => null,
"10" => null,
"11" => null,
"12" => null,
"13" => null,
"14" => null,
"15" => null,
"16" => null,
"17" => null,
"18" => null,
"19" => null,
"20" => null,
"21" => null,
"22" => null,
"23" => null,
"24" => null,
"25" => null,
"26" => null,
"27" => null,
"28" => null,
"29" => null,
"30" => null,
"31" => null,
"32" => null,
"33" => null,
"34" => null,
"35" => null,
"36" => null,
"37" => null,
"38" => null,
"39" => null,
"40" => null,
"41" => null,
"42" => null,
"43" => null,
"44" => null,
"45" => null,
"46" => null,
"47" => null,
"48" => null,
"49" => null,
"50" => null,
"51" => null,
"52" => null,
"53" => null,
"54" => null,
"55" => null,
"56" => null,
"57" => null,
"58" => null,
"59" => null,
"60" => null,
"61" => null,
"62" => null,
"63" => null,
"64" => null,
"65" => null,
"66" => null,
"67" => null,
"68" => null,
"69" => null,
"70" => self::SUCCEED,
"71" => self::FAIL,
"72" => null,
"73" => null,
"74" => null,
"75" => null,
"76" => null,
"77" => null,
"78" => null,
"79" => null,
"80" => null,
"81" => null,
"82" => null,
"83" => null,
"84" => null,
"85" => null,
"86" => null,
"87" => null,
"88" => null,
"89" => null,
"90" => null,
"91" => null,
"92" => null,
"93" => null,
"94" => null,
"95" => null,
"96" => null,
"97" => null,
"98" => null,
"99" => null,
"100" => null,
"101" => null,
];
const SUCCEED = "SUCCEED";
const FAIL = "FAIL";
public static function will_succeed(string $code) : bool {
// Seems to fail when the array has over 100+ entries, and at least one value references
// another constant from the same class (even nested)
return (self::LOOKUP[$code] ?? null) === self::SUCCEED;
}
}',
],
'tooLongArrayInvalidConstantAssignmentValueFalsePositiveWithList' => [
'code' => '<?php
class TestInvalidConstantAssignmentValueFalsePositiveWithList {
const LOOKUP = [
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
self::SUCCEED,
self::FAIL,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
null,
];
const SUCCEED = "SUCCEED";
const FAIL = "FAIL";
public static function will_succeed(int $code) : bool {
// Seems to fail when the array has over 100+ entries, and at least one value references
// another constant from the same class (even nested)
return (self::LOOKUP[$code] ?? null) === self::SUCCEED;
}
}',
],
'valueOf' => [
'code' => '<?php
class A {
Expand Down

0 comments on commit f553392

Please sign in to comment.