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

Construction of enum object with invalid value is possible #95

Closed
slepic opened this issue Jul 18, 2019 · 6 comments
Closed

Construction of enum object with invalid value is possible #95

slepic opened this issue Jul 18, 2019 · 6 comments
Labels

Comments

@slepic
Copy link

slepic commented Jul 18, 2019

Check this sample:

<?php
use MyCLabs\Enum\Enum;

class Enum1 extends Enum
{
        const A1 = 'A1';
        const A2 = 'A2';
}

class Enum2 extends Enum1
{
        const A3 = 'A3';
}

$e2 = new Enum2('A3');
$e1 = new Enum1($e2);

echo $e1 . \PHP_EOL;

Expected exception, instead got A3 printed.

new Enum1('A3');

throws as expected.

@mnapoli mnapoli added the bug label Jul 18, 2019
@mnapoli
Copy link
Member

mnapoli commented Jul 18, 2019

Thanks, that's an interesting problem 🤔

I'm wondering if all Enums should not be made final? (i.e. recommended in the docs)

Because inheritance doesn't work here: yes Enum2 is an instance of Enum2, but there is actually no guarantee that it's a valid value of Enum2 (hence it's not really an instance of Enum2).

@slepic
Copy link
Author

slepic commented Jul 18, 2019

IMO the problem is here:
https://github.com/myclabs/php-enum/blob/master/src/Enum.php#L44

The value should either be validated anyway, or passing an Enum instance to Enum constructor should not be allowed at all. Or maybe instead of instanceof check, you should check get_class($value) === get_called_class()

@mnapoli
Copy link
Member

mnapoli commented Jul 19, 2019

As I said I think the problem is more global.

function (Enum1 $value)

Here an instance of Enum2 would actually be a problem, because there would be no guarantee that it would have a valid value.

So I really think inheritance is incompatible with enums in general.

@KartaviK
Copy link
Contributor

The above error is the result of incorrect use of the enum pattern in the application. But this does not mean that it is forbidden, there are times when it is really needed. Solution in PR #97

@slepic
Copy link
Author

slepic commented Aug 12, 2019

Nice, that's exactly what I would have suggested, unless I got distracted by other stuff... :/ Thanks, @KartaviK

@mnapoli
Copy link
Member

mnapoli commented Aug 19, 2019

Closed by #97, thanks @KartaviK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants