Improve handling of mixed, checking type compatibility

For phan#4276
TysonAndre · Dec 9, 2020 · 40445e4 · 40445e4
1 parent d45f52b
commit 40445e4
Show file tree

Hide file tree

Showing 17 changed files with 91 additions and 26 deletions.
diff --git a/src/Phan/Analysis/ParameterTypesAnalyzer.php b/src/Phan/Analysis/ParameterTypesAnalyzer.php
@@ -655,14 +655,13 @@ private static function analyzeOverrideSignatureForOverriddenMethod(
                     break;
                 }
 
-                // A stricter type on an overriding method is cool
-                if ($parameter->getUnionType()->isEmpty()
-                    || $parameter->getUnionType()->isType(MixedType::instance(false))
-                ) {
+                if ($parameter->getUnionType()->isEmptyOrMixed()) {
+                    // parameter type widening is allowed
                     continue;
                 }
 
-                if ($o_parameter->getUnionType()->isEmpty() || $o_parameter->getUnionType()->isType(MixedType::instance(false))) {
+                if ($o_parameter->getUnionType()->isEmptyOrMixed()) {
+                    // XXX Not sure why this check was here but there are better checks elsewhere for real mismatches
                     continue;
                 }
 

diff --git a/src/Phan/Language/Element/FunctionTrait.php b/src/Phan/Language/Element/FunctionTrait.php
@@ -885,7 +885,7 @@ public static function addParamToScopeOfFunctionOrMethod(
             // to null
             if ($default_is_null) {
                 if ($was_empty) {
-                    $parameter->addUnionType(MixedType::instance(false)->asPHPDocUnionType());
+                    $parameter->addUnionType(MixedType::instance(true)->asPHPDocUnionType());
                 }
                 // The parameter constructor or above check for wasEmpty already took care of null default case
             } else {

diff --git a/src/Phan/Language/Type.php b/src/Phan/Language/Type.php
@@ -104,17 +104,17 @@ class Type
      * A legal type identifier (e.g. 'int' or 'DateTime')
      */
     public const simple_type_regex =
-        '(\??)(?:callable-(?:string|object|array)|associative-array|class-string|lowercase-string|non-(?:zero-int|empty-(?:associative-array|array|list|string|lowercase-string|mixed))|\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*)';
+        '(\??)(?:callable-(?:string|object|array)|associative-array|class-string|lowercase-string|non-(?:zero-int|null-mixed|empty-(?:associative-array|array|list|string|lowercase-string|mixed))|\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*)';
 
     public const simple_noncapturing_type_regex =
-        '\\\\?(?:callable-(?:string|object|array)|associative-array|class-string|lowercase-string|non-(?:zero-int|empty-(?:associative-array|array|list|string|lowercase-string|mixed))|[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*)';
+        '\\\\?(?:callable-(?:string|object|array)|associative-array|class-string|lowercase-string|non-(?:zero-int|null-mixed|empty-(?:associative-array|array|list|string|lowercase-string|mixed))|[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*)';
 
     /**
      * @var string
      * A legal type identifier (e.g. 'int' or 'DateTime')
      */
     public const simple_type_regex_or_this =
-        '(\??)(callable-(?:string|object|array)|associative-array|class-string|lowercase-string|non-(?:zero-int|empty-(?:associative-array|array|list|string|lowercase-string|mixed))|\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*|\$this)';
+        '(\??)(callable-(?:string|object|array)|associative-array|class-string|lowercase-string|non-(?:zero-int|null-mixed|empty-(?:associative-array|array|list|string|lowercase-string|mixed))|\\\\?[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*(?:\\\\[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*)*|\$this)';
 
     public const shape_key_regex =
         '(?:[-.\/^;$%*+_a-zA-Z0-9\x7f-\xff]|\\\\(?:[nrt\\\\]|x[0-9a-fA-F]{2}))+\??';
@@ -1604,11 +1604,11 @@ private static function checkClosureString(
     private static function maybeFindParentType(bool $is_nullable, Context $context, CodeBase $code_base = null): Type
     {
         if ($code_base === null) {
-            return MixedType::instance($is_nullable);
+            return NonNullMixedType::instance($is_nullable);
         }
         $parent_type = UnionTypeVisitor::findParentType($context, $code_base);
         if (!$parent_type) {
-            return MixedType::instance($is_nullable);
+            return NonNullMixedType::instance($is_nullable);
         }
 
         return $parent_type->withIsNullable($is_nullable);
@@ -2872,7 +2872,8 @@ public function canCastToType(Type $type): bool
         }
 
         if ($type instanceof MixedType) {
-            return \get_class($type) === MixedType::class || $this->isPossiblyTruthy();
+            // This is not NullType; it has to be truthy to cast to non-empty-mixed.
+            return \get_class($type) !== NonEmptyMixedType::class || $this->isPossiblyTruthy();
         }
 
         if ($this->is_nullable) {
@@ -2922,7 +2923,8 @@ public function canCastToTypeHandlingTemplates(Type $type, CodeBase $code_base):
         }
 
         if ($type instanceof MixedType) {
-            return true;
+            // This is not NullType; it has to be truthy to cast to non-empty-mixed.
+            return \get_class($type) !== NonEmptyMixedType::class || $this->isPossiblyTruthy();
         }
 
         // A nullable type cannot cast to a non-nullable type
@@ -2968,7 +2970,8 @@ public function canCastToTypeWithoutConfig(Type $type): bool
         }
 
         if ($type instanceof MixedType) {
-            return true;
+            // This is not NullType; it has to be truthy to cast to non-empty-mixed.
+            return \get_class($type) !== NonEmptyMixedType::class || $this->isPossiblyTruthy();
         }
 
         if ($this->is_nullable) {
@@ -3017,7 +3020,8 @@ protected function canCastToNonNullableType(Type $type): bool
         }
 
         if ($type instanceof MixedType) {
-            return true;
+            // This is not NullType; it has to be truthy to cast to non-empty-mixed.
+            return \get_class($type) !== NonEmptyMixedType::class || $this->isPossiblyTruthy();
         }
 
         // Check for allowable type conversions from object types to native types
@@ -3057,7 +3061,8 @@ protected function canCastToNonNullableTypeWithoutConfig(Type $type): bool
         }
 
         if ($type instanceof MixedType) {
-            return true;
+            // This is not NullType; it has to be truthy to cast to non-empty-mixed.
+            return \get_class($type) !== NonEmptyMixedType::class || $this->isPossiblyTruthy();
         }
 
         // Check for allowable type conversions from object types to native types
@@ -3116,7 +3121,8 @@ public function isSubtypeOf(Type $type): bool
         if ($type instanceof MixedType) {
             // e.g. ?int is a subtype of mixed, but ?int is not a subtype of non-empty-mixed/non-null-mixed
             // (check isNullable first)
-            return true;
+            // This is not NullType; it has to be truthy to cast to non-empty-mixed.
+            return \get_class($type) !== NonEmptyMixedType::class || $this->isPossiblyTruthy();
         }
 
         // Get a non-null version of the type we're comparing
@@ -4039,6 +4045,7 @@ public function asScalarType(): ?Type
      */
     public function weaklyOverlaps(Type $other): bool
     {
+        // TODO: Finish implementing, check if types are compatible when both are non-null, check for object vs non-object
         return $this->isPossiblyFalsey() && $other->isPossiblyFalsey();
     }
 

diff --git a/src/Phan/Language/Type/FalseType.php b/src/Phan/Language/Type/FalseType.php
@@ -51,6 +51,11 @@ public function isPossiblyFalse(): bool
         return true;
     }
 
+    public function isPossiblyTrue(): bool
+    {
+        return true;
+    }
+
     public function asNonFalseType(): Type
     {
         if (!($this->is_nullable)) {
@@ -94,10 +99,15 @@ public function canSatisfyComparison($scalar, int $flags): bool
         return self::performComparison(false, $scalar, $flags);
     }
 
+    /**
+     * Check if this type can possibly cast to the declared type, ignoring nullability of this type
+     *
+     * Precondition: This is either non-nullable or the type NullType/VoidType
+     * @unused-param $context
+     */
     public function canCastToDeclaredType(CodeBase $code_base, Context $context, Type $other): bool
     {
-        return $other->isInBoolFamily() ||
-            \get_class($other) === MixedType::class ||
+        return $other->isPossiblyTrue() ||
             $other instanceof TemplateType ||
             (!$context->isStrictTypes() && parent::canCastToDeclaredType($code_base, $context, $other));
     }

diff --git a/src/Phan/Language/Type/MixedType.php b/src/Phan/Language/Type/MixedType.php
@@ -67,6 +67,7 @@ protected function canCastToNonNullableTypeWithoutConfig(Type $type): bool
         return true;
     }
 
+    // FIXME: non-empty-mixed/non-null-mixed is a subtype of mixed, but not vice versa?
     public function isSubtypeOf(Type $type): bool
     {
         return $type instanceof MixedType;
@@ -167,6 +168,16 @@ public function isAlwaysTruthy(): bool
         return false;
     }
 
+    public function isPossiblyFalse(): bool
+    {
+        return true;
+    }
+
+    public function isPossiblyTrue(): bool
+    {
+        return true;
+    }
+
     public function asObjectType(): ?Type
     {
         return ObjectType::instance(false);
@@ -198,6 +209,12 @@ public function __toString(): string
     {
         return $this->is_nullable ? '?mixed' : 'mixed';
     }
+
+    /** @unused-param $other */
+    public function weaklyOverlaps(Type $other): bool
+    {
+        return true;
+    }
 }
 class_exists(NonEmptyMixedType::class);
 class_exists(NonNullMixedType::class);
diff --git a/src/Phan/Language/Type/NativeType.php b/src/Phan/Language/Type/NativeType.php
@@ -216,6 +216,7 @@ private static function initializeTypeCastingMatrix(): array
                 FalseType::NAME    => in_array(FalseType::NAME, $permitted_cast_type_names, true),
                 FloatType::NAME    => in_array(FloatType::NAME, $permitted_cast_type_names, true),
                 IntType::NAME      => in_array(IntType::NAME, $permitted_cast_type_names, true),
+                // TODO: Handle other subtypes of mixed?
                 MixedType::NAME    => true,
                 NullType::NAME     => in_array(NullType::NAME, $permitted_cast_type_names, true),
                 ObjectType::NAME   => in_array(ObjectType::NAME, $permitted_cast_type_names, true),

diff --git a/src/Phan/Language/Type/NonEmptyMixedType.php b/src/Phan/Language/Type/NonEmptyMixedType.php
@@ -67,6 +67,11 @@ public function isPossiblyFalsey(): bool
         return $this->is_nullable;
     }
 
+    public function isPossiblyFalse(): bool
+    {
+        return false;
+    }
+
     public function isAlwaysTruthy(): bool
     {
         return !$this->is_nullable;
@@ -102,4 +107,9 @@ public function __toString(): string
     {
         return $this->is_nullable ? '?non-empty-mixed' : 'non-empty-mixed';
     }
+
+    public function weaklyOverlaps(Type $other): bool
+    {
+        return $other->isPossiblyTruthy();
+    }
 }
diff --git a/src/Phan/Language/Type/NonNullMixedType.php b/src/Phan/Language/Type/NonNullMixedType.php
@@ -95,4 +95,9 @@ public function __toString(): string
     {
         return $this->is_nullable ? '?non-null-mixed' : 'non-null-mixed';
     }
+
+    public function weaklyOverlaps(Type $other): bool
+    {
+        return !$other instanceof NullType && !$other instanceof VoidType;
+    }
 }
