BenSampo / laravel-enum

Simple, extensible and powerful enumeration implementation for Laravel.
https://sampo.co.uk/blog/using-enums-in-laravel
MIT License
2k stars 164 forks source link

allow non-empty-string in hasValue #315

Closed AlexKratky closed 1 year ago

AlexKratky commented 1 year ago

Changes

I believe that hasValue should also accept non-empty-string, not only TValue, because if I use enum like this:

<?php
namespace App\Enum;

use BenSampo\Enum\Enum;

/**
 * @extends Enum<self::*>
 */
class MyEnum extends Enum
{
    public const TEST = 'test';
}
?>

then static analysis does not allow me to pass any other value to than 'test' to hasValue method, which is wrong, because if I already know that its there, I dont need to call hasValue()

spawnia commented 1 year ago

I did not know about the self::* syntax - is this available in PHPStan and/or Psalm? Could you link docs about it? It might be a suitable default for TValue.

The reason behind annotating hasValue() with the generic type is that it would be unnecessary to call it if the value is known to not belong on a type-level. However, I think this might not be possible when the generic type is no longer just a primitive type but rather a union type with discrete values.

If we open up the function, I think we should accept any possible value, non-empty-string is too narrow. It could also be '', int, float or any other pimitive value.

AlexKratky commented 1 year ago

Yeah, I agree, there maybe should be mixed and not only not-empty-string. Not sure aboust psalm, but PHPstan does: https://phpstan.org/writing-php-code/phpdoc-types#literals-and-constants (Literals and constants)

spawnia commented 1 year ago

Let's just allow mixed.

AlexKratky commented 1 year ago

agree, removed annotations so there is only mixed in parameters