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

[TypeDeclaration] Add false and true in union support on ReturnUnionTypeRector #5355

Merged
merged 10 commits into from Jan 3, 2024

Conversation

samsonasik
Copy link
Member

This PR make:

  • on php 8.0 until php 8.1: false|string is allowed, combine true and string become bool|string
  • on >= php 8.2: false|string and true|string is allowed

ref

@samsonasik samsonasik changed the title [TypeDeclaration] Add false and true in union on ReturnUnionTypeRector [TypeDeclaration] Add false and true in union support on ReturnUnionTypeRector Dec 11, 2023
@samsonasik
Copy link
Member Author

By this, I found a bug on ReturnTypeFromStrictTernaryRector, which 2 PHPStan\Type\Constant\ConstantStringType marked as union, which invalid

https://github.com/rectorphp/rector-src/actions/runs/7172210877/job/19528876575?pr=5355#step:5:90

array (2)
   0 => PHPStan\Type\Constant\ConstantStringType #9554
   |  value: 'hey'
   |  isClassString: false
   |  objectType: null
   |  arrayKeyType: null
   1 => PHPStan\Type\Constant\ConstantStringType #9560
   |  value: 'hou'
   |  isClassString: false
   |  objectType: null
   |  arrayKeyType: null

On use case:

final class LocalConstants
{
    const FIRST = 'hey';
    const SECOND = 'hou';

    public function getValue($number)
    {
        return $number ? self::FIRST : self::SECOND;
    }
}

the issue seems on TypeFactory::uniquateTypes()

public function uniquateTypes(array $types, bool $keepConstant = false): array

@samsonasik
Copy link
Member Author

It seems resolved with remove value to be included in hash on TypeHasher 22db784

@samsonasik
Copy link
Member Author

I added failing fixture for both false and true in union, to be bool.

@samsonasik samsonasik marked this pull request as draft December 11, 2023 20:17
@TomasVotruba
Copy link
Member

@samsonasik What needs to be done here to finish/close this one? I'd like to resolve it before next release

@samsonasik
Copy link
Member Author

it currently only phpstan complexity https://github.com/rectorphp/rector-src/actions/runs/7172780220/job/19530707952?pr=5355#step:5:19 , but I may missing something on hash uniquite types

I can rebase and extract method/ignore in phpstan.neon after psr4 work, it seems still error on scoped build

https://github.com/rectorphp/rector-src/actions/runs/7378601824/job/20073924556#step:11:38

@samsonasik samsonasik marked this pull request as ready for review January 3, 2024 05:40
@samsonasik
Copy link
Member Author

All checks have passed 🎉 @TomasVotruba I think it is ready.

@samsonasik
Copy link
Member Author

@TomasVotruba let's merge it to have faster feedback to test ;)

@samsonasik samsonasik merged commit 09c077e into main Jan 3, 2024
41 checks passed
@samsonasik samsonasik deleted the add-false-true-in-union branch January 3, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants