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.

Sign up for GitHub

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

Fixed #33 #35

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fixed #33 #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions src/EnumBitMask.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace BitMask;

use BackedEnum;
use BitMask\Exception\InvalidEnumException;
use BitMask\Exception\NotSingleBitException;
use BitMask\Exception\UnknownEnumException;
use BitMask\Util\Bits;
Expand All @@ -19,16 +21,27 @@ final class EnumBitMask implements BitMaskInterface

/**
* @param class-string $enum
* @throws InvalidEnumException
* @throws UnknownEnumException
*/
public function __construct(
private readonly string $enum,
int $mask = 0,
bool $isIntBacked = false,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed we instantiate the class in static functions. (like the all() method for example). We might run into consistency issues? It might be worth considering to treat all int backed enums as "isIntBacked". But that change would not be backward compatible, as people might be using int backed enum with the existing implementation.

That's why I was thinking about a subclass like IntBackedEnumBitmask.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your thoughts. You’re right that treating all int-backed enums as "isIntBacked" could lead to consistency issues and wouldn't be backward compatible. That's why the current approach includes the boolean parameter to ensure flexibility. However, creating a subclass like IntBackedEnumBitmask could be a cleaner solution in the long term.

) {
if (!is_subclass_of($this->enum, UnitEnum::class)) {
throw new UnknownEnumException('EnumBitMask enum must be subclass of UnitEnum');
}
foreach ($this->enum::cases() as $index => $case) {
if ($isIntBacked) {
if (!is_subclass_of($this->enum, BackedEnum::class)) {
throw new InvalidEnumException('Enum must be a backed enum');
}
/** @phpstan-var BackedEnum $case */
if (!is_int($case->value)) {
throw new InvalidEnumException('Enum must be an int-backed enum with integer values');
}
}
$this->map[$case->name] = Bits::indexToBit($index);
}
$this->bitmask = new BitMask($mask, count($this->enum::cases()) - 1);
Copy link

@DaanBiesterbos DaanBiesterbos Sep 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the difference is, but I had an issue here with the most significant bit in my implementation. Not sure exactly why, but removing the second argument solved the issue for me. Perhaps you can speculate what could be the cause of this?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Regarding the issue with the most significant bit, it’s possible that the problem stems from how the second argument (the mask) interacts with the bit positions in your enum. If the bitmask value overlaps or conflicts with the most significant bit, removing the second argument might have bypassed the issue. I’ll investigate further to pinpoint the exact cause.

Expand All @@ -38,7 +51,7 @@ public function __construct(
* Create an instance with given flags on
*
* @param class-string $enum
* @throws UnknownEnumException|NotSingleBitException
* @throws UnknownEnumException|NotSingleBitException|InvalidEnumException
*/
public static function create(string $enum, UnitEnum ...$bits): self
{
Expand All @@ -50,7 +63,7 @@ public static function create(string $enum, UnitEnum ...$bits): self
*
* @psalm-suppress MixedMethodCall, MixedArgument
* @param class-string $enum
* @throws UnknownEnumException|NotSingleBitException
* @throws UnknownEnumException|NotSingleBitException|InvalidEnumException
*/
public static function all(string $enum): self
{
Expand All @@ -62,7 +75,7 @@ public static function all(string $enum): self
*
* @psalm-suppress PossiblyUnusedMethod
* @param class-string $enum
* @throws UnknownEnumException|NotSingleBitException
* @throws UnknownEnumException|NotSingleBitException|InvalidEnumException
*/
public static function none(string $enum): self
{
Expand All @@ -74,7 +87,7 @@ public static function none(string $enum): self
*
* @psalm-suppress PossiblyUnusedMethod
* @param class-string $enum
* @throws UnknownEnumException|NotSingleBitException
* @throws UnknownEnumException|NotSingleBitException|InvalidEnumException
*/
public static function without(string $enum, UnitEnum ...$bits): self
{
Expand Down
9 changes: 9 additions & 0 deletions src/Exception/InvalidEnumException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?php

declare(strict_types=1);

namespace BitMask\Exception;

use Exception;

final class InvalidEnumException extends Exception implements BitMaskExceptionInterface {}
22 changes: 22 additions & 0 deletions tests/EnumBitMaskTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace BitMask\Tests;

use BitMask\EnumBitMask;
use BitMask\Exception\InvalidEnumException;
use BitMask\Exception\OutOfRangeException;
use BitMask\Exception\UnknownEnumException;
use BitMask\Tests\fixtures\Enum\BackedInt;
Expand Down Expand Up @@ -174,4 +175,25 @@ public function testWithoutFactory(): void
$enumBitmask = EnumBitMask::without(Permissions::class, Permissions::Delete);
assertSame(7, $enumBitmask->get());
}

public function testIsIntBackedEnumInvalidEnum(): void
{
// UnitEnum
$this->expectException(InvalidEnumException::class);
new EnumBitMask(Permissions::class, 3, isIntBacked: true);
}

public function testIsIntBackedEnum(): void
{
// backed int
$backedIntEnumBitmask = new EnumBitMask(BackedInt::class, 3, isIntBacked: true);
assertTrue($backedIntEnumBitmask->has(BackedInt::Create, BackedInt::Read));
assertFalse($backedIntEnumBitmask->has(BackedInt::Update, BackedInt::Delete));

// backed string
$this->expectException(InvalidEnumException::class);
$backedStringEnumBitmask = new EnumBitMask(BackedString::class, 3, isIntBacked: true);
assertTrue($backedStringEnumBitmask->has(BackedString::Create, BackedString::Read));
assertFalse($backedStringEnumBitmask->has(BackedString::Update, BackedString::Delete));
}
}