-
-
Notifications
You must be signed in to change notification settings - Fork 132
Hotfix/ability to create invalid enums #12
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
Merged
mnapoli
merged 6 commits into
myclabs:master
from
mirfilip:hotfix/ability-to-create-invalid-enums
Feb 10, 2015
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
970ed20
Rewrite tests of creating Enum from invalid values - use data provider
mirfilip e65fd09
Improve __toString() tests
mirfilip f12b80a
Fix ability to create invalid enums. Fixes #9
mirfilip e61f39c
Merge branch 'master' of github.com:myclabs/php-enum into hotfix/abil…
mirfilip 749458c
Use PHP 5.3 compatible array() notation instead of []
mirfilip dae119a
One more fix for array() instead of []
mirfilip File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,8 @@ | |
* Create an enum by implementing this class and adding class constants. | ||
* | ||
* @author Matthieu Napoli <[email protected]> | ||
* @author Daniel Costa <[email protected] | ||
* @author Daniel Costa <[email protected]> | ||
* @author Mirosław Filip <[email protected]> | ||
*/ | ||
abstract class Enum | ||
{ | ||
|
@@ -22,7 +23,7 @@ abstract class Enum | |
* @var mixed | ||
*/ | ||
protected $value; | ||
|
||
/** | ||
* Store existing constants in a static cache per object. | ||
* | ||
|
@@ -39,7 +40,7 @@ abstract class Enum | |
*/ | ||
public function __construct($value) | ||
{ | ||
if (!in_array($value, self::toArray())) { | ||
if (!$this->isValid($value)) { | ||
throw new \UnexpectedValueException("Value '$value' is not part of the enum " . get_called_class()); | ||
} | ||
|
||
|
@@ -106,7 +107,7 @@ public static function toArray() | |
*/ | ||
public static function isValid($value) | ||
{ | ||
return in_array($value, self::toArray()); | ||
return in_array($value, self::toArray(), true); | ||
} | ||
|
||
/** | ||
|
@@ -118,7 +119,7 @@ public static function isValid($value) | |
*/ | ||
public static function isValidKey($key) | ||
{ | ||
return in_array($key, self::keys()); | ||
return in_array($key, self::keys(), true); | ||
} | ||
|
||
/** | ||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,11 +15,25 @@ | |
* @method static EnumFixture BAR() | ||
* @method static EnumFixture NUMBER() | ||
* | ||
* @author Daniel Costa <[email protected] | ||
* @method static EnumFixture PROBLEMATIC_NUMBER() | ||
* @method static EnumFixture PROBLEMATIC_NULL() | ||
* @method static EnumFixture PROBLEMATIC_EMPTY_STRING() | ||
* @method static EnumFixture PROBLEMATIC_BOOLEAN_FALSE() | ||
* | ||
* @author Daniel Costa <[email protected]> | ||
* @author Mirosław Filip <[email protected]> | ||
*/ | ||
class EnumFixture extends Enum | ||
{ | ||
const FOO = "foo"; | ||
const BAR = "bar"; | ||
const NUMBER = 42; | ||
|
||
/** | ||
* Values that are known to cause problems when used with soft typing | ||
*/ | ||
const PROBLEMATIC_NUMBER = 0; | ||
const PROBLEMATIC_NULL = null; | ||
const PROBLEMATIC_EMPTY_STRING = ''; | ||
const PROBLEMATIC_BOOLEAN_FALSE = false; | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,8 @@ | |
|
||
/** | ||
* @author Matthieu Napoli <[email protected]> | ||
* @author Daniel Costa <[email protected] | ||
* @author Daniel Costa <[email protected]> | ||
* @author Mirosław Filip <[email protected]> | ||
*/ | ||
class EnumTest extends \PHPUnit_Framework_TestCase | ||
{ | ||
|
@@ -38,42 +39,44 @@ public function testGetKey() | |
} | ||
|
||
/** | ||
* @expectedException \UnexpectedValueException | ||
* @dataProvider invalidValueProvider | ||
*/ | ||
public function testInvalidValueString() | ||
public function testCreatingEnumWithInvalidValue($value) | ||
{ | ||
new EnumFixture("test"); | ||
} | ||
$this->setExpectedException( | ||
'\UnexpectedValueException', | ||
'Value \'' . $value . '\' is not part of the enum MyCLabs\Tests\Enum\EnumFixture' | ||
); | ||
|
||
/** | ||
* @expectedException \UnexpectedValueException | ||
*/ | ||
public function testInvalidValueInt() | ||
{ | ||
new EnumFixture(1234); | ||
new EnumFixture($value); | ||
} | ||
|
||
/** | ||
* @expectedException \UnexpectedValueException | ||
* Contains values not existing in EnumFixture | ||
* @return array | ||
*/ | ||
public function testInvalidValueEmpty() | ||
{ | ||
new EnumFixture(null); | ||
public function invalidValueProvider() { | ||
return array( | ||
"string" => array('test'), | ||
"int" => array(1234), | ||
); | ||
} | ||
|
||
/** | ||
* __toString() | ||
* @dataProvider toStringProvider | ||
*/ | ||
public function testToString() | ||
public function testToString($expected, $enumObject) | ||
{ | ||
$value = new EnumFixture(EnumFixture::FOO); | ||
$this->assertEquals(EnumFixture::FOO, (string) $value); | ||
|
||
$value = new EnumFixture(EnumFixture::BAR); | ||
$this->assertEquals(EnumFixture::BAR, (string) $value); | ||
$this->assertSame($expected, (string) $enumObject); | ||
} | ||
|
||
$value = new EnumFixture(EnumFixture::NUMBER); | ||
$this->assertEquals((string) EnumFixture::NUMBER, (string) $value); | ||
public function toStringProvider() { | ||
return array( | ||
array(EnumFixture::FOO, new EnumFixture(EnumFixture::FOO)), | ||
array(EnumFixture::BAR, new EnumFixture(EnumFixture::BAR)), | ||
array((string) EnumFixture::NUMBER, new EnumFixture(EnumFixture::NUMBER)), | ||
); | ||
} | ||
|
||
/** | ||
|
@@ -82,13 +85,17 @@ public function testToString() | |
public function testKeys() | ||
{ | ||
$values = EnumFixture::keys(); | ||
$this->assertInternalType("array", $values); | ||
$expectedValues = array( | ||
"FOO", | ||
"BAR", | ||
"NUMBER", | ||
"PROBLEMATIC_NUMBER", | ||
"PROBLEMATIC_NULL", | ||
"PROBLEMATIC_EMPTY_STRING", | ||
"PROBLEMATIC_BOOLEAN_FALSE", | ||
); | ||
$this->assertEquals($expectedValues, $values); | ||
|
||
$this->assertSame($expectedValues, $values); | ||
} | ||
|
||
/** | ||
|
@@ -97,13 +104,17 @@ public function testKeys() | |
public function testToArray() | ||
{ | ||
$values = EnumFixture::toArray(); | ||
$this->assertInternalType("array", $values); | ||
$expectedValues = array( | ||
"FOO" => EnumFixture::FOO, | ||
"BAR" => EnumFixture::BAR, | ||
"NUMBER" => EnumFixture::NUMBER, | ||
"FOO" => EnumFixture::FOO, | ||
"BAR" => EnumFixture::BAR, | ||
"NUMBER" => EnumFixture::NUMBER, | ||
"PROBLEMATIC_NUMBER" => EnumFixture::PROBLEMATIC_NUMBER, | ||
"PROBLEMATIC_NULL" => EnumFixture::PROBLEMATIC_NULL, | ||
"PROBLEMATIC_EMPTY_STRING" => EnumFixture::PROBLEMATIC_EMPTY_STRING, | ||
"PROBLEMATIC_BOOLEAN_FALSE" => EnumFixture::PROBLEMATIC_BOOLEAN_FALSE, | ||
); | ||
$this->assertEquals($expectedValues, $values); | ||
|
||
$this->assertSame($expectedValues, $values); | ||
} | ||
|
||
/** | ||
|
@@ -128,11 +139,29 @@ public function testBadStaticAccess() | |
|
||
/** | ||
* isValid() | ||
* @dataProvider isValidProvider | ||
*/ | ||
public function testIsValid() | ||
public function testIsValid($value, $isValid) | ||
{ | ||
$this->assertTrue(EnumFixture::isValid('foo')); | ||
$this->assertFalse(EnumFixture::isValid('baz')); | ||
$this->assertSame($isValid, EnumFixture::isValid($value)); | ||
} | ||
|
||
public function isValidProvider() { | ||
return array( | ||
/** | ||
* Valid values | ||
*/ | ||
array('foo', true), | ||
array(42, true), | ||
array(null, true), | ||
array(0, true), | ||
array('', true), | ||
array(false, true), | ||
/** | ||
* Invalid values | ||
*/ | ||
array('baz', false) | ||
); | ||
} | ||
|
||
/** | ||
|
@@ -150,6 +179,9 @@ public function testIsValidKey() | |
public function testSearch() | ||
{ | ||
$this->assertEquals('FOO', EnumFixture::search('foo')); | ||
$this->assertNotEquals('FOO', EnumFixture::isValidKey('baz')); | ||
/** | ||
* @see https://github.com/myclabs/php-enum/issues/9 | ||
*/ | ||
$this->assertEquals(EnumFixture::PROBLEMATIC_NUMBER, EnumFixture::search(1)); | ||
} | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we search for
1
and we check it returnsPROBLEMATIC_NUMBER
whose value is0
, is that normal?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, it's not normal. I've just added that so that I could reference it in another bug ticket, since search() method is also not type safe. I'll fix that soon.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK so we leave it that way for now but you are planning to fix it?