!empty() leads to non-empty-string

phpstan · Jul 7, 2021 · 790a838 · 790a838
1 parent 3af1728
commit 790a838
Show file tree

Hide file tree

Showing 7 changed files with 67 additions and 58 deletions.
diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php
@@ -24,7 +24,6 @@
 use PHPStan\Type\Accessory\HasOffsetType;
 use PHPStan\Type\Accessory\HasPropertyType;
 use PHPStan\Type\Accessory\NonEmptyArrayType;
-use PHPStan\Type\ArrayType;
 use PHPStan\Type\BooleanType;
 use PHPStan\Type\Constant\ConstantBooleanType;
 use PHPStan\Type\Constant\ConstantIntegerType;
@@ -675,20 +674,12 @@ public function specifyTypesInCondition(
 
 			return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context);
 		} elseif (
-			(
-				$expr instanceof Expr\Isset_
-				&& count($expr->vars) > 0
-				&& $context->true()
-			)
-			|| ($expr instanceof Expr\Empty_ && $context->false())
+			$expr instanceof Expr\Isset_
+			&& count($expr->vars) > 0
+			&& $context->true()
 		) {
 			$vars = [];
-			if ($expr instanceof Expr\Isset_) {
-				$varsToIterate = $expr->vars;
-			} else {
-				$varsToIterate = [$expr->expr];
-			}
-			foreach ($varsToIterate as $var) {
+			foreach ($expr->vars as $var) {
 				$tmpVars = [$var];
 
 				while (
@@ -722,21 +713,6 @@ public function specifyTypesInCondition(
 						return new SpecifiedTypes([], []);
 					}
 				}
-				if ($expr instanceof Expr\Isset_) {
-					$type = $this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope);
-				} else {
-					$type = $this->create(
-						$var,
-						new UnionType([
-							new NullType(),
-							new ConstantBooleanType(false),
-						]),
-						TypeSpecifierContext::createFalse(),
-						false,
-						$scope
-					);
-				}
-
 				if (
 					$var instanceof ArrayDimFetch
 					&& $var->dim !== null
@@ -749,8 +725,10 @@ public function specifyTypesInCondition(
 						false,
 						$scope
 					)->unionWith(
-						$type
+						$this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope)
 					);
+				} else {
+					$type = $this->create($var, new NullType(), TypeSpecifierContext::createFalse(), false, $scope);
 				}
 
 				if (
@@ -779,14 +757,6 @@ public function specifyTypesInCondition(
 				}
 			}
 
-			if (
-				$expr instanceof Expr\Empty_
-				&& (new ArrayType(new MixedType(), new MixedType()))->isSuperTypeOf($scope->getType($expr->expr))->yes()) {
-				$types = $types->unionWith(
-					$this->create($expr->expr, new NonEmptyArrayType(), $context->negate(), false, $scope)
-				);
-			}
-
 			return $types;
 		} elseif (
 			$expr instanceof Expr\BinaryOp\Coalesce
@@ -801,10 +771,12 @@ public function specifyTypesInCondition(
 				$scope
 			);
 		} elseif (
-			$expr instanceof Expr\Empty_ && $context->truthy()
-			&& (new ArrayType(new MixedType(), new MixedType()))->isSuperTypeOf($scope->getType($expr->expr))->yes()
+			$expr instanceof Expr\Empty_
 		) {
-			return $this->create($expr->expr, new NonEmptyArrayType(), $context->negate(), false, $scope);
+			return $this->specifyTypesInCondition($scope, new BooleanOr(
+				new Expr\BooleanNot(new Expr\Isset_([$expr->expr])),
+				new Expr\BooleanNot($expr->expr)
+			), $context);
 		} elseif ($expr instanceof Expr\ErrorSuppress) {
 			return $this->specifyTypesInCondition($scope, $expr->expr, $context);
 		} elseif (

diff --git a/tests/PHPStan/Analyser/TypeSpecifierTest.php b/tests/PHPStan/Analyser/TypeSpecifierTest.php
@@ -533,11 +533,9 @@ public function dataCondition(): array
 			[
 				new Expr\BooleanNot(new Expr\Empty_(new Variable('stringOrNull'))),
 				[
-					'$stringOrNull' => '~false|null',
-				],
-				[
-					'empty($stringOrNull)' => self::SURE_NOT_FALSEY,
+					'$stringOrNull' => '~0|0.0|\'\'|\'0\'|array()|false|null',
 				],
+				[],
 			],
 			[
 				new Expr\BinaryOp\Identical(
@@ -552,21 +550,17 @@ public function dataCondition(): array
 			],
 			[
 				new Expr\Empty_(new Variable('array')),
+				[],
 				[
-					'$array' => '~nonEmpty',
-				],
-				[
-					'$array' => 'nonEmpty & ~false|null',
+					'$array' => '~0|0.0|\'\'|\'0\'|array()|false|null',
 				],
 			],
 			[
 				new BooleanNot(new Expr\Empty_(new Variable('array'))),
 				[
-					'$array' => 'nonEmpty & ~false|null',
-				],
-				[
-					'$array' => '~nonEmpty',
+					'$array' => '~0|0.0|\'\'|\'0\'|array()|false|null',
 				],
+				[],
 			],
 			[
 				new FuncCall(new Name('count'), [

diff --git a/tests/PHPStan/Analyser/data/bug-3991.php b/tests/PHPStan/Analyser/data/bug-3991.php
@@ -22,7 +22,7 @@ public static function email($config = null)
 			assertType('array|stdClass|null', $config);
 			$config = new \stdClass();
 		} elseif (! (is_array($config) || $config instanceof \stdClass)) {
-			assertNativeType('mixed~array|stdClass|false|null', $config);
+			assertNativeType('mixed~0|0.0|\'\'|\'0\'|array()|stdClass|false|null', $config);
 			assertType('*NEVER*', $config);
 		}
 

diff --git a/tests/PHPStan/Analyser/data/non-empty-string.php b/tests/PHPStan/Analyser/data/non-empty-string.php
@@ -113,4 +113,20 @@ public function doWithNumeric(string $s): void
 		assertType('non-empty-string', $s);
 	}
 
+	public function doEmpty(string $s): void
+	{
+		if (empty($s)) {
+			return;
+		}
+
+		assertType('non-empty-string', $s);
+	}
+
+	public function doEmpty2(string $s): void
+	{
+		if (!empty($s)) {
+			assertType('non-empty-string', $s);
+		}
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php b/tests/PHPStan/Rules/Methods/CallMethodsRuleTest.php
@@ -1987,4 +1987,12 @@ public function testBug4844(): void
 		$this->analyse([__DIR__ . '/data/bug-4844.php'], []);
 	}
 
+	public function testBug5258(): void
+	{
+		$this->checkThisOnly = false;
+		$this->checkNullables = true;
+		$this->checkUnionTypes = true;
+		$this->analyse([__DIR__ . '/data/bug-5258.php'], []);
+	}
+
 }
diff --git a/tests/PHPStan/Rules/Methods/data/bug-5258.php b/tests/PHPStan/Rules/Methods/data/bug-5258.php
@@ -0,0 +1,23 @@
+<?php declare(strict_types = 1);
+
+namespace Bug5258;
+
+class HelloWorld
+{
+	/**
+	 * @param array{some_key?:string, other_key:string} $params
+	 */
+	public function method1(array $params): void
+	{
+		if (!empty($params['some_key'])) $this->method2($params);
+
+		if (!empty($params['other_key'])) $this->method2($params);
+	}
+
+	/**
+	 * @param array{other_key:string} $params
+	 **/
+	public function method2(array$params): void
+	{
+	}
+}
diff --git a/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php b/tests/PHPStan/Rules/Variables/DefinedVariableRuleTest.php
@@ -107,16 +107,12 @@ public function testDefinedVariables(): void
 				'Undefined variable: $variableInEmpty',
 				145,
 			],
-			[
-				'Undefined variable: $negatedVariableInEmpty',
-				152,
-			],
 			[
 				'Undefined variable: $variableInEmpty',
 				155,
 			],
 			[
-				'Undefined variable: $negatedVariableInEmpty',
+				'Variable $negatedVariableInEmpty might not be defined.',
 				156,
 			],
 			[