Code changes to handle false positives for mixed

and run phpcbf
phan · Dec 7, 2020 · cd5bdb5 · cd5bdb5
1 parent c18ae5b
commit cd5bdb5
Show file tree

Hide file tree

Showing 27 changed files with 116 additions and 61 deletions.
diff --git a/.phan/plugins/DuplicateArrayKeyPlugin.php b/.phan/plugins/DuplicateArrayKeyPlugin.php
@@ -171,7 +171,7 @@ private function extendedLooseEqualityCheck(array $values_to_check, array $child
             if ($old_index !== null) {
                 $this->emitPluginIssue(
                     $this->code_base,
-                    clone($this->context)->withLineNumberStart($children[$i]->lineno),
+                    (clone($this->context))->withLineNumberStart($children[$i]->lineno),
                     'PhanPluginDuplicateSwitchCaseLooseEquality',
                     "Switch case({STRING_LITERAL}) is loosely equivalent (==) to an earlier case ({STRING_LITERAL}) in switch statement - the earlier entry may be chosen instead.",
                     [self::normalizeSwitchKey($values_to_check[$i]), self::normalizeSwitchKey($values_to_check[$old_index])],

diff --git a/internal/lib/IncompatibleRealStubsSignatureDetector.php b/internal/lib/IncompatibleRealStubsSignatureDetector.php
@@ -554,7 +554,7 @@ public function updateFunctionSignaturesParamNames(): void
              */
             try {
                 $new_signatures[$method_name] = $this->updateSignatureParamNames($method_name, $arguments);
-            } catch (InvalidArgumentException|FQSENException $e) {
+            } catch (InvalidArgumentException | FQSENException $e) {
                 fwrite(STDERR, "Unexpected invalid signature for $method_name, skipping: $e\n");
             }
         }
@@ -575,7 +575,7 @@ public function updateFunctionSignaturesParamNames(): void
                      */
                     try {
                         $delta_contents[$section][$method_name] = $this->updateSignatureParamNames($method_name, $arguments);
-                    } catch (InvalidArgumentException|FQSENException $e) {
+                    } catch (InvalidArgumentException | FQSENException $e) {
                         fwrite(STDERR, "Unexpected invalid signature for $method_name for $delta_path, skipping: $e\n");
                     }
                 }

diff --git a/internal/lib/IncompatibleSignatureDetectorBase.php b/internal/lib/IncompatibleSignatureDetectorBase.php
@@ -214,7 +214,7 @@ public function updateFunctionSignatures(): void
             }
             try {
                 $new_signatures[$method_name] = static::updateSignature($method_name, $arguments);
-            } catch (FQSENException|InvalidArgumentException $e) {
+            } catch (FQSENException | InvalidArgumentException $e) {
                 static::info("Skipping invalid signature for $method_name: $e\n");
                 $new_signatures[$method_name] = $arguments;
             }

