Fix isValid static analysis

[enumag]

I don't think the @psalm-assert-if-true is correct here. It causes false-positives like this with PHPStan 1.9:

Call to static method MyCLabs\Enum\Enum<string>::isValid() with string will always evaluate to true.

Which is of course wrong. Even if value is a string it doesn't necessarily mean it's one of the valid values for the enum.

Instead I think we should type $value with T since it doesn't really make sense to call isValid with a different type anyway - that would always return false.

cc @michaelpetri

[michaelpetri]

imo this doesn't make sense, if i declare value as T i would already knew it is valid, no?

Maybe you can rather prepare a fail case which proofs that there is a problem with the current implementation?

[enumag]

Let's assume this class:

/**
 * @extends Enum<string>
 */
class Status extends Enum {
    const SUCCESS = 'SUCCESS';
    const FAILURE = 'FAILURE';
}

The behavior of isValid is as follows:

var_dump(Status::isValid('SUCCESS')); // true
var_dump(Status::isValid('UNKNOWN')); // false
var_dump(Status::isValid(1)); // false

With your implementation static analysis fails for all 3:

// Call to static method MyCLabs\Enum\Enum<string>::isValid() with 'SUCCESS' will always evaluate to true. 
var_dump(Status::isValid('SUCCESS')); // true

// Call to static method MyCLabs\Enum\Enum<string>::isValid() with 'UNKNOWN' will always evaluate to true. 
var_dump(Status::isValid('UNKNOWN')); // false + the static analysis is incorrect

// Call to static method MyCLabs\Enum\Enum<string>::isValid() with 1 will always evaluate to false. 
var_dump(Status::isValid(1)); // false

With my suggestion:

var_dump(Status::isValid('SUCCESS')); // true, static analysis passes
var_dump(Status::isValid('UNKNOWN')); // false, static analysis passes
var_dump(Status::isValid(1)); // false, static analysis fails saying that isValid expects a string, int given

[michaelpetri]

imo the problem is your Enum<string> annotation, with it you declare T as any string, but it is only SUCCESS and FAILURE.

Can your try this:

/**
 * @extends Enum<Status::*>
 */
class Status extends Enum {
    const SUCCESS = 'SUCCESS';
    const FAILURE = 'FAILURE';
}

[enumag]

Ooooh! Okay, interesting. I was completely unaware of that syntax. With that it works fine of course. It should be mentioned in the readme somewhere. Thanks!

[michaelpetri]

Agree with the readme, maybe you can just open a PR for it?

In addition, worth to mention you can also use only a specific scope of constants

/**
 * @extends Enum<Status::VALUE_*>
 */
class Status extends Enum {
    const VALUE_SUCCESS = 'SUCCESS';
    const VALUE_FAILURE = 'FAILURE';
    const FOO = 'BAR';
}

or as literal values

/**
 * @extends Enum<'SUCCESS'|'FAILURE'>
 */
class Status extends Enum {
    const SUCCESS = 'SUCCESS';
    const FAILURE = 'FAILURE';
}