diff --git a/config.xsd b/config.xsd index c9a4cd88895..5a0dab58d68 100644 --- a/config.xsd +++ b/config.xsd @@ -462,6 +462,7 @@ + diff --git a/dictionaries/CallMap.php b/dictionaries/CallMap.php index df6b3456d8a..df1739c6609 100644 --- a/dictionaries/CallMap.php +++ b/dictionaries/CallMap.php @@ -11714,7 +11714,7 @@ 'rewind' => ['bool', 'stream'=>'resource'], 'rewinddir' => ['null|false', 'dir_handle='=>'resource'], 'rmdir' => ['bool', 'directory'=>'string', 'context='=>'resource'], -'round' => ['float', 'num'=>'float', 'precision='=>'int', 'mode='=>'int'], +'round' => ['float', 'num'=>'float', 'precision='=>'int', 'mode='=>'0|positive-int'], 'rpm_close' => ['bool', 'rpmr'=>'resource'], 'rpm_get_tag' => ['mixed', 'rpmr'=>'resource', 'tagnum'=>'int'], 'rpm_is_valid' => ['bool', 'filename'=>'string'], diff --git a/dictionaries/CallMap_historical.php b/dictionaries/CallMap_historical.php index 036c5c710c7..a8ad6acd48e 100644 --- a/dictionaries/CallMap_historical.php +++ b/dictionaries/CallMap_historical.php @@ -14751,7 +14751,7 @@ 'rewind' => ['bool', 'stream'=>'resource'], 'rewinddir' => ['null|false', 'dir_handle='=>'resource'], 'rmdir' => ['bool', 'directory'=>'string', 'context='=>'resource'], - 'round' => ['float', 'num'=>'float', 'precision='=>'int', 'mode='=>'int'], + 'round' => ['float', 'num'=>'float', 'precision='=>'int', 'mode='=>'0|positive-int'], 'rpm_close' => ['bool', 'rpmr'=>'resource'], 'rpm_get_tag' => ['mixed', 'rpmr'=>'resource', 'tagnum'=>'int'], 'rpm_is_valid' => ['bool', 'filename'=>'string'], diff --git a/docs/running_psalm/error_levels.md b/docs/running_psalm/error_levels.md index c58cca026ff..38e628bfd93 100644 --- a/docs/running_psalm/error_levels.md +++ b/docs/running_psalm/error_levels.md @@ -187,6 +187,7 @@ These issues are treated as errors at level 3 and below. - [PossiblyUndefinedMethod](issues/PossiblyUndefinedMethod.md) - [PossiblyUndefinedVariable](issues/PossiblyUndefinedVariable.md) - [PropertyTypeCoercion](issues/PropertyTypeCoercion.md) + - [RiskyCast](issues/RiskyCast.md) ## Errors ignored at level 5 and higher diff --git a/docs/running_psalm/issues.md b/docs/running_psalm/issues.md index 50385f60e31..20ddd0eca76 100644 --- a/docs/running_psalm/issues.md +++ b/docs/running_psalm/issues.md @@ -214,6 +214,7 @@ - [RedundantPropertyInitializationCheck](issues/RedundantPropertyInitializationCheck.md) - [ReferenceConstraintViolation](issues/ReferenceConstraintViolation.md) - [ReservedWord](issues/ReservedWord.md) + - [RiskyCast](issues/RiskyCast.md) - [StringIncrement](issues/StringIncrement.md) - [TaintedCallable](issues/TaintedCallable.md) - [TaintedCookie](issues/TaintedCookie.md) diff --git a/docs/running_psalm/issues/RiskyCast.md b/docs/running_psalm/issues/RiskyCast.md new file mode 100644 index 00000000000..ef90d6e37b8 --- /dev/null +++ b/docs/running_psalm/issues/RiskyCast.md @@ -0,0 +1,17 @@ +# RiskyCast + +Emitted when attempting to cast an array to int or float + +```php +node_data->getType($stmt->expr); if ($maybe_type) { if ($maybe_type->isInt()) { - $valid_int_type = $maybe_type; if (!$maybe_type->from_calculation) { self::handleRedundantCast($maybe_type, $statements_analyzer, $stmt); } } - if (count($maybe_type->getAtomicTypes()) === 1 - && $maybe_type->getSingleAtomic() instanceof TBool) { - $as_int = false; - $type = new Union([ - new TLiteralInt(0), - new TLiteralInt(1), - ]); - - if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph - ) { - $type->parent_nodes = $maybe_type->parent_nodes; - } - - $statements_analyzer->node_data->setType($stmt, $type); - } + $type = self::castIntAttempt( + $statements_analyzer, + $maybe_type, + $stmt->expr, + true + ); + } else { + $type = Type::getInt(); } - if ($as_int) { - $type = $valid_int_type ?? Type::getInt(); - - if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph - ) { - $type->parent_nodes = $maybe_type->parent_nodes ?? []; - } - - $statements_analyzer->node_data->setType($stmt, $type); - } + $statements_analyzer->node_data->setType($stmt, $type); return true; } @@ -116,13 +111,15 @@ public static function analyze( if ($maybe_type->isFloat()) { self::handleRedundantCast($maybe_type, $statements_analyzer, $stmt); } - } - $type = Type::getFloat(); - - if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph - ) { - $type->parent_nodes = $maybe_type->parent_nodes ?? []; + $type = self::castFloatAttempt( + $statements_analyzer, + $maybe_type, + $stmt->expr, + true + ); + } else { + $type = Type::getFloat(); } $statements_analyzer->node_data->setType($stmt, $type); @@ -300,6 +297,379 @@ public static function analyze( return false; } + public static function castIntAttempt( + StatementsAnalyzer $statements_analyzer, + Union $stmt_type, + PhpParser\Node\Expr $stmt, + bool $explicit_cast = false + ): Union { + $codebase = $statements_analyzer->getCodebase(); + + $risky_cast = []; + $invalid_casts = []; + $valid_ints = []; + $castable_types = []; + + $atomic_types = $stmt_type->getAtomicTypes(); + + $parent_nodes = []; + + if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) { + $parent_nodes = $stmt_type->parent_nodes; + } + + while ($atomic_types) { + $atomic_type = array_pop($atomic_types); + + if ($atomic_type instanceof TInt) { + $valid_ints[] = $atomic_type; + + continue; + } + + if ($atomic_type instanceof TFloat) { + if ($atomic_type instanceof TLiteralFloat) { + $valid_ints[] = new TLiteralInt((int) $atomic_type->value); + } else { + $castable_types[] = new TInt(); + } + + continue; + } + + if ($atomic_type instanceof TString) { + if ($atomic_type instanceof TLiteralString) { + $valid_ints[] = new TLiteralInt((int) $atomic_type->value); + } elseif ($atomic_type instanceof TNumericString) { + $castable_types[] = new TInt(); + } else { + // any normal string is technically $valid_int[] = new TLiteralInt(0); + // however we cannot be certain that it's not inferred, therefore less strict + $castable_types[] = new TInt(); + } + + continue; + } + + if ($atomic_type instanceof TNull || $atomic_type instanceof TFalse) { + $valid_ints[] = new TLiteralInt(0); + continue; + } + + if ($atomic_type instanceof TTrue) { + $valid_ints[] = new TLiteralInt(1); + continue; + } + + if ($atomic_type instanceof TBool) { + // do NOT use TIntRange here, as it will cause invalid behavior, e.g. bitwiseAssignment + $valid_ints[] = new TLiteralInt(0); + $valid_ints[] = new TLiteralInt(1); + continue; + } + + // could be invalid, but allow it, as it is allowed for TString below too + if ($atomic_type instanceof TMixed + || $atomic_type instanceof TClosedResource + || $atomic_type instanceof TResource + || $atomic_type instanceof Scalar + ) { + $castable_types[] = new TInt(); + + continue; + } + + if ($atomic_type instanceof TNamedObject) { + $intersection_types = [$atomic_type]; + + if ($atomic_type->extra_types) { + $intersection_types = array_merge($intersection_types, $atomic_type->extra_types); + } + + foreach ($intersection_types as $intersection_type) { + if (!$intersection_type instanceof TNamedObject) { + continue; + } + + // prevent "Could not get class storage for mixed" + if (!$codebase->classExists($intersection_type->value)) { + continue; + } + + foreach (self::PSEUDO_CASTABLE_CLASSES as $pseudo_castable_class) { + if (strtolower($intersection_type->value) === strtolower($pseudo_castable_class) + || $codebase->classExtends( + $intersection_type->value, + $pseudo_castable_class + ) + ) { + $castable_types[] = new TInt(); + continue 3; + } + } + } + } + + if ($atomic_type instanceof TNonEmptyArray + || $atomic_type instanceof TNonEmptyList + ) { + $risky_cast[] = $atomic_type->getId(); + + $valid_ints[] = new TLiteralInt(1); + + continue; + } + + if ($atomic_type instanceof TArray + || $atomic_type instanceof TList + || $atomic_type instanceof TKeyedArray + ) { + // if type is not specific, it can be both 0 or 1, depending on whether the array has data or not + // welcome to off-by-one hell if that happens :-) + $risky_cast[] = $atomic_type->getId(); + + $valid_ints[] = new TLiteralInt(0); + $valid_ints[] = new TLiteralInt(1); + + continue; + } + + if ($atomic_type instanceof TTemplateParam) { + $atomic_types = array_merge($atomic_types, $atomic_type->as->getAtomicTypes()); + + continue; + } + + // always 1 for "error" cases + $valid_ints[] = new TLiteralInt(1); + + $invalid_casts[] = $atomic_type->getId(); + } + + if ($invalid_casts) { + IssueBuffer::maybeAdd( + new InvalidCast( + $invalid_casts[0] . ' cannot be cast to int', + new CodeLocation($statements_analyzer->getSource(), $stmt) + ), + $statements_analyzer->getSuppressedIssues() + ); + } elseif ($risky_cast) { + IssueBuffer::maybeAdd( + new RiskyCast( + 'Casting ' . $risky_cast[0] . ' to int has possibly unintended value of 0/1', + new CodeLocation($statements_analyzer->getSource(), $stmt) + ), + $statements_analyzer->getSuppressedIssues() + ); + } elseif ($explicit_cast && !$castable_types) { + // todo: emit error here + } + + $valid_types = array_merge($valid_ints, $castable_types); + + if (!$valid_types) { + $int_type = Type::getInt(); + } else { + $int_type = TypeCombiner::combine( + $valid_types, + $codebase + ); + } + + if ($statements_analyzer->data_flow_graph) { + $int_type->parent_nodes = $parent_nodes; + } + + return $int_type; + } + + public static function castFloatAttempt( + StatementsAnalyzer $statements_analyzer, + Union $stmt_type, + PhpParser\Node\Expr $stmt, + bool $explicit_cast = false + ): Union { + $codebase = $statements_analyzer->getCodebase(); + + $risky_cast = []; + $invalid_casts = []; + $valid_floats = []; + $castable_types = []; + + $atomic_types = $stmt_type->getAtomicTypes(); + + $parent_nodes = []; + + if ($statements_analyzer->data_flow_graph instanceof VariableUseGraph) { + $parent_nodes = $stmt_type->parent_nodes; + } + + while ($atomic_types) { + $atomic_type = array_pop($atomic_types); + + if ($atomic_type instanceof TFloat) { + $valid_floats[] = $atomic_type; + + continue; + } + + if ($atomic_type instanceof TInt) { + if ($atomic_type instanceof TLiteralInt) { + $valid_floats[] = new TLiteralFloat((float) $atomic_type->value); + } else { + $castable_types[] = new TFloat(); + } + + continue; + } + + if ($atomic_type instanceof TString) { + if ($atomic_type instanceof TLiteralString) { + $valid_floats[] = new TLiteralFloat((float) $atomic_type->value); + } elseif ($atomic_type instanceof TNumericString) { + $castable_types[] = new TFloat(); + } else { + // any normal string is technically $valid_floats[] = new TLiteralFloat(0.0); + // however we cannot be certain that it's not inferred, therefore less strict + $castable_types[] = new TFloat(); + } + + continue; + } + + if ($atomic_type instanceof TNull || $atomic_type instanceof TFalse) { + $valid_floats[] = new TLiteralFloat(0.0); + continue; + } + + if ($atomic_type instanceof TTrue) { + $valid_floats[] = new TLiteralFloat(1.0); + continue; + } + + if ($atomic_type instanceof TBool) { + $valid_floats[] = new TLiteralFloat(0.0); + $valid_floats[] = new TLiteralFloat(1.0); + continue; + } + + // could be invalid, but allow it, as it is allowed for TString below too + if ($atomic_type instanceof TMixed + || $atomic_type instanceof TClosedResource + || $atomic_type instanceof TResource + || $atomic_type instanceof Scalar + ) { + $castable_types[] = new TFloat(); + + continue; + } + + if ($atomic_type instanceof TNamedObject) { + $intersection_types = [$atomic_type]; + + if ($atomic_type->extra_types) { + $intersection_types = array_merge($intersection_types, $atomic_type->extra_types); + } + + foreach ($intersection_types as $intersection_type) { + if (!$intersection_type instanceof TNamedObject) { + continue; + } + + // prevent "Could not get class storage for mixed" + if (!$codebase->classExists($intersection_type->value)) { + continue; + } + + foreach (self::PSEUDO_CASTABLE_CLASSES as $pseudo_castable_class) { + if (strtolower($intersection_type->value) === strtolower($pseudo_castable_class) + || $codebase->classExtends( + $intersection_type->value, + $pseudo_castable_class + ) + ) { + $castable_types[] = new TFloat(); + continue 3; + } + } + } + } + + if ($atomic_type instanceof TNonEmptyArray + || $atomic_type instanceof TNonEmptyList + ) { + $risky_cast[] = $atomic_type->getId(); + + $valid_floats[] = new TLiteralFloat(1.0); + + continue; + } + + if ($atomic_type instanceof TArray + || $atomic_type instanceof TList + || $atomic_type instanceof TKeyedArray + ) { + // if type is not specific, it can be both 0 or 1, depending on whether the array has data or not + // welcome to off-by-one hell if that happens :-) + $risky_cast[] = $atomic_type->getId(); + + $valid_floats[] = new TLiteralFloat(0.0); + $valid_floats[] = new TLiteralFloat(1.0); + + continue; + } + + if ($atomic_type instanceof TTemplateParam) { + $atomic_types = array_merge($atomic_types, $atomic_type->as->getAtomicTypes()); + + continue; + } + + // always 1.0 for "error" cases + $valid_floats[] = new TLiteralFloat(1.0); + + $invalid_casts[] = $atomic_type->getId(); + } + + if ($invalid_casts) { + IssueBuffer::maybeAdd( + new InvalidCast( + $invalid_casts[0] . ' cannot be cast to float', + new CodeLocation($statements_analyzer->getSource(), $stmt) + ), + $statements_analyzer->getSuppressedIssues() + ); + } elseif ($risky_cast) { + IssueBuffer::maybeAdd( + new RiskyCast( + 'Casting ' . $risky_cast[0] . ' to float has possibly unintended value of 0.0/1.0', + new CodeLocation($statements_analyzer->getSource(), $stmt) + ), + $statements_analyzer->getSuppressedIssues() + ); + } elseif ($explicit_cast && !$castable_types) { + // todo: emit error here + } + + $valid_types = array_merge($valid_floats, $castable_types); + + if (!$valid_types) { + $float_type = Type::getFloat(); + } else { + $float_type = TypeCombiner::combine( + $valid_types, + $codebase + ); + } + + if ($statements_analyzer->data_flow_graph) { + $float_type->parent_nodes = $parent_nodes; + } + + return $float_type; + } + public static function castStringAttempt( StatementsAnalyzer $statements_analyzer, Context $context, @@ -352,8 +722,28 @@ public static function castStringAttempt( continue; } + if ($atomic_type instanceof TTrue + ) { + $valid_strings[] = new TLiteralString('1'); + continue; + } + + if ($atomic_type instanceof TBool + ) { + $valid_strings[] = new TLiteralString('1'); + $valid_strings[] = new TLiteralString(''); + continue; + } + + if ($atomic_type instanceof TClosedResource + || $atomic_type instanceof TResource + ) { + $castable_types[] = new TNonEmptyString(); + + continue; + } + if ($atomic_type instanceof TMixed - || $atomic_type instanceof TResource || $atomic_type instanceof Scalar ) { $castable_types[] = new TString(); diff --git a/src/Psalm/Internal/Cli/LanguageServer.php b/src/Psalm/Internal/Cli/LanguageServer.php index 634cdee16a4..1a0144def8c 100644 --- a/src/Psalm/Internal/Cli/LanguageServer.php +++ b/src/Psalm/Internal/Cli/LanguageServer.php @@ -32,6 +32,7 @@ use function in_array; use function ini_set; use function is_array; +use function is_numeric; use function is_string; use function preg_replace; use function realpath; @@ -298,7 +299,7 @@ function () use ($current_dir, $options, $vendor_dir): ?\Composer\Autoload\Class $find_unused_code = 'auto'; } - if (isset($options['disable-on-change'])) { + if (isset($options['disable-on-change']) && is_numeric($options['disable-on-change'])) { $project_analyzer->onchange_line_limit = (int) $options['disable-on-change']; } diff --git a/src/Psalm/Internal/Cli/Psalter.php b/src/Psalm/Internal/Cli/Psalter.php index 7db204c5f61..12e025fce7e 100644 --- a/src/Psalm/Internal/Cli/Psalter.php +++ b/src/Psalm/Internal/Cli/Psalter.php @@ -45,6 +45,7 @@ use function ini_set; use function is_array; use function is_dir; +use function is_numeric; use function is_string; use function microtime; use function pathinfo; @@ -230,7 +231,7 @@ function () use ($current_dir, $options, $vendor_dir): ?\Composer\Autoload\Class chdir($current_dir); } - $threads = isset($options['threads']) ? (int)$options['threads'] : 1; + $threads = isset($options['threads']) && is_numeric($options['threads']) ? (int)$options['threads'] : 1; if (isset($options['no-cache'])) { $providers = new Providers( diff --git a/src/Psalm/Internal/Cli/Refactor.php b/src/Psalm/Internal/Cli/Refactor.php index 864e9d4aebd..a3b5a114b37 100644 --- a/src/Psalm/Internal/Cli/Refactor.php +++ b/src/Psalm/Internal/Cli/Refactor.php @@ -35,6 +35,7 @@ use function in_array; use function ini_set; use function is_array; +use function is_numeric; use function is_string; use function max; use function microtime; @@ -284,7 +285,7 @@ function () use ($current_dir, $options, $vendor_dir): ?\Composer\Autoload\Class chdir($current_dir); } - $threads = isset($options['threads']) + $threads = isset($options['threads']) && is_numeric($options['threads']) ? (int)$options['threads'] : max(1, ProjectAnalyzer::getCpuCount() - 2); diff --git a/src/Psalm/Internal/Provider/FunctionReturnTypeProvider.php b/src/Psalm/Internal/Provider/FunctionReturnTypeProvider.php index 8b124ea5532..e4636d5c3a0 100644 --- a/src/Psalm/Internal/Provider/FunctionReturnTypeProvider.php +++ b/src/Psalm/Internal/Provider/FunctionReturnTypeProvider.php @@ -35,6 +35,7 @@ use Psalm\Internal\Provider\ReturnTypeProvider\MktimeReturnTypeProvider; use Psalm\Internal\Provider\ReturnTypeProvider\ParseUrlReturnTypeProvider; use Psalm\Internal\Provider\ReturnTypeProvider\RandReturnTypeProvider; +use Psalm\Internal\Provider\ReturnTypeProvider\RoundReturnTypeProvider; use Psalm\Internal\Provider\ReturnTypeProvider\StrReplaceReturnTypeProvider; use Psalm\Internal\Provider\ReturnTypeProvider\StrTrReturnTypeProvider; use Psalm\Internal\Provider\ReturnTypeProvider\TriggerErrorReturnTypeProvider; @@ -109,6 +110,7 @@ public function __construct() $this->registerClass(TriggerErrorReturnTypeProvider::class); $this->registerClass(RandReturnTypeProvider::class); $this->registerClass(InArrayReturnTypeProvider::class); + $this->registerClass(RoundReturnTypeProvider::class); } /** diff --git a/src/Psalm/Internal/Provider/ReturnTypeProvider/RoundReturnTypeProvider.php b/src/Psalm/Internal/Provider/ReturnTypeProvider/RoundReturnTypeProvider.php new file mode 100644 index 00000000000..eb8ee561f54 --- /dev/null +++ b/src/Psalm/Internal/Provider/ReturnTypeProvider/RoundReturnTypeProvider.php @@ -0,0 +1,78 @@ + + */ + public static function getFunctionIds(): array + { + return ['round']; + } + + public static function getFunctionReturnType(FunctionReturnTypeProviderEvent $event): ?Type\Union + { + $call_args = $event->getCallArgs(); + if (count($call_args) === 0) { + return null; + } + + $statements_source = $event->getStatementsSource(); + $nodeTypeProvider = $statements_source->getNodeTypeProvider(); + + $num_arg = $nodeTypeProvider->getType($call_args[0]->value); + + $precision_val = 0; + if ($statements_source instanceof StatementsAnalyzer && count($call_args) > 1) { + $type = $statements_source->node_data->getType($call_args[1]->value); + + if ($type !== null && $type->isSingle()) { + $atomic_type = array_values($type->getAtomicTypes())[0]; + if ($atomic_type instanceof Type\Atomic\TLiteralInt) { + $precision_val = $atomic_type->value; + } + } + } + + $mode_val = PHP_ROUND_HALF_UP; + if ($statements_source instanceof StatementsAnalyzer && count($call_args) > 2) { + $type = $statements_source->node_data->getType($call_args[2]->value); + + if ($type !== null && $type->isSingle()) { + $atomic_type = array_values($type->getAtomicTypes())[0]; + if ($atomic_type instanceof Type\Atomic\TLiteralInt) { + /** @var positive-int|0 $mode_val */ + $mode_val = $atomic_type->value; + } + } + } + + if ($num_arg !== null && $num_arg->isSingle()) { + $num_type = array_values($num_arg->getAtomicTypes())[0]; + if ($num_type instanceof Type\Atomic\TLiteralFloat || $num_type instanceof Type\Atomic\TLiteralInt) { + $rounded_val = round($num_type->value, $precision_val, $mode_val); + return new Type\Union([new Type\Atomic\TLiteralFloat($rounded_val)]); + } + } + + return new Type\Union([new Type\Atomic\TFloat()]); + } +} diff --git a/src/Psalm/Issue/RiskyCast.php b/src/Psalm/Issue/RiskyCast.php new file mode 100644 index 00000000000..6576d430aeb --- /dev/null +++ b/src/Psalm/Issue/RiskyCast.php @@ -0,0 +1,9 @@ + [ ' 'lowercase-string', ], ], + 'round_literalValue' => [ + ' [ + '$a===' => 'float(10.36)', + ], + ], ]; } diff --git a/tests/ToStringTest.php b/tests/ToStringTest.php index 1a35aae8b73..f9101fa9cbb 100644 --- a/tests/ToStringTest.php +++ b/tests/ToStringTest.php @@ -505,6 +505,28 @@ public function __toString(): string ', 'error_message' => 'ImplicitToStringCast' ], + 'toStringTypecastNonString' => [ + ' 'InvalidCast', + ], + 'riskyArrayToIntCast' => [ + ' 'RiskyCast', + ], + 'riskyArrayToFloatCast' => [ + ' 'RiskyCast', + ], ]; } } diff --git a/tests/TypeReconciliation/ValueTest.php b/tests/TypeReconciliation/ValueTest.php index b5963ac4125..cda14767394 100644 --- a/tests/TypeReconciliation/ValueTest.php +++ b/tests/TypeReconciliation/ValueTest.php @@ -909,6 +909,14 @@ function foo(string $s) : void { $interval = \DateInterval::createFromDateString("30 дней"); if ($interval === false) {}', ], + 'literalInt' => [ + ' [ + '$a===' => '5', + ], + ], ]; }