diff --git a/src/Phan/AST/ContextNode.php b/src/Phan/AST/ContextNode.php
@@ -1467,7 +1467,7 @@ public function getProperty(
             foreach ($union_type->getTypeSet() as $type) {
                 if (!$type->isPossiblyObject()) {
                     $invalid = $invalid->withType($type);
-                } elseif ($type->isNullable()) {
+                } elseif ($type->isNullableLabeled()) {
                     $invalid = $invalid->withType(NullType::instance(false));
                 }
             }
@@ -1476,15 +1476,21 @@ public function getProperty(
                     $invalid = $invalid->nonNullableClone();
                 }
                 if (!$invalid->isEmpty()) {
-                    $this->emitIssue(
-                        Issue::PossiblyUndeclaredProperty,
-                        $node->lineno,
-                        $property_name,
-                        $union_type,
-                        $invalid
-                    );
-                    if ($property) {
-                        return $property;
+                    foreach ($invalid->getTypeset() as $type) {
+                        if (!$type->isNullableLabeled()) {
+                            continue;
+                        }
+                        $this->emitIssue(
+                            Issue::PossiblyUndeclaredProperty,
+                            $node->lineno,
+                            $property_name,
+                            $union_type,
+                            $invalid
+                        );
+                        if ($property) {
+                            return $property;
+                        }
+                        break;
                     }
                 }
             }

diff --git a/src/Phan/Analysis/AssignOperatorAnalysisVisitor.php b/src/Phan/Analysis/AssignOperatorAnalysisVisitor.php
@@ -574,7 +574,7 @@ private function analyzeBitwiseOperation(Node $node): Context
             $this->warnAboutInvalidUnionType(
                 $node,
                 static function (Type $type): bool {
-                    return ($type instanceof IntType || $type instanceof StringType || $type instanceof MixedType) && !$type->isNullable();
+                    return ($type instanceof IntType || $type instanceof StringType || $type instanceof MixedType) && !$type->isNullableLabeled();
                 },
                 $left_type,
                 $right_type,
@@ -720,7 +720,7 @@ private function analyzeBinaryShift(Node $node): void
         $this->warnAboutInvalidUnionType(
             $node,
             static function (Type $type): bool {
-                return ($type instanceof IntType || $type instanceof MixedType) && !$type->isNullable();
+                return ($type instanceof IntType || $type instanceof MixedType) && !$type->isNullableLabeled();
             },
             $left,
             $right,

diff --git a/src/Phan/Analysis/AssignmentVisitor.php b/src/Phan/Analysis/AssignmentVisitor.php
@@ -1878,9 +1878,9 @@ public function typeCheckDimAssignment(UnionType $assign_type, Node $node): Unio
                     }
                 }
             } elseif (!$assign_type->hasTypeMatchingCallback(static function (Type $type) use ($simple_xml_element_type): bool {
-                return !$type->isNullable() && ($type instanceof MixedType || $type === $simple_xml_element_type);
+                return !$type->isNullableLabeled() && ($type instanceof MixedType || $type === $simple_xml_element_type);
             })) {
-                // Imitate the check in UnionTypeVisitor, don't warn for mixed, etc.
+                // Imitate the check in UnionTypeVisitor, don't warn for mixed (but warn for `?mixed`), etc.
                 $this->emitIssue(
                     Issue::TypeArraySuspicious,
                     $node->lineno,

diff --git a/src/Phan/Analysis/BinaryOperatorFlagVisitor.php b/src/Phan/Analysis/BinaryOperatorFlagVisitor.php
@@ -838,8 +838,6 @@ public static function checkInvalidArrayShapeCombination(
         }
         $common_left_fields = null;
         foreach ($left->getRealTypeSet() as $type) {
-            // if ($type->isNullable()) { return; }
-
             if (!$type instanceof ArrayShapeType) {
                 if ($type instanceof ListType) {
                     continue;

diff --git a/src/Phan/Analysis/ConditionVisitorUtil.php b/src/Phan/Analysis/ConditionVisitorUtil.php
@@ -496,6 +496,8 @@ final protected function removeLiteralScalarFromVariable(
                 };
             }
         } else {
+            // Remove loosely equal types.
+            // TODO: Does this properly remove null for `$var != 0`?
             $cb = static function (Type $type) use ($value): bool {
                 return $type instanceof LiteralTypeInterface && $type->getValue() == $value;
             };

diff --git a/src/Phan/Analysis/NegatedConditionVisitor.php b/src/Phan/Analysis/NegatedConditionVisitor.php
@@ -573,6 +573,7 @@ private static function createNegationCallbackMap(): array
                 // Add types which are not scalars
                 foreach ($union_type->getTypeSet() as $type) {
                     if ($type_filter($type)) {
+                        // e.g. mixed|SomeClass can be null because mixed can be null.
                         $has_null = $has_null || $type->isNullable();
                         continue;
                     }
@@ -583,6 +584,7 @@ private static function createNegationCallbackMap(): array
                 if ($has_null && !$has_other_nullable_types) {
                     $new_type_builder->addType(NullType::instance(false));
                 }
+                // TODO: Infer real type sets as well?
                 $variable->setUnionType($new_type_builder->getPHPDocUnionType());
             };
         };

diff --git a/src/Phan/Analysis/ParameterTypesAnalyzer.php b/src/Phan/Analysis/ParameterTypesAnalyzer.php
@@ -238,7 +238,7 @@ private static function analyzeRealSignatureCompatibility(CodeBase $code_base, F
             foreach ($real_parameter->getUnionType()->getTypeSet() as $type) {
                 $type_class = \get_class($type);
                 if ($php70_checks) {
-                    if ($type->isNullable()) {
+                    if ($type->isNullableLabeled()) {
                         if ($real_parameter->isUsingNullableSyntax()) {
                             Issue::maybeEmit(
                                 $code_base,
@@ -315,15 +315,15 @@ private static function analyzeRealSignatureCompatibility(CodeBase $code_base, F
                             (string)$type
                         );
                     } else {
-                        if ($type->isNullable()) {
+                        if ($type->isNullableLabeled()) {
                             // Don't emit CompatibleNullableTypePHP70 for `void`.
-                                Issue::maybeEmit(
-                                    $code_base,
-                                    $method->getContext(),
-                                    Issue::CompatibleNullableTypePHP70,
-                                    $method->getFileRef()->getLineNumberStart(),
-                                    (string)$type
-                                );
+                            Issue::maybeEmit(
+                                $code_base,
+                                $method->getContext(),
+                                Issue::CompatibleNullableTypePHP70,
+                                $method->getFileRef()->getLineNumberStart(),
+                                (string)$type
+                            );
                         }
                         if ($type_class === IterableType::class) {
                             Issue::maybeEmit(

diff --git a/src/Phan/Analysis/PostOrderAnalysisVisitor.php b/src/Phan/Analysis/PostOrderAnalysisVisitor.php
@@ -47,6 +47,7 @@
 use Phan\Language\Type\LiteralStringType;
 use Phan\Language\Type\MixedType;
 use Phan\Language\Type\NonEmptyMixedType;
+use Phan\Language\Type\NonNullMixedType;
 use Phan\Language\Type\NullType;
 use Phan\Language\Type\ObjectType;
 use Phan\Language\Type\StringType;
@@ -286,7 +287,9 @@ private function analyzeUnsetDim(Node $node): void
             // unset($x[$i]) should convert a list<T> or non-empty-list<T> to an array<Y>
             $union_type = $union_type->withAssociativeArrays(true)->asMappedUnionType(static function (Type $type): Type {
                 if ($type instanceof NonEmptyMixedType) {
-                    return MixedType::instance($type->isNullable());
+                    // convert non-empty-mixed to non-null-mixed because `unset($x[$i])` could have removed the last element of an array,
+                    // but that would still not be null.
+                    return $type->isNullableLabeled() ? MixedType::instance(true) : NonNullMixedType::instance(false);
                 }
                 return $type;
             });
@@ -862,7 +865,7 @@ private function analyzeBinaryShift(Node $node): void
         $this->warnAboutInvalidUnionType(
             $node,
             static function (Type $type): bool {
-                if ($type->isNullable()) {
+                if ($type->isNullableLabeled()) {
                     return false;
                 }
                 if ($type instanceof IntType || $type instanceof MixedType) {
@@ -896,7 +899,7 @@ private function analyzeBinaryBitwiseOp(Node $node): void
         $this->warnAboutInvalidUnionType(
             $node,
             static function (Type $type): bool {
-                if ($type->isNullable()) {
+                if ($type->isNullableLabeled()) {
                     return false;
                 }
                 if ($type instanceof IntType || $type instanceof StringType || $type instanceof MixedType) {
@@ -4579,7 +4582,8 @@ private function updateParameterTypeByArgument(
         $parameter_list[$parameter_offset] = $pass_by_reference_variable;
     }
 
-    private function analyzeArgumentWithConstructorPropertyPromotion(Method $method, Parameter $parameter): void {
+    private function analyzeArgumentWithConstructorPropertyPromotion(Method $method, Parameter $parameter): void
+    {
         if (!$method->isNewConstructor()) {
             return;
         }

diff --git a/src/Phan/Language/Element/Property.php b/src/Phan/Language/Element/Property.php
@@ -429,6 +429,7 @@ public function inheritStaticUnionType(FullyQualifiedClassName $old, FullyQualif
             if (!$type->isObjectWithKnownFQSEN()) {
                 continue;
             }
+            // $type is the name of a class - replace it with $new and preserve nullability.
             if (FullyQualifiedClassName::fromType($type) === $old) {
                 $union_type = $union_type
                     ->withoutType($type)

diff --git a/src/Phan/Language/Type.php b/src/Phan/Language/Type.php
@@ -3107,18 +3107,21 @@ public function isSubtypeOf(Type $type): bool
             return true;
         }
 
-        if ($type instanceof MixedType) {
-            return true;
-        }
-
+        $other_is_nullable = $type->isNullable();
         // A nullable type is not a subtype of a non-nullable type
-        if ($this->is_nullable && !$type->is_nullable) {
+        if ($this->isNullable() && !$other_is_nullable) {
             return false;
         }
 
+        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;
+        }
+
         // Get a non-null version of the type we're comparing
         // against.
-        if ($type->is_nullable) {
+        if ($other_is_nullable) {
             $type = $type->withIsNullable(false);
 
             // Check one more time to see if the types are equal
@@ -3248,6 +3251,7 @@ public function isExclusivelyNarrowedFormOrEquivalentTo(
             return true;
         }
         if ($this->isNullable() && !$union_type->containsNullable()) {
+            // e.g. can't cast ?int to int, mixed to non-null-mixed, etc.
             return false;
         }
         $this_resolved = $this->withStaticResolvedInContext($context);

diff --git a/src/Phan/Language/Type/ArrayType.php b/src/Phan/Language/Type/ArrayType.php
@@ -220,6 +220,7 @@ private static function combineArrayTypeListsOverriding(array $left_types, array
     /**
      * @param non-empty-list<Type> $right_types the original types being added to
      * @return list<ArrayType>
+     * @suppress PhanPartialTypeMismatchArgument UnionType::typeSetFromString is list<Type>
      */
     private static function computeRealTypeSetFromArrayTypeLists(array $right_types, bool $is_assignment): array
     {
@@ -230,6 +231,7 @@ private static function computeRealTypeSetFromArrayTypeLists(array $right_types,
             }
         }
         static $array_type_set;
+        // @phan-suppress-next-line PhanPartialTypeMismatchReturn Type cannot cast to ArrayType
         return $array_type_set ?? ($array_type_set = UnionType::typeSetFromString('array'));
     }
 

diff --git a/src/Phan/Language/Type/LiteralStringType.php b/src/Phan/Language/Type/LiteralStringType.php
@@ -442,6 +442,12 @@ public function asNonTruthyType(): Type
     {
         return $this->value ? NullType::instance(false) : $this;
     }
+
+    /** @override */
+    public function isPossiblyNumeric(): bool
+    {
+        return \is_numeric($this->value);
+    }
 }
 
 LiteralStringType::init();
diff --git a/src/Phan/Language/Type/NonNullMixedType.php b/src/Phan/Language/Type/NonNullMixedType.php
@@ -19,12 +19,16 @@ final class NonNullMixedType extends MixedType
     /** @phan-override */
     public const NAME = 'non-null-mixed';
 
+    /**
+     * @suppress PhanPartialTypeMismatchArgument static::make() is Type, not mixed
+     */
     public static function instance(bool $is_nullable)
     {
         if ($is_nullable) {
             return MixedType::instance(true);
         }
         static $instance = null;
+        // @phan-suppress-next-line PhanPartialTypeMismatchReturn Type can't cast to NonNullMixedType
         return $instance ?? ($instance = static::make('\\', self::NAME, [], false, Type::FROM_NODE));
     }
 

diff --git a/src/Phan/Language/Type/SelfType.php b/src/Phan/Language/Type/SelfType.php
@@ -111,7 +111,8 @@ public function __toString(): string
      * Either this or 'self' resolved in the given context.
      * @suppress PhanAccessReadOnlyProperty
      */
-    public function withStaticResolvedInContext(Context $context): Type {
+    public function withStaticResolvedInContext(Context $context): Type
+    {
         // TODO: Special handling of self<static>, if needed?
         return $this->withSelfResolvedInContext($context);
     }
@@ -123,7 +124,8 @@ public function withStaticResolvedInContext(Context $context): Type {
      *
      * TODO: Handle `(at)return OtherType<self>`
      */
-    public function withSelfResolvedInContext(Context $context): Type {
+    public function withSelfResolvedInContext(Context $context): Type
+    {
         // If the context isn't in a class scope, there's nothing
         // we can do
         if (!$context->isInClassScope()) {

diff --git a/src/Phan/Language/Type/VoidType.php b/src/Phan/Language/Type/VoidType.php
@@ -100,6 +100,7 @@ public function canCastToType(Type $type): bool
             return true;
         }
 
+        // TODO Make this more strict with real types, somehow?
         if (Config::get_null_casts_as_any_type()) {
             return true;
         }