WIP(broken) - mixed should be treated as nullable

- Work on fixing the false positives during redundant condition detection - Add a separate `non-null-mixed` type - TODO: Avoid regressions in inferred phpdoc type for array access - TODO: Continue to infer phpdoc `?mixed` type with a `?` in issue messages when combining mixed with null or converting mixed to nullable. (and emit PhanTypeArraySuspiciousNullable) For phan#4276
TysonAndre · Dec 1, 2020 · 64eb70b · 64eb70b
1 parent edc8fe4
commit 64eb70b
Show file tree

Hide file tree

Showing 15 changed files with 176 additions and 20 deletions.
diff --git a/src/Phan/AST/UnionTypeVisitor.php b/src/Phan/AST/UnionTypeVisitor.php
@@ -1707,7 +1707,7 @@ public function visitDim(Node $node, bool $treat_undef_as_nullable = false): Uni
         // If we have generics, we're all set
         if (!$generic_types->isEmpty()) {
             $generic_types = $generic_types->asNormalizedTypes();
-            if (!($node->flags & self::FLAG_IGNORE_NULLABLE) && $union_type->containsNullable()) {
+            if (!($node->flags & self::FLAG_IGNORE_NULLABLE) && $union_type->containsNonMixedNullable()) {
                 $this->emitIssue(
                     Issue::TypeArraySuspiciousNullable,
                     $node->lineno,

diff --git a/src/Phan/Analysis/ArgumentType.php b/src/Phan/Analysis/ArgumentType.php
@@ -1113,7 +1113,7 @@ private static function warnInvalidArgumentType(
                 return null;
             }
             // @phan-suppress-next-line PhanAccessMethodInternal
-            if (!$argument_type_expanded_resolved->canCastToUnionTypeIfNonNull($alternate_parameter_type)) {
+            if ($argument_type_expanded_resolved->isNull() || !$argument_type_expanded_resolved->canCastToUnionTypeIfNonNull($alternate_parameter_type)) {
                 if ($argument_type->hasRealTypeSet() && $alternate_parameter_type->hasRealTypeSet()) {
                     $real_arg_type = $argument_type->getRealUnionType();
                     $real_parameter_type = $alternate_parameter_type->getRealUnionType();

diff --git a/src/Phan/Analysis/ConditionVisitor.php b/src/Phan/Analysis/ConditionVisitor.php
@@ -855,10 +855,12 @@ private static function initTypeModifyingClosuresForVisitCall(): array
                     // FIXME move this to PostOrderAnalysisVisitor so that all expressions can be analyzed, not just variables?
                     $new_type = $default_if_empty;
                 } else {
-                    $new_type = $new_type->nonNullableClone();
+                    // Add the missing type set before making the non-nullable clone.
+                    // Otherwise, it'd have the real type set non-null-mixed.
                     if (!$new_type->hasRealTypeSet()) {
                         $new_type = $new_type->withRealTypeSet($default_if_empty->getRealTypeSet());
                     }
+                    $new_type = $new_type->nonNullableClone();
                     if (!$allow_undefined) {
                         $new_type = $new_type->withIsPossiblyUndefined(false);
                     }

diff --git a/src/Phan/Analysis/PostOrderAnalysisVisitor.php b/src/Phan/Analysis/PostOrderAnalysisVisitor.php
@@ -1655,7 +1655,7 @@ private function emitTypeMismatchReturnIssue(UnionType $expression_type, Functio
             // Suppressing TypeMismatchReturnReal also suppresses less severe return type mismatches
             return;
         }
-        if ($this->checkCanCastToReturnTypeIfWasNonNullInstead($expression_type, $method_return_type)) {
+        if (!$expression_type->isNull() && $this->checkCanCastToReturnTypeIfWasNonNullInstead($expression_type, $method_return_type)) {
             if ($this->shouldSuppressIssue(Issue::TypeMismatchReturn, $lineno)) {
                 // Suppressing TypeMismatchReturn also suppresses TypeMismatchReturnNullable
                 return;

diff --git a/src/Phan/Language/EmptyUnionType.php b/src/Phan/Language/EmptyUnionType.php
@@ -351,7 +351,7 @@ public function containsNullableOrUndefined(): bool
     /** @override */
     public function nonNullableClone(): UnionType
     {
-        return $this;
+        return UnionType::fromFullyQualifiedRealString('non-null-mixed');
     }
 
     /** @override */
@@ -369,7 +369,7 @@ public function withNullableRealTypes(): UnionType
     /** @override */
     public function withIsNullable(bool $is_nullable): UnionType
     {
-        return $this;
+        return $is_nullable ? $this : $this->nonNullableClone();
     }
 
     /**
@@ -553,10 +553,13 @@ public function canCastToUnionTypeWithoutConfig(
      * @internal
      * @override
      */
+    /**
+     * No longer a special case
     public function canCastToUnionTypeIfNonNull(UnionType $target): bool
     {
         return false;
     }
+     */
 
     public function canCastToUnionTypeHandlingTemplates(
         UnionType $target,

diff --git a/src/Phan/Language/Type.php b/src/Phan/Language/Type.php
@@ -56,6 +56,7 @@
 use Phan\Language\Type\NonEmptyListType;
 use Phan\Language\Type\NonEmptyMixedType;
 use Phan\Language\Type\NonEmptyStringType;
+use Phan\Language\Type\NonNullMixedType;
 use Phan\Language\Type\NonZeroIntType;
 use Phan\Language\Type\NullType;
 use Phan\Language\Type\ObjectType;
@@ -241,6 +242,7 @@ class Type
         'non-empty-array' => true,
         'non-empty-associative-array' => true,
         'non-empty-mixed' => true,
+        'non-null-mixed' => true,
         'non-empty-list'  => true,
         'non-empty-string' => true,
         'non-empty-lowercase-string' => true,
@@ -815,6 +817,8 @@ public static function fromInternalTypeName(
                 return MixedType::instance($is_nullable);
             case 'non-empty-mixed':
                 return NonEmptyMixedType::instance($is_nullable);
+            case 'non-null-mixed':
+                return NonNullMixedType::instance($is_nullable);
             case 'non-empty-array':
                 return NonEmptyGenericArrayType::fromElementType(MixedType::instance(false), $is_nullable, GenericArrayType::KEY_MIXED);
             case 'non-empty-associative-array':

diff --git a/src/Phan/Language/Type/MixedType.php b/src/Phan/Language/Type/MixedType.php
@@ -181,5 +181,18 @@ public function asNonFalseyType(): Type
     {
         return NonEmptyMixedType::instance(false);
     }
+
+    /** Overridden by NonEmptyMixedType */
+    public function isNullable(): bool
+    {
+        return true;
+    }
+
+    /** Overridden by NonEmptyMixedType */
+    public function __toString(): string
+    {
+        return $this->is_nullable ? '?mixed' : 'mixed';
+    }
 }
 class_exists(NonEmptyMixedType::class);
+class_exists(NonNullMixedType::class);
diff --git a/src/Phan/Language/Type/NonEmptyMixedType.php b/src/Phan/Language/Type/NonEmptyMixedType.php
@@ -9,7 +9,7 @@
 use Phan\Language\Type;
 
 /**
- * Represents the PHPDoc `non-empty-mixed` type, which can cast to/from any type and is truthy.
+ * Represents the PHPDoc `non-empty-mixed` type, which can cast to/from any non-empty type and is truthy.
  *
  * For purposes of analysis, there's usually no difference between mixed and nullable mixed.
  * @phan-pure
@@ -90,4 +90,16 @@ public function asNonFalseyType(): Type
     {
         return $this->withIsNullable(false);
     }
+
+    /** @override */
+    public function isNullable(): bool
+    {
+        return $this->is_nullable;
+    }
+
+    /** @override */
+    public function __toString(): string
+    {
+        return $this->is_nullable ? '?non-empty-mixed' : 'non-empty-mixed';
+    }
 }
diff --git a/src/Phan/Language/Type/NonNullMixedType.php b/src/Phan/Language/Type/NonNullMixedType.php
@@ -0,0 +1,94 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Phan\Language\Type;
+
+use Phan\CodeBase;
+use Phan\Language\Context;
+use Phan\Language\Type;
+
+/**
+ * Represents the PHPDoc `non-empty-mixed` type, which can cast to/from any non-null type and is non-null
+ *
+ * For purposes of analysis, there's usually no difference between mixed and nullable mixed.
+ * @phan-pure
+ */
+final class NonNullMixedType extends MixedType
+{
+    /** @phan-override */
+    public const NAME = 'non-null-mixed';
+
+    public static function instance(bool $is_nullable)
+    {
+        if ($is_nullable) {
+            return MixedType::instance(true);
+        }
+        static $instance = null;
+        return $instance ?? ($instance = static::make('\\', self::NAME, [], false, Type::FROM_NODE));
+    }
+
+    public function canCastToType(Type $type): bool
+    {
+        return !($type instanceof NullType || $type instanceof VoidType);
+    }
+
+    /**
+     * @param Type[] $target_type_set 1 or more types @phan-unused-param
+     * @override
+     */
+    public function canCastToAnyTypeInSet(array $target_type_set): bool
+    {
+        foreach ($target_type_set as $t) {
+            if ($this->canCastToType($t)) {
+                return true;
+            }
+        }
+        return (bool)$target_type_set;
+    }
+
+    public function asGenericArrayType(int $key_type): Type
+    {
+        return GenericArrayType::fromElementType($this, false, $key_type);
+    }
+
+    /**
+     * @unused-param $code_base
+     * @unused-param $context
+     */
+    public function canCastToDeclaredType(CodeBase $code_base, Context $context, Type $other): bool
+    {
+        return $this->canCastToType($other);
+    }
+
+    public function asObjectType(): ?Type
+    {
+        return ObjectType::instance(false);
+    }
+
+    public function asArrayType(): ?Type
+    {
+        return NonEmptyGenericArrayType::fromElementType(
+            MixedType::instance(false),
+            false,
+            GenericArrayType::KEY_MIXED
+        );
+    }
+
+    public function asNonFalseyType(): Type
+    {
+        return $this;
+    }
+
+    /** @override */
+    public function isNullable(): bool
+    {
+        return $this->is_nullable;
+    }
+
+    /** @override */
+    public function __toString(): string
+    {
+        return $this->is_nullable ? '?non-null-mixed' : 'non-null-mixed';
+    }
+}
diff --git a/src/Phan/Language/UnionType.php b/src/Phan/Language/UnionType.php
@@ -1365,6 +1365,28 @@ public function containsNullable(): bool
         return false;
     }
 
+    /**
+     * @return bool - True if not empty and at least one type is NullType or nullable.
+     *
+     * To reduce false positives for unknown array element types (etc.),
+     * this distinguishes between the phpdoc types ?mixed and mixed,
+     * even though both can contain false.
+     */
+    public function containsNonMixedNullable(): bool
+    {
+        foreach ($this->type_set as $type) {
+            if ($type->isNullable()) {
+                if ($type instanceof MixedType) {
+                    if ($type->__toString() === 'mixed') {
+                        continue;
+                    }
+                }
+                return true;
+            }
+        }
+        return false;
+    }
+
     /**
      * @return bool - True if empty or at least one type is NullType or nullable.
      * e.g. true for `?int`, `int|null`, or ``
@@ -1381,11 +1403,12 @@ public function containsNullableOrIsEmpty(): bool
 
     /**
      * @return bool - True if not empty and at least one type is NullType or mixed.
+     * TODO deprecate and remove
      */
     public function containsNullableOrMixed(): bool
     {
         foreach ($this->type_set as $type) {
-            if ($type->isNullable() || $type instanceof MixedType) {
+            if ($type->isNullable()) {
                 return true;
             }
         }
@@ -1443,7 +1466,7 @@ private static function toNonNullableTypeList(array $type_list): array
 
             $result[] = $type->withIsNullable(false);
         }
-        return $result;
+        return $result ?: UnionType::typeSetFromString('non-null-mixed');
     }
 
 

diff --git a/tests/Phan/Language/UnionTypeTest.php b/tests/Phan/Language/UnionTypeTest.php
@@ -114,7 +114,7 @@ public function testInt(): void
         $this->assertUnionTypeStringEqual('$argc * 1.5', 'float');
         $this->assertUnionTypeStringEqual('constant($argv[0]) - constant($argv[1])', 'float|int');
         $this->assertUnionTypeStringEqual('constant($argv[0]) + constant($argv[1])', 'float|int');
-        $this->assertUnionTypeStringEqual('-constant($argv[0])', 'float|int');
+        $this->assertUnionTypeStringEqual('-constant($argv[0])', '0|float|int');
         $this->assertUnionTypeStringEqual('-(1.5)', '-1.5');
         $this->assertUnionTypeStringEqual('(rand(0,1) ? "12" : 2.5) - (rand(0,1) ? "3" : 1.5)', 'float|int');
         $this->assertUnionTypeStringEqual('(rand(0,1) ? "12" : 2.5) * (rand(0,1) ? "3" : 1.5)', 'float|int');

diff --git a/tests/files/expected/0460_silence_condition.php.expected b/tests/files/expected/0460_silence_condition.php.expected
@@ -1,2 +1,4 @@
-%s:11 PhanTypeMismatchArgumentInternal%SReal Argument 1 ($value) is $x of type string but \count() takes \Countable|\ResourceBundle|\SimpleXMLElement|array%S
-%s:14 PhanTypeMismatchArgumentInternal Argument 1 ($value) is $y of type string but \count() takes \Countable|\ResourceBundle|\SimpleXMLElement|array
+%s:9 PhanDebugAnnotation @phan-debug-var requested for variable $x - it has union type array|string
+%s:11 PhanDebugAnnotation @phan-debug-var requested for variable $x - it has union type string(real=string)
+%s:13 PhanTypeMismatchArgumentInternalReal Argument 1 ($value) is $x of type string but \count() takes \Countable|\ResourceBundle|\SimpleXMLElement|array (real type \Countable|array)
+%s:16 PhanTypeMismatchArgumentInternal Argument 1 ($value) is $y of type string but \count() takes \Countable|\ResourceBundle|\SimpleXMLElement|array
diff --git a/tests/files/src/0460_silence_condition.php b/tests/files/src/0460_silence_condition.php
@@ -7,7 +7,9 @@
  * The error control operator is unnecessary here
  */
 function silence460($x, $y) {
+    '@phan-debug-var $x';
     if (@is_string($x)) {
+        '@phan-debug-var $x';
         echo count($x);
     }
     if (@!is_array($y)) {

diff --git a/tests/php72_files/expected/0001_objecthint.php.expected b/tests/php72_files/expected/0001_objecthint.php.expected
@@ -1,4 +1,4 @@
-%s:9 PhanTypeMismatchReturnReal Returning null of type null but object_test() is declared to return object
+/tests/php72_files/src/0001_objecthint.php:9 PhanTypeMismatchReturnReal Returning null of type null but object_test() is declared to return object
 %s:13 PhanTypeMismatchArgumentReal Argument 1 ($x) is null of type null but \object_test() takes object defined at %s:3
 %s:14 PhanTypeMismatchArgumentReal Argument 1 ($x) is [] of type array{} but \object_test() takes object defined at %s:3
 %s:14 PhanTypeMismatchArgumentReal Argument 2 ($y) is [] of type array{} but \object_test() takes ?object defined at %s:3
@@ -7,4 +7,5 @@
 %s:30 PhanNonClassMethodCall Call to method __construct on non-class type false
 %s:30 PhanTypeExpectedObjectOrClassName Expected an object instance or the name of a class but saw expression false with type false
 %s:31 PhanUndeclaredClassMethod Call to method __construct from undeclared class \true
+%s:32 PhanNonClassMethodCall Call to method __construct on non-class type mixed
 %s:32 PhanTypeExpectedObjectOrClassName Expected an object instance or the name of a class but saw expression mixed with type mixed
diff --git a/tests/php80_files/expected/025_named_arg_callable_missing.php.expected b/tests/php80_files/expected/025_named_arg_callable_missing.php.expected
@@ -1,20 +1,20 @@
 %s:8 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (other: true)
-%s:8 PhanMissingNamedArgument Missing named argument for int $requiredInt in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, ?mixed $other = null) defined at %s:4
-%s:8 PhanMissingNamedArgument Missing named argument for string $requiredString in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, ?mixed $other = null) defined at %s:4
-%s:8 PhanParamTooFewCallable Call with 1 arg(s) to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, ?mixed $other = null) (as a provided callable) which requires 2 arg(s) defined at %s:4
+%s:8 PhanMissingNamedArgument Missing named argument for int $requiredInt in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, $other = null) defined at %s:4
+%s:8 PhanMissingNamedArgument Missing named argument for string $requiredString in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, $other = null) defined at %s:4
+%s:8 PhanParamTooFewCallable Call with 1 arg(s) to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, $other = null) (as a provided callable) which requires 2 arg(s) defined at %s:4
 %s:9 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (optionalFlag: true)
-%s:9 PhanMissingNamedArgument Missing named argument for string $requiredString in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, ?mixed $other = null) defined at %s:4
+%s:9 PhanMissingNamedArgument Missing named argument for string $requiredString in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, $other = null) defined at %s:4
 %s:10 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (optionalFlag: true)
 %s:10 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (other: 123)
-%s:10 PhanMissingNamedArgument Missing named argument for string $requiredString in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, ?mixed $other = null) defined at %s:4
+%s:10 PhanMissingNamedArgument Missing named argument for string $requiredString in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, $other = null) defined at %s:4
 %s:11 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (optionalFlag: true)
 %s:11 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (other: 123)
 %s:11 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (requiredInt: 0)
-%s:11 PhanMissingNamedArgument Missing named argument for string $requiredString in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, ?mixed $other = null) defined at %s:4
+%s:11 PhanMissingNamedArgument Missing named argument for string $requiredString in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, $other = null) defined at %s:4
 %s:12 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (optionalFlag: true)
 %s:12 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (other: 123)
 %s:12 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (requiredString: 0)
-%s:12 PhanMissingNamedArgument Missing named argument for int $requiredInt in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, ?mixed $other = null) defined at %s:4
+%s:12 PhanMissingNamedArgument Missing named argument for int $requiredInt in call to \C24::main(int $requiredInt, string $requiredString, bool $optionalFlag = false, $other = null) defined at %s:4
 %s:13 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (definitelyInvalidFlag: 'value')
 %s:13 PhanUndeclaredNamedArgumentInternal Saw a call with undeclared named argument (definitelyInvalidFlag: 'value') to \strlen(string $string)
 %s:14 PhanCompatibleNamedArgument Cannot use named arguments before php 8.0 in argument (flags: 123)