From 9cf410c3342f51bffc1cb7fa134fba485c3cdaef Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 14:00:00 +0100 Subject: [PATCH 01/12] added failling test --- .../InvalidBinaryOperationRuleTest.php | 32 +++++++++++++++++++ .../Rules/Operators/data/invalid-binary.php | 16 ++++++++++ 2 files changed, 48 insertions(+) diff --git a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php index b89c5e985e..db91823346 100644 --- a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php @@ -113,6 +113,38 @@ public function testRule(): void 'Binary operation "+" between stdClass and int results in an error.', 157, ], + [ + 'Binary operation "+" between stdClass and int results in an error.', + 182, + ], + [ + 'Binary operation "-" between stdClass and int results in an error.', + 183, + ], + [ + 'Binary operation "*" between stdClass and int results in an error.', + 184, + ], + [ + 'Binary operation "/" between stdClass and int results in an error.', + 185, + ], + [ + 'Binary operation "+" between string and 10 results in an error.', + 187, + ], + [ + 'Binary operation "-" between string and 10 results in an error.', + 188, + ], + [ + 'Binary operation "*" between string and 10 results in an error.', + 189, + ], + [ + 'Binary operation "/" between string and 10 results in an error.', + 190, + ], ]); } diff --git a/tests/PHPStan/Rules/Operators/data/invalid-binary.php b/tests/PHPStan/Rules/Operators/data/invalid-binary.php index 9ea75a0589..4d69bc8871 100644 --- a/tests/PHPStan/Rules/Operators/data/invalid-binary.php +++ b/tests/PHPStan/Rules/Operators/data/invalid-binary.php @@ -173,3 +173,19 @@ function (array $args) { function (array $args) { isset($args['y']) ? $args + [] : $args; }; + +/** + * @param non-empty-string $foo + * @param string $bar + */ +function bug6624($foo, $bar) { + echo ($foo + 10); + echo ($foo - 10); + echo ($foo * 10); + echo ($foo / 10); + + echo ($bar + 10); + echo ($bar - 10); + echo ($bar * 10); + echo ($bar / 10); +} From 16dc449bbd5b8e9ac2b70e426522ea9b7dc1eb2c Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 14:45:32 +0100 Subject: [PATCH 02/12] more testcoverage --- .../InvalidBinaryOperationRuleTest.php | 48 +++++++++++++++---- .../Rules/Operators/data/invalid-binary.php | 10 ++++ 2 files changed, 50 insertions(+), 8 deletions(-) diff --git a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php index db91823346..be991bdda7 100644 --- a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php @@ -114,37 +114,69 @@ public function testRule(): void 157, ], [ - 'Binary operation "+" between stdClass and int results in an error.', + 'Binary operation "+" between non-empty-string and 10 results in an error.', 182, ], [ - 'Binary operation "-" between stdClass and int results in an error.', + 'Binary operation "-" between non-empty-string and 10 results in an error.', 183, ], [ - 'Binary operation "*" between stdClass and int results in an error.', + 'Binary operation "*" between non-empty-string and 10 results in an error.', 184, ], [ - 'Binary operation "/" between stdClass and int results in an error.', + 'Binary operation "/" between non-empty-string and 10 results in an error.', 185, ], [ - 'Binary operation "+" between string and 10 results in an error.', + 'Binary operation "+" between 10 and non-empty-string results in an error.', 187, ], [ - 'Binary operation "-" between string and 10 results in an error.', + 'Binary operation "-" between 10 and non-empty-string results in an error.', 188, ], [ - 'Binary operation "*" between string and 10 results in an error.', + 'Binary operation "*" between 10 and non-empty-string results in an error.', 189, ], [ - 'Binary operation "/" between string and 10 results in an error.', + 'Binary operation "/" between 10 and non-empty-string results in an error.', 190, ], + [ + 'Binary operation "+" between string and 10 results in an error.', + 192, + ], + [ + 'Binary operation "-" between string and 10 results in an error.', + 193, + ], + [ + 'Binary operation "*" between string and 10 results in an error.', + 194, + ], + [ + 'Binary operation "/" between string and 10 results in an error.', + 195, + ], + [ + 'Binary operation "+" between 10 and string results in an error.', + 197, + ], + [ + 'Binary operation "-" between 10 and string results in an error.', + 198, + ], + [ + 'Binary operation "*" between 10 and string results in an error.', + 199, + ], + [ + 'Binary operation "/" between 10 and string results in an error.', + 200, + ], ]); } diff --git a/tests/PHPStan/Rules/Operators/data/invalid-binary.php b/tests/PHPStan/Rules/Operators/data/invalid-binary.php index 4d69bc8871..2f29a56df7 100644 --- a/tests/PHPStan/Rules/Operators/data/invalid-binary.php +++ b/tests/PHPStan/Rules/Operators/data/invalid-binary.php @@ -184,8 +184,18 @@ function bug6624($foo, $bar) { echo ($foo * 10); echo ($foo / 10); + echo (10 + $foo); + echo (10 - $foo); + echo (10 * $foo); + echo (10 / $foo); + echo ($bar + 10); echo ($bar - 10); echo ($bar * 10); echo ($bar / 10); + + echo (10 + $bar); + echo (10 - $bar); + echo (10 * $bar); + echo (10 / $bar); } From 5e3f7b3f50d42f06c99e3e90ef2f93fd09857f54 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 14:58:09 +0100 Subject: [PATCH 03/12] more testcoverage --- .../Rules/Operators/data/invalid-binary.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/PHPStan/Rules/Operators/data/invalid-binary.php b/tests/PHPStan/Rules/Operators/data/invalid-binary.php index 2f29a56df7..96d31764a4 100644 --- a/tests/PHPStan/Rules/Operators/data/invalid-binary.php +++ b/tests/PHPStan/Rules/Operators/data/invalid-binary.php @@ -178,7 +178,7 @@ function (array $args) { * @param non-empty-string $foo * @param string $bar */ -function bug6624($foo, $bar) { +function bug6624_should_error($foo, $bar) { echo ($foo + 10); echo ($foo - 10); echo ($foo * 10); @@ -199,3 +199,18 @@ function bug6624($foo, $bar) { echo (10 * $bar); echo (10 / $bar); } + +/** + * @param numeric-string $foo + */ +function bug6624_no_error($foo) { + echo ($foo + 10); + echo ($foo - 10); + echo ($foo * 10); + echo ($foo / 10); + + echo (10 + $foo); + echo (10 - $foo); + echo (10 * $foo); + echo (10 / $foo); +} From 3475dce4b1d43a6ca9080df5baa136f2f2b03065 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 15:04:54 +0100 Subject: [PATCH 04/12] type inference coverage --- tests/PHPStan/Analyser/data/bug-6624.php | 31 ++++++++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 tests/PHPStan/Analyser/data/bug-6624.php diff --git a/tests/PHPStan/Analyser/data/bug-6624.php b/tests/PHPStan/Analyser/data/bug-6624.php new file mode 100644 index 0000000000..bbdf3b9395 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-6624.php @@ -0,0 +1,31 @@ + Date: Thu, 10 Feb 2022 15:05:20 +0100 Subject: [PATCH 05/12] fix --- src/Analyser/MutatingScope.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index bbd489a844..a59f99d99d 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1400,6 +1400,11 @@ private function resolveType(Expr $node): Type return new ErrorType(); } + if ($leftType->isNonEmptyString()->yes() && !$leftType->isNumericString()->yes() || + $rightType->isNonEmptyString()->yes() && !$rightType->isNumericString()->yes()) { + return new ErrorType(); + } + $leftNumberType = $leftType->toNumber(); $rightNumberType = $rightType->toNumber(); @@ -1419,6 +1424,11 @@ private function resolveType(Expr $node): Type $resultType = TypeCombinator::union($leftNumberType, $rightNumberType); if ($node instanceof Expr\AssignOp\Div || $node instanceof Expr\BinaryOp\Div) { + // division on strings is not allowed. its allowed on numeric strings though. + if ($leftType instanceof StringType || $rightType instanceof StringType) { + return new ErrorType(); + } + if ($types instanceof MixedType || $resultType instanceof IntegerType) { return new BenevolentUnionType([new IntegerType(), new FloatType()]); } From 955555392138213fd798c9f6ec76b1aef31e34d7 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 15:07:22 +0100 Subject: [PATCH 06/12] fix cs --- src/Analyser/MutatingScope.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index a59f99d99d..435fc90db5 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1425,7 +1425,7 @@ private function resolveType(Expr $node): Type $resultType = TypeCombinator::union($leftNumberType, $rightNumberType); if ($node instanceof Expr\AssignOp\Div || $node instanceof Expr\BinaryOp\Div) { // division on strings is not allowed. its allowed on numeric strings though. - if ($leftType instanceof StringType || $rightType instanceof StringType) { + if ($leftType instanceof StringType || $rightType instanceof StringType) { return new ErrorType(); } From 601f64609f5daf04978c9536e9eff25e193159bb Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 15:18:48 +0100 Subject: [PATCH 07/12] literal-strings should not error --- .../Analyser/NodeScopeResolverTest.php | 1 + .../Rules/Operators/data/invalid-binary.php | 33 ++++++++++++------- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/tests/PHPStan/Analyser/NodeScopeResolverTest.php b/tests/PHPStan/Analyser/NodeScopeResolverTest.php index 088619e961..3535aac6dc 100644 --- a/tests/PHPStan/Analyser/NodeScopeResolverTest.php +++ b/tests/PHPStan/Analyser/NodeScopeResolverTest.php @@ -704,6 +704,7 @@ public function dataFileAsserts(): iterable yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6500.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6488.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/bug-6624.php'); } /** diff --git a/tests/PHPStan/Rules/Operators/data/invalid-binary.php b/tests/PHPStan/Rules/Operators/data/invalid-binary.php index 96d31764a4..d5a79a1400 100644 --- a/tests/PHPStan/Rules/Operators/data/invalid-binary.php +++ b/tests/PHPStan/Rules/Operators/data/invalid-binary.php @@ -201,16 +201,27 @@ function bug6624_should_error($foo, $bar) { } /** - * @param numeric-string $foo + * @param numeric-string $numericString + * @param literal-string $literalString */ -function bug6624_no_error($foo) { - echo ($foo + 10); - echo ($foo - 10); - echo ($foo * 10); - echo ($foo / 10); - - echo (10 + $foo); - echo (10 - $foo); - echo (10 * $foo); - echo (10 / $foo); +function bug6624_no_error($numericString, $literalString) { + echo ($numericString + 10); + echo ($numericString - 10); + echo ($numericString * 10); + echo ($numericString / 10); + + echo (10 + $numericString); + echo (10 - $numericString); + echo (10 * $numericString); + echo (10 / $numericString); + + echo ($literalString + 10); + echo ($literalString - 10); + echo ($literalString * 10); + echo ($literalString / 10); + + echo (10 + $literalString); + echo (10 - $literalString); + echo (10 * $literalString); + echo (10 / $literalString); } From 726e38949e5b80f5e9f73a87a09d6badc3d6f850 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 15:48:09 +0100 Subject: [PATCH 08/12] added `Type->isString()` --- src/Analyser/MutatingScope.php | 2 +- src/Type/Accessory/AccessoryLiteralStringType.php | 5 +++++ src/Type/Accessory/AccessoryNonEmptyStringType.php | 5 +++++ src/Type/Accessory/AccessoryNumericStringType.php | 5 +++++ src/Type/Accessory/HasOffsetType.php | 5 +++++ src/Type/Accessory/NonEmptyArrayType.php | 5 +++++ src/Type/ArrayType.php | 5 +++++ src/Type/CallableType.php | 5 +++++ src/Type/ClassStringType.php | 5 +++++ src/Type/ClosureType.php | 5 +++++ src/Type/Constant/ConstantStringType.php | 5 +++++ src/Type/FloatType.php | 5 +++++ src/Type/IntersectionType.php | 5 +++++ src/Type/IterableType.php | 5 +++++ src/Type/JustNullableTypeTrait.php | 5 +++++ src/Type/MixedType.php | 5 +++++ src/Type/NeverType.php | 5 +++++ src/Type/NullType.php | 5 +++++ src/Type/ObjectType.php | 5 +++++ src/Type/StaticType.php | 5 +++++ src/Type/StrictMixedType.php | 5 +++++ src/Type/StringType.php | 5 +++++ src/Type/Traits/ObjectTypeTrait.php | 5 +++++ src/Type/Type.php | 5 +++++ src/Type/UnionType.php | 5 +++++ src/Type/VoidType.php | 5 +++++ .../Rules/Api/data/class-implements-out-of-phpstan.php | 5 +++++ 27 files changed, 131 insertions(+), 1 deletion(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 435fc90db5..321100fa90 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1425,7 +1425,7 @@ private function resolveType(Expr $node): Type $resultType = TypeCombinator::union($leftNumberType, $rightNumberType); if ($node instanceof Expr\AssignOp\Div || $node instanceof Expr\BinaryOp\Div) { // division on strings is not allowed. its allowed on numeric strings though. - if ($leftType instanceof StringType || $rightType instanceof StringType) { + if ($leftType->isString()->yes() || $rightType->isString()->yes()) { return new ErrorType(); } diff --git a/src/Type/Accessory/AccessoryLiteralStringType.php b/src/Type/Accessory/AccessoryLiteralStringType.php index a271bb2a07..887841fd63 100644 --- a/src/Type/Accessory/AccessoryLiteralStringType.php +++ b/src/Type/Accessory/AccessoryLiteralStringType.php @@ -164,6 +164,11 @@ public function toArray(): Type ); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createMaybe(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createMaybe(); diff --git a/src/Type/Accessory/AccessoryNonEmptyStringType.php b/src/Type/Accessory/AccessoryNonEmptyStringType.php index b8f50792e1..8fa898e033 100644 --- a/src/Type/Accessory/AccessoryNonEmptyStringType.php +++ b/src/Type/Accessory/AccessoryNonEmptyStringType.php @@ -160,6 +160,11 @@ public function toArray(): Type ); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createMaybe(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createMaybe(); diff --git a/src/Type/Accessory/AccessoryNumericStringType.php b/src/Type/Accessory/AccessoryNumericStringType.php index 75d044bb7e..1a2123b833 100644 --- a/src/Type/Accessory/AccessoryNumericStringType.php +++ b/src/Type/Accessory/AccessoryNumericStringType.php @@ -156,6 +156,11 @@ public function toArray(): Type ); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createYes(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createYes(); diff --git a/src/Type/Accessory/HasOffsetType.php b/src/Type/Accessory/HasOffsetType.php index edf56ef6b0..5804ded061 100644 --- a/src/Type/Accessory/HasOffsetType.php +++ b/src/Type/Accessory/HasOffsetType.php @@ -134,6 +134,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createMaybe(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createMaybe(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createMaybe(); diff --git a/src/Type/Accessory/NonEmptyArrayType.php b/src/Type/Accessory/NonEmptyArrayType.php index 57094d492f..9c4419d69b 100644 --- a/src/Type/Accessory/NonEmptyArrayType.php +++ b/src/Type/Accessory/NonEmptyArrayType.php @@ -139,6 +139,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createYes(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/ArrayType.php b/src/Type/ArrayType.php index cf6dda2f9f..82272a1a3e 100644 --- a/src/Type/ArrayType.php +++ b/src/Type/ArrayType.php @@ -199,6 +199,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createYes(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/CallableType.php b/src/Type/CallableType.php index 5b96b285bc..d13f571e2e 100644 --- a/src/Type/CallableType.php +++ b/src/Type/CallableType.php @@ -304,6 +304,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createMaybe(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createMaybe(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/ClassStringType.php b/src/Type/ClassStringType.php index aa79e138dd..f37d5d3ff2 100644 --- a/src/Type/ClassStringType.php +++ b/src/Type/ClassStringType.php @@ -62,6 +62,11 @@ public function isSuperTypeOf(Type $type): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createYes(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createMaybe(); diff --git a/src/Type/ClosureType.php b/src/Type/ClosureType.php index f2ccf89e93..ed82d34117 100644 --- a/src/Type/ClosureType.php +++ b/src/Type/ClosureType.php @@ -381,6 +381,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/Constant/ConstantStringType.php b/src/Type/Constant/ConstantStringType.php index 8fdab07c8d..1407a7f66e 100644 --- a/src/Type/Constant/ConstantStringType.php +++ b/src/Type/Constant/ConstantStringType.php @@ -240,6 +240,11 @@ public function toFloat(): Type return new ConstantFloatType((float) $this->value); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createYes(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createFromBoolean(is_numeric($this->getValue())); diff --git a/src/Type/FloatType.php b/src/Type/FloatType.php index 9b14721832..790752ed57 100644 --- a/src/Type/FloatType.php +++ b/src/Type/FloatType.php @@ -118,6 +118,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/IntersectionType.php b/src/Type/IntersectionType.php index 87c80d8b6f..90144aa5e1 100644 --- a/src/Type/IntersectionType.php +++ b/src/Type/IntersectionType.php @@ -349,6 +349,11 @@ public function isArray(): TrinaryLogic return $this->intersectResults(static fn (Type $type): TrinaryLogic => $type->isArray()); } + public function isString(): TrinaryLogic + { + return $this->intersectResults(static fn (Type $type): TrinaryLogic => $type->isString()); + } + public function isNumericString(): TrinaryLogic { return $this->intersectResults(static fn (Type $type): TrinaryLogic => $type->isNumericString()); diff --git a/src/Type/IterableType.php b/src/Type/IterableType.php index 8eeb62d74a..a9e4c7e0d1 100644 --- a/src/Type/IterableType.php +++ b/src/Type/IterableType.php @@ -228,6 +228,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createMaybe(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/JustNullableTypeTrait.php b/src/Type/JustNullableTypeTrait.php index f3f2847495..e1c10120b1 100644 --- a/src/Type/JustNullableTypeTrait.php +++ b/src/Type/JustNullableTypeTrait.php @@ -58,6 +58,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/MixedType.php b/src/Type/MixedType.php index 7c130546a3..1213e3fc42 100644 --- a/src/Type/MixedType.php +++ b/src/Type/MixedType.php @@ -405,6 +405,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createMaybe(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createMaybe(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createMaybe(); diff --git a/src/Type/NeverType.php b/src/Type/NeverType.php index 0fa866a387..440de1a262 100644 --- a/src/Type/NeverType.php +++ b/src/Type/NeverType.php @@ -231,6 +231,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/NullType.php b/src/Type/NullType.php index 29f15f0b20..f86b990074 100644 --- a/src/Type/NullType.php +++ b/src/Type/NullType.php @@ -175,6 +175,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/ObjectType.php b/src/Type/ObjectType.php index a615e24977..671bb56205 100644 --- a/src/Type/ObjectType.php +++ b/src/Type/ObjectType.php @@ -779,6 +779,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/StaticType.php b/src/Type/StaticType.php index 6d681bd720..18599c356e 100644 --- a/src/Type/StaticType.php +++ b/src/Type/StaticType.php @@ -332,6 +332,11 @@ public function isArray(): TrinaryLogic return $this->getStaticObjectType()->isArray(); } + public function isString(): TrinaryLogic + { + return $this->getStaticObjectType()->isString(); + } + public function isNumericString(): TrinaryLogic { return $this->getStaticObjectType()->isNumericString(); diff --git a/src/Type/StrictMixedType.php b/src/Type/StrictMixedType.php index f98580057f..09e00d0bd0 100644 --- a/src/Type/StrictMixedType.php +++ b/src/Type/StrictMixedType.php @@ -151,6 +151,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/StringType.php b/src/Type/StringType.php index cf47790a72..8abcad2769 100644 --- a/src/Type/StringType.php +++ b/src/Type/StringType.php @@ -133,6 +133,11 @@ public function toArray(): Type ); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createYes(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createMaybe(); diff --git a/src/Type/Traits/ObjectTypeTrait.php b/src/Type/Traits/ObjectTypeTrait.php index 40f6ef6a60..be20a00f5e 100644 --- a/src/Type/Traits/ObjectTypeTrait.php +++ b/src/Type/Traits/ObjectTypeTrait.php @@ -105,6 +105,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/src/Type/Type.php b/src/Type/Type.php index 24a17d63c5..ab1e6a8161 100644 --- a/src/Type/Type.php +++ b/src/Type/Type.php @@ -98,6 +98,11 @@ public function isSmallerThan(Type $otherType): TrinaryLogic; public function isSmallerThanOrEqual(Type $otherType): TrinaryLogic; + /** + * Whether the type is a plain string, and not one of the further specialized string-types. + */ + public function isString(): TrinaryLogic; + public function isNumericString(): TrinaryLogic; public function isNonEmptyString(): TrinaryLogic; diff --git a/src/Type/UnionType.php b/src/Type/UnionType.php index d7cd5380ae..b3a95c4231 100644 --- a/src/Type/UnionType.php +++ b/src/Type/UnionType.php @@ -391,6 +391,11 @@ public function isArray(): TrinaryLogic return $this->unionResults(static fn (Type $type): TrinaryLogic => $type->isArray()); } + public function isString(): TrinaryLogic + { + return $this->unionResults(static fn (Type $type): TrinaryLogic => $type->isString()); + } + public function isNumericString(): TrinaryLogic { return $this->unionResults(static fn (Type $type): TrinaryLogic => $type->isNumericString()); diff --git a/src/Type/VoidType.php b/src/Type/VoidType.php index 0c40218abe..166ca12ee4 100644 --- a/src/Type/VoidType.php +++ b/src/Type/VoidType.php @@ -100,6 +100,11 @@ public function isArray(): TrinaryLogic return TrinaryLogic::createNo(); } + public function isString(): TrinaryLogic + { + return TrinaryLogic::createNo(); + } + public function isNumericString(): TrinaryLogic { return TrinaryLogic::createNo(); diff --git a/tests/PHPStan/Rules/Api/data/class-implements-out-of-phpstan.php b/tests/PHPStan/Rules/Api/data/class-implements-out-of-phpstan.php index 91dbb321c5..ddcbbe3fb8 100644 --- a/tests/PHPStan/Rules/Api/data/class-implements-out-of-phpstan.php +++ b/tests/PHPStan/Rules/Api/data/class-implements-out-of-phpstan.php @@ -234,6 +234,11 @@ public function isSmallerThanOrEqual(Type $otherType): \PHPStan\TrinaryLogic // TODO: Implement isSmallerThanOrEqual() method. } + public function isString(): \PHPStan\TrinaryLogic + { + // TODO: Implement isString() method. + } + public function isNumericString(): \PHPStan\TrinaryLogic { // TODO: Implement isNumericString() method. From 47f84844c2d4c4380fe9a5dd846ad62687442d68 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 15:54:11 +0100 Subject: [PATCH 09/12] fix --- src/Analyser/MutatingScope.php | 3 ++- src/Type/Accessory/AccessoryLiteralStringType.php | 2 +- src/Type/Accessory/AccessoryNonEmptyStringType.php | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 321100fa90..c537c263de 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1425,7 +1425,8 @@ private function resolveType(Expr $node): Type $resultType = TypeCombinator::union($leftNumberType, $rightNumberType); if ($node instanceof Expr\AssignOp\Div || $node instanceof Expr\BinaryOp\Div) { // division on strings is not allowed. its allowed on numeric strings though. - if ($leftType->isString()->yes() || $rightType->isString()->yes()) { + if ($leftType->isString()->yes() && !$leftType->isNumericString()->yes() + || $rightType->isString()->yes() && !$rightType->isNumericString()->yes()) { return new ErrorType(); } diff --git a/src/Type/Accessory/AccessoryLiteralStringType.php b/src/Type/Accessory/AccessoryLiteralStringType.php index 887841fd63..ad874199e6 100644 --- a/src/Type/Accessory/AccessoryLiteralStringType.php +++ b/src/Type/Accessory/AccessoryLiteralStringType.php @@ -166,7 +166,7 @@ public function toArray(): Type public function isString(): TrinaryLogic { - return TrinaryLogic::createMaybe(); + return TrinaryLogic::createYes(); } public function isNumericString(): TrinaryLogic diff --git a/src/Type/Accessory/AccessoryNonEmptyStringType.php b/src/Type/Accessory/AccessoryNonEmptyStringType.php index 8fa898e033..048a1d946e 100644 --- a/src/Type/Accessory/AccessoryNonEmptyStringType.php +++ b/src/Type/Accessory/AccessoryNonEmptyStringType.php @@ -162,7 +162,7 @@ public function toArray(): Type public function isString(): TrinaryLogic { - return TrinaryLogic::createMaybe(); + return TrinaryLogic::createYes(); } public function isNumericString(): TrinaryLogic From 6b5f0894a8fee8694b2898e465665d406f729d02 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 16:00:22 +0100 Subject: [PATCH 10/12] more testcoverage --- src/Analyser/MutatingScope.php | 4 +- .../InvalidBinaryOperationRuleTest.php | 64 ++++++++++++++----- .../Rules/Operators/data/invalid-binary.php | 13 +++- 3 files changed, 62 insertions(+), 19 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index c537c263de..0ac6665cc9 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1425,8 +1425,8 @@ private function resolveType(Expr $node): Type $resultType = TypeCombinator::union($leftNumberType, $rightNumberType); if ($node instanceof Expr\AssignOp\Div || $node instanceof Expr\BinaryOp\Div) { // division on strings is not allowed. its allowed on numeric strings though. - if ($leftType->isString()->yes() && !$leftType->isNumericString()->yes() - || $rightType->isString()->yes() && !$rightType->isNumericString()->yes()) { + if ($leftType->isString()->yes() && !$leftType->isNumericString()->yes() && !$leftType->isLiteralString()->yes() + || $rightType->isString()->yes() && !$rightType->isNumericString()->yes() && !$rightType->isLiteralString()->yes()) { return new ErrorType(); } diff --git a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php index be991bdda7..920c371f36 100644 --- a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php @@ -115,67 +115,99 @@ public function testRule(): void ], [ 'Binary operation "+" between non-empty-string and 10 results in an error.', - 182, + 183, ], [ 'Binary operation "-" between non-empty-string and 10 results in an error.', - 183, + 184, ], [ 'Binary operation "*" between non-empty-string and 10 results in an error.', - 184, + 185, ], [ 'Binary operation "/" between non-empty-string and 10 results in an error.', - 185, + 186, ], [ 'Binary operation "+" between 10 and non-empty-string results in an error.', - 187, + 188, ], [ 'Binary operation "-" between 10 and non-empty-string results in an error.', - 188, + 189, ], [ 'Binary operation "*" between 10 and non-empty-string results in an error.', - 189, + 190, ], [ 'Binary operation "/" between 10 and non-empty-string results in an error.', - 190, + 191, ], [ 'Binary operation "+" between string and 10 results in an error.', - 192, + 193, ], [ 'Binary operation "-" between string and 10 results in an error.', - 193, + 194, ], [ 'Binary operation "*" between string and 10 results in an error.', - 194, + 195, ], [ 'Binary operation "/" between string and 10 results in an error.', - 195, + 196, ], [ 'Binary operation "+" between 10 and string results in an error.', - 197, + 198, ], [ 'Binary operation "-" between 10 and string results in an error.', - 198, + 199, ], [ 'Binary operation "*" between 10 and string results in an error.', - 199, + 200, ], [ 'Binary operation "/" between 10 and string results in an error.', - 200, + 201, + ], + [ + 'Binary operation "+" between class-string and 10 results in an error.', + 203, + ], + [ + 'Binary operation "-" between class-string and 10 results in an error.', + 204, + ], + [ + 'Binary operation "*" between class-string and 10 results in an error.', + 205, + ], + [ + 'Binary operation "/" between class-string and 10 results in an error.', + 206, + ], + [ + 'Binary operation "+" between 10 and class-string results in an error.', + 208, + ], + [ + 'Binary operation "-" between 10 and class-string results in an error.', + 209, + ], + [ + 'Binary operation "*" between 10 and class-string results in an error.', + 210, + ], + [ + 'Binary operation "/" between 10 and class-string results in an error.', + 211, ], ]); } diff --git a/tests/PHPStan/Rules/Operators/data/invalid-binary.php b/tests/PHPStan/Rules/Operators/data/invalid-binary.php index d5a79a1400..a5d0ed6325 100644 --- a/tests/PHPStan/Rules/Operators/data/invalid-binary.php +++ b/tests/PHPStan/Rules/Operators/data/invalid-binary.php @@ -177,8 +177,9 @@ function (array $args) { /** * @param non-empty-string $foo * @param string $bar + * @param class-string $foobar */ -function bug6624_should_error($foo, $bar) { +function bug6624_should_error($foo, $bar, $foobar) { echo ($foo + 10); echo ($foo - 10); echo ($foo * 10); @@ -198,6 +199,16 @@ function bug6624_should_error($foo, $bar) { echo (10 - $bar); echo (10 * $bar); echo (10 / $bar); + + echo ($foobar + 10); + echo ($foobar - 10); + echo ($foobar * 10); + echo ($foobar / 10); + + echo (10 + $foobar); + echo (10 - $foobar); + echo (10 * $foobar); + echo (10 / $foobar); } /** From 57669312a5d76f8930c918466c2008de91378140 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 17:43:14 +0100 Subject: [PATCH 11/12] more testcoverage. simplify fix --- src/Analyser/MutatingScope.php | 10 +-- .../InvalidBinaryOperationRuleTest.php | 80 +++++++++++++------ .../Rules/Operators/data/invalid-binary.php | 34 +++++--- 3 files changed, 81 insertions(+), 43 deletions(-) diff --git a/src/Analyser/MutatingScope.php b/src/Analyser/MutatingScope.php index 0ac6665cc9..1149594fcd 100644 --- a/src/Analyser/MutatingScope.php +++ b/src/Analyser/MutatingScope.php @@ -1400,8 +1400,8 @@ private function resolveType(Expr $node): Type return new ErrorType(); } - if ($leftType->isNonEmptyString()->yes() && !$leftType->isNumericString()->yes() || - $rightType->isNonEmptyString()->yes() && !$rightType->isNumericString()->yes()) { + if (($leftType->isString()->yes() && !$leftType->isNumericString()->yes()) + || ($rightType->isString()->yes() && !$rightType->isNumericString()->yes())) { return new ErrorType(); } @@ -1424,12 +1424,6 @@ private function resolveType(Expr $node): Type $resultType = TypeCombinator::union($leftNumberType, $rightNumberType); if ($node instanceof Expr\AssignOp\Div || $node instanceof Expr\BinaryOp\Div) { - // division on strings is not allowed. its allowed on numeric strings though. - if ($leftType->isString()->yes() && !$leftType->isNumericString()->yes() && !$leftType->isLiteralString()->yes() - || $rightType->isString()->yes() && !$rightType->isNumericString()->yes() && !$rightType->isLiteralString()->yes()) { - return new ErrorType(); - } - if ($types instanceof MixedType || $resultType instanceof IntegerType) { return new BenevolentUnionType([new IntegerType(), new FloatType()]); } diff --git a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php index 920c371f36..0e88a2e23b 100644 --- a/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php +++ b/tests/PHPStan/Rules/Operators/InvalidBinaryOperationRuleTest.php @@ -115,99 +115,131 @@ public function testRule(): void ], [ 'Binary operation "+" between non-empty-string and 10 results in an error.', - 183, + 184, ], [ 'Binary operation "-" between non-empty-string and 10 results in an error.', - 184, + 185, ], [ 'Binary operation "*" between non-empty-string and 10 results in an error.', - 185, + 186, ], [ 'Binary operation "/" between non-empty-string and 10 results in an error.', - 186, + 187, ], [ 'Binary operation "+" between 10 and non-empty-string results in an error.', - 188, + 189, ], [ 'Binary operation "-" between 10 and non-empty-string results in an error.', - 189, + 190, ], [ 'Binary operation "*" between 10 and non-empty-string results in an error.', - 190, + 191, ], [ 'Binary operation "/" between 10 and non-empty-string results in an error.', - 191, + 192, ], [ 'Binary operation "+" between string and 10 results in an error.', - 193, + 194, ], [ 'Binary operation "-" between string and 10 results in an error.', - 194, + 195, ], [ 'Binary operation "*" between string and 10 results in an error.', - 195, + 196, ], [ 'Binary operation "/" between string and 10 results in an error.', - 196, + 197, ], [ 'Binary operation "+" between 10 and string results in an error.', - 198, + 199, ], [ 'Binary operation "-" between 10 and string results in an error.', - 199, + 200, ], [ 'Binary operation "*" between 10 and string results in an error.', - 200, + 201, ], [ 'Binary operation "/" between 10 and string results in an error.', - 201, + 202, ], [ 'Binary operation "+" between class-string and 10 results in an error.', - 203, + 204, ], [ 'Binary operation "-" between class-string and 10 results in an error.', - 204, + 205, ], [ 'Binary operation "*" between class-string and 10 results in an error.', - 205, + 206, ], [ 'Binary operation "/" between class-string and 10 results in an error.', - 206, + 207, ], [ 'Binary operation "+" between 10 and class-string results in an error.', - 208, + 209, ], [ 'Binary operation "-" between 10 and class-string results in an error.', - 209, + 210, ], [ 'Binary operation "*" between 10 and class-string results in an error.', - 210, + 211, ], [ 'Binary operation "/" between 10 and class-string results in an error.', - 211, + 212, + ], + [ + 'Binary operation "+" between literal-string and 10 results in an error.', + 214, + ], + [ + 'Binary operation "-" between literal-string and 10 results in an error.', + 215, + ], + [ + 'Binary operation "*" between literal-string and 10 results in an error.', + 216, + ], + [ + 'Binary operation "/" between literal-string and 10 results in an error.', + 217, + ], + [ + 'Binary operation "+" between 10 and literal-string results in an error.', + 219, + ], + [ + 'Binary operation "-" between 10 and literal-string results in an error.', + 220, + ], + [ + 'Binary operation "*" between 10 and literal-string results in an error.', + 221, + ], + [ + 'Binary operation "/" between 10 and literal-string results in an error.', + 222, ], ]); } diff --git a/tests/PHPStan/Rules/Operators/data/invalid-binary.php b/tests/PHPStan/Rules/Operators/data/invalid-binary.php index a5d0ed6325..c144298b6d 100644 --- a/tests/PHPStan/Rules/Operators/data/invalid-binary.php +++ b/tests/PHPStan/Rules/Operators/data/invalid-binary.php @@ -178,8 +178,9 @@ function (array $args) { * @param non-empty-string $foo * @param string $bar * @param class-string $foobar + * @param literal-string $literalString */ -function bug6624_should_error($foo, $bar, $foobar) { +function bug6624_should_error($foo, $bar, $foobar, $literalString) { echo ($foo + 10); echo ($foo - 10); echo ($foo * 10); @@ -209,13 +210,22 @@ function bug6624_should_error($foo, $bar, $foobar) { echo (10 - $foobar); echo (10 * $foobar); echo (10 / $foobar); + + echo ($literalString + 10); + echo ($literalString - 10); + echo ($literalString * 10); + echo ($literalString / 10); + + echo (10 + $literalString); + echo (10 - $literalString); + echo (10 * $literalString); + echo (10 / $literalString); } /** * @param numeric-string $numericString - * @param literal-string $literalString */ -function bug6624_no_error($numericString, $literalString) { +function bug6624_no_error($numericString) { echo ($numericString + 10); echo ($numericString - 10); echo ($numericString * 10); @@ -226,13 +236,15 @@ function bug6624_no_error($numericString, $literalString) { echo (10 * $numericString); echo (10 / $numericString); - echo ($literalString + 10); - echo ($literalString - 10); - echo ($literalString * 10); - echo ($literalString / 10); + $numericLiteral = "123"; - echo (10 + $literalString); - echo (10 - $literalString); - echo (10 * $literalString); - echo (10 / $literalString); + echo ($numericLiteral + 10); + echo ($numericLiteral - 10); + echo ($numericLiteral * 10); + echo ($numericLiteral / 10); + + echo (10 + $numericLiteral); + echo (10 - $numericLiteral); + echo (10 * $numericLiteral); + echo (10 / $numericLiteral); } From e2341fb22fd4342ae052237173c1f24ac4252879 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Thu, 10 Feb 2022 17:49:40 +0100 Subject: [PATCH 12/12] removed comment initially I had a different assumption what this method would provide --- src/Type/Type.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/Type/Type.php b/src/Type/Type.php index ab1e6a8161..5c0b727d7c 100644 --- a/src/Type/Type.php +++ b/src/Type/Type.php @@ -98,9 +98,6 @@ public function isSmallerThan(Type $otherType): TrinaryLogic; public function isSmallerThanOrEqual(Type $otherType): TrinaryLogic; - /** - * Whether the type is a plain string, and not one of the further specialized string-types. - */ public function isString(): TrinaryLogic; public function isNumericString(): TrinaryLogic;