diff --git a/src/Phan/Language/Type/TrueType.php b/src/Phan/Language/Type/TrueType.php
@@ -41,6 +41,11 @@ public function isPossiblyTrue(): bool
         return true;
     }
 
+    public function isPossiblyFalse(): bool
+    {
+        return true;
+    }
+
     public function isAlwaysTrue(): bool
     {
         return !$this->is_nullable;  // If it can be null, it's not **always** identical to true
@@ -91,8 +96,7 @@ public function canSatisfyComparison($scalar, int $flags): bool
 
     public function canCastToDeclaredType(CodeBase $code_base, Context $context, Type $other): bool
     {
-        return $other->isInBoolFamily() ||
-            $other instanceof MixedType ||
+        return $other->isPossiblyTrue() ||
             $other instanceof TemplateType ||
             (!$context->isStrictTypes() && parent::canCastToDeclaredType($code_base, $context, $other));
     }

diff --git a/src/Phan/Plugin/Internal/ArrayReturnTypeOverridePlugin.php b/src/Phan/Plugin/Internal/ArrayReturnTypeOverridePlugin.php
@@ -607,7 +607,7 @@ static function (UnionType $union_type): bool {
                     ]
                 );
             }
-            return $mixed_type->asPHPDocUnionType();
+            return $false_type->asPHPDocUnionType();
         };
         /**
          * @param list<Node|int|float|string> $args

diff --git a/src/Phan/Plugin/Internal/MethodSearcherPlugin.php b/src/Phan/Plugin/Internal/MethodSearcherPlugin.php
@@ -271,6 +271,7 @@ private static function guessUnionType(FunctionInterface $function): UnionType
         return UnionType::empty();
     }
 
+    // TODO: Handle non-null-mixed/non-empty-mixed
     private static function isMixed(UnionType $union_type): bool
     {
         foreach ($union_type->getTypeSet() as $type) {

diff --git a/tests/Phan/Language/EmptyUnionTypeTest.php b/tests/Phan/Language/EmptyUnionTypeTest.php
@@ -18,10 +18,13 @@
 use Phan\Language\Type\ArrayType;
 use Phan\Language\Type\FalseType;
 use Phan\Language\Type\GenericArrayType;
+use Phan\Language\Type\LiteralIntType;
 use Phan\Language\Type\IntType;
 use Phan\Language\Type\MixedType;
+use Phan\Language\Type\NonNullMixedType;
 use Phan\Language\Type\ObjectType;
 use Phan\Language\Type\TemplateType;
+use Phan\Language\Type\TrueType;
 use Phan\Language\UnionType;
 use Phan\Tests\BaseTest;
 use ReflectionClass;
@@ -181,9 +184,12 @@ public function getPossibleArgValues(ReflectionParameter $param): array
                     IntType::instance(false),
                     ArrayType::instance(false),
                     FalseType::instance(true),
+                    TrueType::instance(false),
                     ObjectType::instance(false),
                     MixedType::instance(false),
+                    NonNullMixedType::instance(false),
                     Type::fromFullyQualifiedString('\stdClass'),
+                    LiteralIntType::instanceForValue(0, false),
                 ];
             case UnionType::class:
                 // TODO: Add tests of real union types
@@ -198,6 +204,7 @@ public function getPossibleArgValues(ReflectionParameter $param): array
                     ObjectType::instance(false)->asPHPDocUnionType(),
                     ObjectType::instance(false)->asRealUnionType(),
                     MixedType::instance(false)->asPHPDocUnionType(),
+                    MixedType::instance(true)->asPHPDocUnionType(),
                     Type::fromFullyQualifiedString('\stdClass')->asPHPDocUnionType(),
                 ];
             case Closure::class:

diff --git a/tests/Phan/Language/TypeTest.php b/tests/Phan/Language/TypeTest.php
@@ -28,6 +28,8 @@
 use Phan\Language\Type\LiteralIntType;
 use Phan\Language\Type\LiteralStringType;
 use Phan\Language\Type\MixedType;
+use Phan\Language\Type\NonEmptyMixedType;
+use Phan\Language\Type\NonNullMixedType;
 use Phan\Language\Type\ObjectType;
 use Phan\Language\Type\ResourceType;
 use Phan\Language\Type\StaticType;
@@ -81,6 +83,8 @@ public function testBasicTypes(): void
         $this->assertParsesAsType(IntType::instance(false), 'int');
         $this->assertParsesAsType(IterableType::instance(false), 'iterable');
         $this->assertParsesAsType(MixedType::instance(false), 'mixed');
+        $this->assertParsesAsType(NonEmptyMixedType::instance(false), 'non-empty-mixed');
+        $this->assertParsesAsType(NonNullMixedType::instance(false), 'non-null-mixed');
         $this->assertParsesAsType(ObjectType::instance(false), 'object');
         $this->assertParsesAsType(ResourceType::instance(false), 'resource');
         $this->assertParsesAsType(StaticType::instance(false), 'static');

diff --git a/tests/files/expected/0003_required_after_optional.php.expected b/tests/files/expected/0003_required_after_optional.php.expected
@@ -1 +1 @@
-%s:4 PhanParamReqAfterOpt Required parameter ($required) follows optional (mixed $optional = null)
+%s:4 PhanParamReqAfterOpt Required parameter ($required) follows optional (?mixed $optional = null)
diff --git a/tests/files/expected/0057_nullable_required_typed_parameter.php.expected b/tests/files/expected/0057_nullable_required_typed_parameter.php.expected
@@ -1,4 +1,4 @@
 %s:6 PhanParamReqAfterOpt Required parameter (int $b) follows optional (\DateTime|null $a = null)
 %s:8 PhanParamReqAfterOpt Required parameter (\DateTime $d) follows optional (?int $c = null)
-%s:8 PhanParamReqAfterOpt Required parameter (\DateTime $f) follows optional (mixed $e = null)
+%s:8 PhanParamReqAfterOpt Required parameter (\DateTime $f) follows optional (?mixed $e = null)
 %s:8 PhanParamReqAfterOpt Required parameter (int $b) follows optional (?\DateTime $a = null)
diff --git a/tests/files/expected/0101_one_of_each.php.expected b/tests/files/expected/0101_one_of_each.php.expected
@@ -11,7 +11,7 @@
 %s:32 PhanNoopConstant Unused constant
 %s:37 PhanNoopProperty Unused property
 %s:42 PhanNoopVariable Unused variable
-%s:48 PhanParamReqAfterOpt Required parameter ($p2) follows optional (mixed $p1 = null)
+%s:48 PhanParamReqAfterOpt Required parameter ($p2) follows optional (?mixed $p1 = null)
 %s:51 PhanParamSpecial1 Argument 2 ($pieces) is string but \join() takes string[] when argument 1 is string
 %s:52 PhanCompatibleImplodeOrder In php 7.4, passing glue string after the array is deprecated for \join(). Should this swap the parameters of type array{} and string?
 %s:54 PhanParamSpecial2 Argument 1 ($pieces) is string but \join() takes string[] when passed only one argument

diff --git a/tests/files/expected/0124_override_signature.php.expected b/tests/files/expected/0124_override_signature.php.expected
@@ -17,5 +17,5 @@
 %s:67 PhanParamSignatureRealMismatchParamIsReference Declaration of function k(&$a) should be compatible with function k($a) (parameter #1 is a reference parameter overriding a non-reference parameter) defined in %s:62
 %s:68 PhanParamSignatureMismatch Declaration of function l($b) should be compatible with function l(&$b) defined in %s:63 (Difference in passing by reference in override $b of parameter &$b)
 %s:68 PhanParamSignatureRealMismatchParamIsNotReference Declaration of function l($b) should be compatible with function l(&$b) (parameter #1 is a non-reference parameter overriding a reference parameter) defined in %s:63
-%s:76 PhanParamSignatureMismatch Declaration of function m($a) should be compatible with function m(mixed $a = null) defined in %s:72 (Saw more required parameters in the override)
+%s:76 PhanParamSignatureMismatch Declaration of function m($a) should be compatible with function m(?mixed $a = null) defined in %s:72 (Saw more required parameters in the override)
 %s:76 PhanParamSignatureRealMismatchTooManyRequiredParameters Declaration of function m($a) should be compatible with function m($a = null) (the method override requires 1 parameter(s), but the overridden method requires only 0) defined in %s:72