General code cleanup

[xHeaven]

• Extended .gitignore with .idea (for IntelliJ IDEs) and .phpunit.cache (for tests)
• Removed unnecessary checks and added proper types instead
• Simplified control flow in some cases
• Added missing types
• Fixed phpstan configuration deprecation
• Added proper typing to is() and isNot() - probably BC, even though it'd never make sense to pass anything other than an Enum due to === check as $this will always be an UnitEnum (either pure or backed, both extend this interface), so anything other than an UnitEnum will yield a false result
  • if this is not the desired behavior and we want users to be able to pass impossible-to-be-true values, I can revert this change moved to [2.x] Narrow argument type for is/isNot #30

[stancl]

Regarding the iterable change, this is the current code coverage:

Could we add a test that passes an Iterator so the case is covered? The line with the exception is irrelevant now as that will be a runtime type error.

The change does make sense to me, I'd just like to confirm everything works as intended. I think the reason I went with those particular types is that they were used by Laravel collections. But in retrospect array|Traversable would've probably achieved the same thing with less code and so does iterable.

[xHeaven]

Regarding the iterable change, this is the current code coverage:

-snipped img-

Could we add a test that passes an Iterator so the case is covered? The line with the exception is irrelevant now as that will be a runtime type error.

The change does make sense to me, I'd just like to confirm everything works as intended. I think the reason I went with those particular types is that they were used by Laravel collections. But in retrospect array|Traversable would've probably achieved the same thing with less code and so does iterable.

I'm a bit confused here, the proposed changes have 100% coverage.

[stancl]

The new version does — the old version doesn't. Meaning there were cases the logic handled that aren't covered by our tests.

[xHeaven]

The new version does — the old version doesn't. Meaning there were cases the logic handled that aren't covered by our tests.

Should be there. commit

in() would also have to be updated to use is() instead of inline ===

This is also done. commit

[stancl]

Bit unrelated to this PR but I just randomly noticed one issue in the code and it could be fixed here if you want to address it.

Essentially, if you use the From trait on a backed enum, to be able to use fromName(), the from() method in the trait will not override the backed enum's native from() at runtime but editors think it does. So if you have

enum MyBackedEnum: int
{
    use From; // for fromName()

    case FOO = 1;
}

And you do: MyBackedEnum::from(1), it may seem like you're calling:

public static function from(string $case): static

(at least Intelephense thinks this). Where the string typehint on $case will not allow you to do MyBackedEnum::from(1), you'd need to cast it to a string nonsensically.

Do you think we should just change the signature to from(int|string), matching the native from?

[xHeaven]

Bit unrelated to this PR but I just randomly noticed one issue in the code and it could be fixed here if you want to address it.

Essentially, if you use the From trait on a backed enum, to be able to use fromName(), the from() method in the trait will not override the backed enum's native from() at runtime but editors think it does. So if you have

enum MyBackedEnum: int
{
    use From; // for fromName()

    case FOO = 1;
}

And you do: MyBackedEnum::from(1), it may seem like you're calling:

public static function from(string $case): static

(at least Intelephense thinks this). Where the string typehint on $case will not allow you to do MyBackedEnum::from(1), you'd need to cast it to a string nonsensically.

Do you think we should just change the signature to from(int|string), matching the native from?

Uh, this is an odd one.

Honestly, I'd probably move out from and tryFrom into a FromPure trait and keep fromName and tryFromName in place, or something like that.

Ghetto-fixing it with adding the int type as an acceptable one could be a solution, but that'd raise another issue: people could pass an integer instead of a string for pure enums, which would make absolutely no sense.
Maybe we could handle these cases separately, checking whether the caller enum is pure, and if an integer is passed, instead of relying on fromName, we could rely on fromValue or something like that.

[stancl]

but that'd raise another issue: people could pass an integer instead of a string for pure enums, which would make absolutely no sense

Yeah but it'd just fail, which is kinda expected. The from() for pure enums is essentially a polyfill in a way, and with PHP not allowing method overloading I think the only thing that makes sense here is to just use the same types.

I mentioned that my LSP complains (but only in some projects...?) when I pass the backed value (int) to from(), since it can see a definition that accepts string. This could make the user try to cast the value to a string which then results in a runtime type failure.

So I think we have to make the types the same either way.

Maybe we could handle these cases separately, checking whether the caller enum is pure, and if an integer is passed

The caller is always pure because if it's a backed enum, our trait method will be ignored in favor of the native from(). I was thinking about adding a check for the type inside the method but that wouldn't really help anything, pure enums have a limited number of strings from which they can be created via from() and everything else should just fail.

[xHeaven]

but that'd raise another issue: people could pass an integer instead of a string for pure enums, which would make absolutely no sense

Yeah but it'd just fail, which is kinda expected. The from() for pure enums is essentially a polyfill in a way, and with PHP not allowing method overloading I think the only thing that makes sense here is to just use the same types.

I mentioned that my LSP complains (but only in some projects...?) when I pass the backed value (int) to from(), since it can see a definition that accepts string. This could make the user try to cast the value to a string which then results in a runtime type failure.

So I think we have to make the types the same either way.

Maybe we could handle these cases separately, checking whether the caller enum is pure, and if an integer is passed

The caller is always pure because if it's a backed enum, our trait method will be ignored in favor of the native from(). I was thinking about adding a check for the type inside the method but that wouldn't really help anything, pure enums have a limited number of strings from which they can be created via from() and everything else should just fail.

Fair enough, let's just change the signature then. Should I proceed with it?

I mentioned that my LSP complains (but only in some projects...?)

Perhaps this happens in projects where strict_types are declared? In a non-strict environment, integers just get casted to string implicitly if you pass them to a function that accepts a string. In a strict environment, though, it's a problem that you have to solve.

[stancl]

Yeah that was it:

Perhaps we should also add the strict types declaration to our test files?

[xHeaven]

Yeah that was it: -snipped image-

Perhaps we should also add the strict types declaration to our test files?

I tried that, and the error is shown in my IDE, but apparently, Pest (or indirectly PHPUnit) doesn't seem to care about it - the tests pass even with seemingly invalid values.
See:

Likely because the tests are running not directly from the test file itself, but from an underlying layer of PHPUnit which is not strict. Honestly, I have absolutely no idea how to fix this.

In a production environment with strict_types though, it raises a TypeError, see this code on PHPSandbox:

<?php

declare(strict_types=1);

enum MyEnum
{
    use \ArchTech\Enums\From;

    case HELLO;
    case WORLD;
}

MyEnum::from(1);

In any way, extending the method signature to string|int from string would solve these issues, so I'd say let's just change the method signature.

[stancl]

Well, Status::from(0) is valid. It's a backed enum so our trait's from() doesn't get used. It's only editors that think it does get used. So the point of the strict_types is to get these editor errors rather than change any behavior.

[xHeaven]

Well, Status::from(0) is valid. It's a backed enum so our trait's from() doesn't get used. It's only editors that think it does get used. So the point of the strict_types is to get these editor errors rather than change any behavior.

For some reason I always forget that backend enums won't actually use the from and tryFrom methods from the trait. Smh. Thanks for clarifying for the 10th time.

I can commit the changed tests, let me know if you want them in this PR.

Also, I can change the method signature for from and tryFrom, however, since they only act as a proxy for fromName and tryFromName, do we want their signatures changed as well?