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.

Sign up for GitHub

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

Jump to bottom

Fix native type on unset #2107

Merged
merged 3 commits into from Dec 14, 2022
Merged

Fix native type on unset #2107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
293f39a
fix native-expressions test to use assertNativeType correctly
rajyan Dec 13, 2022
be666f4
add failing test for native type unset
rajyan Dec 13, 2022
56d7785
set correct native types on unset
rajyan Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/Analyser/MutatingScope.php
View file
Open in desktop
Expand Up @@ -3352,8 +3352,8 @@ public function unsetExpression(Expr $expr): self
$exprVarType = $scope->getType($expr->var);
$dimType = $scope->getType($expr->dim);
$unsetType = $exprVarType->unsetOffset($dimType);
$exprVarNativeType = $scope->getType($expr->var);
$dimNativeType = $scope->getType($expr->dim);
$exprVarNativeType = $scope->getNativeType($expr->var);
$dimNativeType = $scope->getNativeType($expr->dim);
$unsetNativeType = $exprVarNativeType->unsetOffset($dimNativeType);
$scope = $scope->assignExpression($expr->var, $unsetType, $unsetNativeType)->invalidateExpression(
new FuncCall(new FullyQualified('count'), [new Arg($expr->var)]),
Expand Down
17 changes: 15 additions & 2 deletions tests/PHPStan/Analyser/data/native-expressions.php
View file
Open in desktop
Expand Up @@ -3,6 +3,7 @@
namespace NativeExpressions;

use function PHPStan\Testing\assertType;
use function PHPStan\Testing\assertNativeType;
Comment on lines 5 to +6
Copy link
Contributor

Choose a reason for hiding this comment

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

@ondrejmirtes wdyt about adding a rule/check which verifies that tests included by TypeInferenceTestCase contain proper full-qualified method calls or imports when assertType or assertNativeType is contained?

Copy link
Member

Choose a reason for hiding this comment

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

@staabm We can make TypeInferenceTestCase crash on the supported functions with correct names but without (or with a wrong) namespace :)


function doFoo(): string
{
Expand All @@ -12,7 +13,7 @@ function (): void {
/** @var non-empty-string $a */
$a = doFoo();
assertType('non-empty-string', $a);
assertNativeType('string', $a);
assertNativeType('mixed', $a); // could be fixed
};

/**
Expand All @@ -34,10 +35,22 @@ public function __construct(
private array $array
){
assertType('non-empty-array', $this->array);
assertNativeType('array', $this->array);
assertNativeType('non-empty-array', $this->array); // could be fixed issue https://github.com/phpstan/phpstan/issues/6260
if(count($array) === 0){
throw new \InvalidArgumentException();
}
}

/**
* @param array{a: 'b'} $a
* @return void
*/
public function doUnset(array $a){
assertType("array{a: 'b'}", $a);
assertNativeType('array', $a);
unset($a['a']);
assertType("array{}", $a);
assertNativeType("array<mixed~'a', mixed>", $a);
}
}