Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Binary operations on non-empty-string and string should error #1010

Merged
merged 12 commits into from Feb 10, 2022
11 changes: 11 additions & 0 deletions src/Analyser/MutatingScope.php
Expand Up @@ -1400,6 +1400,11 @@ private function resolveType(Expr $node): Type
return new ErrorType();
}

if ($leftType->isNonEmptyString()->yes() && !$leftType->isNumericString()->yes() ||
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. || vs. && operator priority needs to be disambiguated with parentheses
  2. Why isNonEmptyString and not isString?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. fixed
  2. I used non-empty-string because the initial bug report was about non-emptry-string problems, while it already worked for regular string.
    I am now using isString instead of isNonEmptyString in the first block, which renders this second block of changes unnecessary. I removed it.

$rightType->isNonEmptyString()->yes() && !$rightType->isNumericString()->yes()) {
return new ErrorType();
}

$leftNumberType = $leftType->toNumber();
$rightNumberType = $rightType->toNumber();

Expand All @@ -1419,6 +1424,12 @@ 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()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. || vs. && operator priority needs to be disambiguated with parentheses
  2. I don't know why literal-string is mentioned in this condition

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. fixed
  2. I mixed up my mental model in "123" would be a literal-string, but it is also a numeric-string, so the additional check is not necessary. fixed a bug so we don't accept literal-string for binary operations

return new ErrorType();
}

if ($types instanceof MixedType || $resultType instanceof IntegerType) {
return new BenevolentUnionType([new IntegerType(), new FloatType()]);
}
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Accessory/AccessoryLiteralStringType.php
Expand Up @@ -164,6 +164,11 @@ public function toArray(): Type
);
}

public function isString(): TrinaryLogic
{
return TrinaryLogic::createYes();
}

public function isNumericString(): TrinaryLogic
{
return TrinaryLogic::createMaybe();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Accessory/AccessoryNonEmptyStringType.php
Expand Up @@ -160,6 +160,11 @@ public function toArray(): Type
);
}

public function isString(): TrinaryLogic
{
return TrinaryLogic::createYes();
}

public function isNumericString(): TrinaryLogic
{
return TrinaryLogic::createMaybe();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Accessory/AccessoryNumericStringType.php
Expand Up @@ -156,6 +156,11 @@ public function toArray(): Type
);
}

public function isString(): TrinaryLogic
{
return TrinaryLogic::createYes();
}

public function isNumericString(): TrinaryLogic
{
return TrinaryLogic::createYes();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Accessory/HasOffsetType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Accessory/NonEmptyArrayType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/ArrayType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/CallableType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/ClassStringType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/ClosureType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Constant/ConstantStringType.php
Expand Up @@ -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()));
Expand Down
5 changes: 5 additions & 0 deletions src/Type/FloatType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/IntersectionType.php
Expand Up @@ -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());
Expand Down
5 changes: 5 additions & 0 deletions src/Type/IterableType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/JustNullableTypeTrait.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/MixedType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/NeverType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/NullType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/ObjectType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/StaticType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/StrictMixedType.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/StringType.php
Expand Up @@ -133,6 +133,11 @@ public function toArray(): Type
);
}

public function isString(): TrinaryLogic
{
return TrinaryLogic::createYes();
}

public function isNumericString(): TrinaryLogic
{
return TrinaryLogic::createMaybe();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Traits/ObjectTypeTrait.php
Expand Up @@ -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();
Expand Down
5 changes: 5 additions & 0 deletions src/Type/Type.php
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions src/Type/UnionType.php
Expand Up @@ -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());
Expand Down
5 changes: 5 additions & 0 deletions src/Type/VoidType.php
Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions tests/PHPStan/Analyser/NodeScopeResolverTest.php
Expand Up @@ -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');
}

/**
Expand Down
31 changes: 31 additions & 0 deletions tests/PHPStan/Analyser/data/bug-6624.php
@@ -0,0 +1,31 @@
<?php

namespace Bug6624;

use function PHPStan\Testing\assertType;

/**
* @param non-empty-string $foo
* @param string $bar
*/
function bug6624_should_error($foo, $bar) {
assertType('*ERROR*',$foo + 10);
assertType('*ERROR*',$foo - 10);
assertType('*ERROR*',$foo * 10);
assertType('*ERROR*',$foo / 10);

assertType('*ERROR*',10 + $foo);
assertType('*ERROR*',10 - $foo);
assertType('*ERROR*',10 * $foo);
assertType('*ERROR*',10 / $foo);

assertType('*ERROR*',$bar + 10);
assertType('*ERROR*',$bar - 10);
assertType('*ERROR*',$bar * 10);
assertType('*ERROR*',$bar / 10);

assertType('*ERROR*',10 + $bar);
assertType('*ERROR*',10 - $bar);
assertType('*ERROR*',10 * $bar);
assertType('*ERROR*',10 / $bar);
}
Expand Up @@ -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.
Expand Down