Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Rector\Tests\DeadCode\Rector\Cast\RecastingRemovalRector\FixturePhp74;

final class SkipSubstr
{
public function run(string $value): string
{
return (string) substr($value, 10, 10);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace DeadCode\Rector\Cast\RecastingRemovalRector;

use Iterator;
use PHPUnit\Framework\Attributes\DataProvider;
use Rector\Testing\PHPUnit\AbstractRectorTestCase;

final class RecastingRemovalRectorPhp74Test extends AbstractRectorTestCase
{
#[DataProvider('provideData')]
public function test(string $filePath): void
{
$this->doTestFile($filePath);
}

public static function provideData(): Iterator
{
return self::yieldFilesFromDirectory(__DIR__ . '/FixturePhp74');
}

public function provideConfigFilePath(): string
{
return __DIR__ . '/config/configured_rule_php74.php';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

use Rector\Config\RectorConfig;
use Rector\DeadCode\Rector\Cast\RecastingRemovalRector;
use Rector\ValueObject\PhpVersion;

return RectorConfig::configure()
->withRules([RecastingRemovalRector::class]);
->withRules([RecastingRemovalRector::class])
->withPhpVersion(PhpVersion::PHP_80);
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

declare(strict_types=1);

use Rector\Config\RectorConfig;
use Rector\DeadCode\Rector\Cast\RecastingRemovalRector;
use Rector\ValueObject\PhpVersion;

return RectorConfig::configure()
->withRules([RecastingRemovalRector::class])
->withPhpVersion(PhpVersion::PHP_74);
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnUnionTypeRector\FixtureTrueInUnion;

/**
* true|othertype work on >= php 8.2, ref https://3v4l.org/UJqXT
*/
final class TrueInUnionStrToUpper
{
public function run($value)
{
if ($value) {
return true;
}

return strtoupper('warning');
}
}

?>
-----
<?php

namespace Rector\Tests\TypeDeclaration\Rector\ClassMethod\ReturnUnionTypeRector\FixtureTrueInUnion;

/**
* true|othertype work on >= php 8.2, ref https://3v4l.org/UJqXT
*/
final class TrueInUnionStrToUpper
{
public function run($value): true|string
{
if ($value) {
return true;
}

return strtoupper('warning');
}
}

?>
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,13 @@ public function shouldSkip(File $file, FullyQualifiedObjectType $fullyQualifiedO
$namespace = strtolower((string) $namespace);

$shortNameLowered = $fullyQualifiedObjectType->getShortNameLowered();
/**
* on php 7.x, substr() result can return false, so force (string) is needed
* @see https://github.com/rectorphp/rector-src/pull/7436
*/
$subClassName = (string) substr(

$subClassName = substr(
Copy link
Member

Choose a reason for hiding this comment

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

this cast is needed here as downgrade rule is not yet created.

Copy link
Member Author

Choose a reason for hiding this comment

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

This should be fixed with incorrect input instead. This structure is clearly wrong.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I will create separate PR target your branch for different logic to handle this.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great 👍 We'll need to understand what values are going in any why it returns false.
It should be fixed to return always string instead

Copy link
Member

Choose a reason for hiding this comment

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

see #7450

$fullyQualifiedObjectType->getClassName(),
0,
-strlen($fullyQualifiedObjectType->getShortName()) - 1
);

$fullyQualifiedObjectTypeNamespace = strtolower($subClassName);

foreach ($classLikeNames as $classLikeName) {
Expand Down
11 changes: 1 addition & 10 deletions rules/DeadCode/Rector/Cast/RecastingRemovalRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@
use PhpParser\Node\Expr\Cast\Int_;
use PhpParser\Node\Expr\Cast\Object_;
use PhpParser\Node\Expr\Cast\String_;
use PhpParser\Node\Expr\FuncCall;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Expr\StaticCall;
use PHPStan\Reflection\Php\PhpPropertyReflection;
use PHPStan\Type\ArrayType;
use PHPStan\Type\BooleanType;
use PHPStan\Type\Constant\ConstantArrayType;
use PHPStan\Type\Constant\ConstantBooleanType;
use PHPStan\Type\FloatType;
use PHPStan\Type\IntegerType;
use PHPStan\Type\MixedType;
Expand Down Expand Up @@ -55,7 +53,7 @@ final class RecastingRemovalRector extends AbstractRector
public function __construct(
private readonly PropertyFetchAnalyzer $propertyFetchAnalyzer,
private readonly ReflectionResolver $reflectionResolver,
private readonly ExprAnalyzer $exprAnalyzer
private readonly ExprAnalyzer $exprAnalyzer,
) {
}

Expand Down Expand Up @@ -105,13 +103,6 @@ public function refactor(Node $node): ?Node
return null;
}

// substr can return false on php 7.x
if ($node->expr instanceof FuncCall
&& $this->isName($node->expr, 'substr')
&& ! $node->expr->isFirstClassCallable()) {
$nodeType = new UnionType([new StringType(), new ConstantBooleanType(false)]);
}

if ($nodeType instanceof ConstantArrayType && $nodeClass === Array_::class) {
if ($this->shouldSkip($node->expr)) {
return null;
Expand Down
24 changes: 24 additions & 0 deletions src/NodeTypeResolver/NodeTypeResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
use PHPStan\Type\NullType;
use PHPStan\Type\ObjectType;
use PHPStan\Type\ObjectWithoutClassType;
use PHPStan\Type\StringType;
use PHPStan\Type\ThisType;
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
Expand All @@ -55,9 +56,11 @@
use Rector\NodeTypeResolver\NodeTypeCorrector\AccessoryNonEmptyStringTypeCorrector;
use Rector\NodeTypeResolver\NodeTypeCorrector\GenericClassStringTypeCorrector;
use Rector\NodeTypeResolver\PHPStan\ObjectWithoutClassTypeWithParentTypes;
use Rector\Php\PhpVersionProvider;
use Rector\StaticTypeMapper\ValueObject\Type\AliasedObjectType;
use Rector\StaticTypeMapper\ValueObject\Type\ShortenedObjectType;
use Rector\TypeDeclaration\PHPStan\ObjectTypeSpecifier;
use Rector\ValueObject\PhpVersion;

final class NodeTypeResolver
{
Expand All @@ -83,6 +86,7 @@ public function __construct(
private readonly AccessoryNonEmptyArrayTypeCorrector $accessoryNonEmptyArrayTypeCorrector,
private readonly RenamedClassesDataCollector $renamedClassesDataCollector,
private readonly NodeNameResolver $nodeNameResolver,
private readonly PhpVersionProvider $phpVersionProvider,
iterable $nodeTypeResolvers
) {
foreach ($nodeTypeResolvers as $nodeTypeResolver) {
Expand Down Expand Up @@ -620,6 +624,10 @@ private function resolveNativeTypeWithBuiltinMethodCallFallback(Expr $expr, Scop
return $scope->getNativeType($expr);
}

if ($this->isSubstrOnPHP74($expr)) {
return new UnionType([new StringType(), new ConstantBooleanType(false)]);
}

return $scope->getType($expr);
}

Expand Down Expand Up @@ -651,4 +659,20 @@ private function isEnumTypeMatch(MethodCall|NullsafeMethodCall $call, ObjectType

return $classReflection->getName() === $objectType->getClassName();
}

/**
* substr can return false on php 7.x and bellow
*/
private function isSubstrOnPHP74(FuncCall $funcCall): bool
{
if ($funcCall->isFirstClassCallable()) {
return false;
}

if (! $this->nodeNameResolver->isName($funcCall, 'substr')) {
return false;
}

return ! $this->phpVersionProvider->isAtLeastPhpVersion(PhpVersion::PHP_80);
Copy link
Member

Choose a reason for hiding this comment

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

PHPStan already detect it clearly when run on php 7.x

see https://phpstan.org/r/853f59a6-22ad-48ac-af93-66606b569613

on PHP 7.2 - 7.4 tab.

Copy link
Member Author

Choose a reason for hiding this comment

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

But not on PHP 8.0+. This also allow to add our test with PHP 7.4 version.

}
}