add RiskyCast

kkmuffme · Sep 15, 2022 · 0ca91f8 · 0ca91f8
1 parent 810396d
commit 0ca91f8
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 12 deletions.
diff --git a/config.xsd b/config.xsd
@@ -462,6 +462,7 @@
             <xs:element name="RedundantIdentityWithTrue" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="ReferenceConstraintViolation" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="ReservedWord" type="IssueHandlerType" minOccurs="0" />
+            <xs:element name="RiskyCast" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="StringIncrement" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedCallable" type="IssueHandlerType" minOccurs="0" />
             <xs:element name="TaintedCookie" type="IssueHandlerType" minOccurs="0" />

diff --git 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
@@ -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
@@ -0,0 +1,17 @@
+# RiskyCast
+
+Emitted when attempting to cast an array to int or float
+
+```php
+<?php
+
+$foo = (int) array( 'hello' );
+```
+
+## Why this is bad
+
+The value resulting from the cast depends on if the array is empty or not and can easily lead to off-by-one errors
+
+## How to fix
+
+Don't cast arrays to int or float.
diff --git a/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php b/src/Psalm/Internal/Analyzer/Statements/Expression/CastAnalyzer.php
@@ -17,6 +17,7 @@
 use Psalm\Issue\PossiblyInvalidCast;
 use Psalm\Issue\RedundantCast;
 use Psalm\Issue\RedundantCastGivenDocblockType;
+use Psalm\Issue\RiskyCast;
 use Psalm\Issue\UnrecognizedExpression;
 use Psalm\IssueBuffer;
 use Psalm\Type;
@@ -299,7 +300,7 @@ public static function castIntAttempt(
     ): Union {
         $codebase = $statements_analyzer->getCodebase();
 
-        $possibly_unwanted_cast = [];
+        $risky_cast = [];
         $invalid_casts = [];
         $valid_ints = [];
         $castable_types = [];
@@ -444,7 +445,7 @@ public static function castIntAttempt(
             if ($atomic_type instanceof TNonEmptyArray
                 || $atomic_type instanceof TNonEmptyList
             ) {
-                $possibly_unwanted_cast[] = $atomic_type->getId();
+                $risky_cast[] = $atomic_type->getId();
 
                 $valid_ints[] = new TLiteralInt(1);
 
@@ -457,7 +458,7 @@ public static function castIntAttempt(
             ) {
                 // 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 :-)
-                $possibly_unwanted_cast[] = $atomic_type->getId();
+                $risky_cast[] = $atomic_type->getId();
 
                 $valid_ints[] = new TLiteralInt(0);
                 $valid_ints[] = new TLiteralInt(1);
@@ -485,10 +486,10 @@ public static function castIntAttempt(
                 ),
                 $statements_analyzer->getSuppressedIssues()
             );
-        } elseif (!empty($possibly_unwanted_cast)) {
+        } elseif (!empty($risky_cast)) {
             IssueBuffer::maybeAdd(
-                new PossiblyInvalidCast(
-                    'Casting ' . $possibly_unwanted_cast[0] . ' to int has possibly unintended value of 1',
+                new RiskyCast(
+                    'Casting ' . $risky_cast[0] . ' to int has possibly unintended value of 0/1',
                     new CodeLocation($statements_analyzer->getSource(), $stmt)
                 ),
                 $statements_analyzer->getSuppressedIssues()
@@ -524,7 +525,7 @@ public static function castFloatAttempt(
     ): Union {
         $codebase = $statements_analyzer->getCodebase();
 
-        $possibly_unwanted_cast = [];
+        $risky_cast = [];
         $invalid_casts = [];
         $valid_floats = [];
         $castable_types = [];
@@ -670,7 +671,7 @@ public static function castFloatAttempt(
             if ($atomic_type instanceof TNonEmptyArray
                 || $atomic_type instanceof TNonEmptyList
             ) {
-                $possibly_unwanted_cast[] = $atomic_type->getId();
+                $risky_cast[] = $atomic_type->getId();
 
                 $valid_floats[] = new TLiteralFloat(1.0);
 
@@ -683,7 +684,7 @@ public static function castFloatAttempt(
             ) {
                 // 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 :-)
-                $possibly_unwanted_cast[] = $atomic_type->getId();
+                $risky_cast[] = $atomic_type->getId();
 
                 $valid_floats[] = new TLiteralFloat(0.0);
                 $valid_floats[] = new TLiteralFloat(1.0);
@@ -711,10 +712,10 @@ public static function castFloatAttempt(
                 ),
                 $statements_analyzer->getSuppressedIssues()
             );
-        } elseif (!empty($possibly_unwanted_cast)) {
+        } elseif (!empty($risky_cast)) {
             IssueBuffer::maybeAdd(
-                new PossiblyInvalidCast(
-                    'Casting ' . $possibly_unwanted_cast[0] . ' to float has possibly unintended value of 1.0',
+                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()

diff --git a/src/Psalm/Issue/RiskyCast.php b/src/Psalm/Issue/RiskyCast.php
@@ -0,0 +1,9 @@
+<?php
+
+namespace Psalm\Issue;
+
+class RiskyCast extends CodeIssue
+{
+    public const ERROR_LEVEL = 3;
+    public const SHORTCODE = 313;
+}