Add factory methods to create an enum bit mask

[gisostallenberg]

• Add a method to create a bit mask using one or multiple cases
• Add a method to create a bit mask using all cases
• Add a method to create a bit mask with no flags on
• Add a method to create a bit mask without some flags

[yaroslavche]

Looks ok to me.
Please fix docblocks regarding following rules:
if the docblock consists of one line, make it one line
do not repeat the method signature in the docblock (if the method returns self/static, you do not need to duplicate this information in the docblock)

Also, psalm have a several errors. PossiblyUnused can be suppressed.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (3072607) to head (45a325f).
Report is 2 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##                main       #31   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
- Complexity        35        41    +6     
===========================================
  Files              3         3           
  Lines             77        88   +11     
===========================================
+ Hits              77        88   +11     

[yaroslavche]

EnumBitMask::none is same as skip second argument in base factory method. Is it needed anyway?

In addition, without has psalm suppressions. It would be great to avoid the suppressions. So even if without is a simple factory method, maybe we should stick with all()->remove()?

[gisostallenberg]

@yaroslavche One of the points of a factory method is that it provides a clean and clear interface, so I'm very much for adding without. But I'm happy to remove it if that is not what you want.

After you added ba147a6, I'm not really sure what I can do to make this accepted.

There is a reason to add @return $this in doc block. $this means "the same object", while self or static means "an instance of type ...". At least PHPStan can check that and make sure the returned instance is the same object.

[yaroslavche]

@gisostallenberg In general, I agree with you. That makes sense. I've just waited for your opinion on that (sorry, that didn't tag you).

Regarding the use of static/$this in DocBlocks, I believe the method signature already indicates whether a method is static or not, making it unnecessary to duplicate this information in the DocBlock. A static method will never return $this, so there is no need to specify it explicitly in the DocBlock. Additionally, since the EnumBitMask class is final, the static keyword is redundant.
I aim to maintain clean and concise documentation without unnecessary duplication. However, I might be wrong in some aspects.

I will update the DocBlocks to make @throws consistent with others, add documentation to the README, and then merge the changes and make a new release. Thank you for your contribution.

[gisostallenberg]

I aim to maintain clean and concise documentation without unnecessary duplication. However, I might be wrong in some aspects.

You are right in the aspects of static methods not returning $this and that self is better for a final class.

I only added the docblock for $this return type in EnumBitMask::set and EnumBitMask::remove, where I think they have added value. See 35d40b2#diff-65d7c807c5a88226a7bc83369db7e5a2b25614c48428227993607486b49fb1c8R95

Thanks for merging!

Will you release a new version?

[yaroslavche]

@gisostallenberg

Will you release a new version?

yes, I've already added a new release: v3.0.2

I only added the docblock for $this return type in EnumBitMask::set and EnumBitMask::remove

My reasoning behind this change is as follows: With this PR, the final class now includes static factory methods that consistently return a new object of the class, which aligns with the expected behavior of factory methods. Conversely, when a non-static method returns self (and not a cloned instance), it indicates that the method operates on and returns the same object instance that invoked it.

[gisostallenberg]

yes, I've already added a new release: v3.0.2

Thanks!!

My reasoning behind this change is as follows: With this PR, the final class now includes static factory methods that consistently return a new object of the class, which aligns with the expected behavior of factory methods. Conversely, when a non-static method returns self (and not a cloned instance), it indicates that the method operates on and returns the same object instance that invoked it.

Sure. I agree. Besides that one can use the EnumBitMask::set and EnumBitMask::remove methods on an actual instance and in that case it would be a nice addition. But off course it is just a nice to have.

Thank you so much for your time spend on this PR 🙏🏽 .

[yaroslavche]

Thank you for your efforts! Really appreciate it <3