Detect changes in ENUMs as BC breaks

[bdsl]

See #767

I went slightly further than discussed in the issue, reporting a BC break for cases removed, as well as for cases added. In both cases cases marked @internal do not trigger the BC break.

I think there's more to do for a complete handling of enums, including detecting if an entire Enum is removed, or becomes a trait, class or interface. Actually I see the entire Enum removed is already handled by the catch of IdentifierNotFound

BackwardCompatibilityCheck/src/CompareClasses.php

Line 76 in 4c05d1a

} catch (IdentifierNotFound) {

and I could easily extend this PR to handle became trait/interface/class by simply treating one of those as a thing that has no cases - so an enum becoming one of those is not an issue itself, its only an issue because it means cases go away. And an enum with no cases becoming a different sort of classlike is not a BC break. Let me know if you'd like me to do that.

[bdsl]

If you approve the workflow I'll try to fix any issues it detects.

I've run unit tests, psalm and phpcs on my local. It might be nice to have some end to end testing as well though. For this sort of project I wish git would work recursively, letting us have a git repo of a sample project inside this repo's test assets directory.

[Ocramius]

@bdsl done

[bdsl]

I also noticed there seems to be quite a bit of code duplication in the existing code between the handling of Classes, Traits and Interface. I think there could be a nice way to remove that duplication, but I didn't want to get into restructuring the existing code on my first PR. E.g. the ClassBased, TraitBased, InterfaceBased and new EnumBased interfaces are all the same except with differently named parameters, and the three MultipleChecksOnA... classes are also extremely similar. I didn't feel like writing MultipleChecksOnAnEnum which is partly why I implemented all the checks within one class EnumBased\ClassChanged

[bdsl]

ah didn't realise you need to approve the workflow separately for every commit. I've turned on github actions in my fork so we can see the results there: https://github.com/bdsl/BackwardCompatibilityCheck/actions

[bdsl]

The Infection check is currently failing. But on my local using vendor/bin/roave-infection-static-analysis-plugin --threads=8 --logger-html=infection.html I'm now getting 0 undetected mutants, so I'm not quite sure why. I'll have to come back to this later to see how to reproduce the way that github actions is running infection.

[bdsl]

@Ocramius can you see where I'm going wrong with the mutation tests? On my laptop (php 8.3 with xdebug in WSL) I'm getting 0 returned from the infection command as shown below. I don't know if it could be because I'm running xdebug rather than phpdbg.

$ git show --stat; git status; ./vendor/bin/roave-infection-static-analysis-plugin; echo $?
commit b5cfc9b2e141b0cd772db8e5f7ca95d9cf68a898 (HEAD -> 767-enum-cases, bdsl/767-enum-cases)
Author: Barney Laurance <barney@redmagic.org.uk>
Date:   Sun May 19 17:34:06 2024 +0100

    Roave/BackwardCompatibilityCheck#767: Remove unecassary array_values calls
    
    Infection detected that these were not needed

 src/DetectChanges/BCBreak/EnumBased/CasesChanged.php | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)
On branch 767-enum-cases
nothing to commit, working tree clean

    ____      ____          __  _
   /  _/___  / __/__  _____/ /_(_)___  ____
   / // __ \/ /_/ _ \/ ___/ __/ / __ \/ __ \
 _/ // / / / __/  __/ /__/ /_/ / /_/ / / / /
/___/_/ /_/_/  \___/\___/\__/_/\____/_/ /_/

#StandWithUkraine

Infection - PHP Mutation Testing Framework version 0.27.10


Running initial test suite...

PHPUnit version: 9.6.18

  755 [============================] 7 secs

Generate mutants...

Processing source code files: 115/115
.: killed, M: escaped, U: uncovered, E: fatal error, X: syntax error, T: timed out, S: skipped, I: ignored

..................................................   ( 50 / 639)
..................................................   (100 / 639)
..................................................   (150 / 639)
..................................................   (200 / 639)
..................................................   (250 / 639)
..................................................   (300 / 639)
..................................................   (350 / 639)
..................................................   (400 / 639)
..................................................   (450 / 639)
..................................................   (500 / 639)
..................................................   (550 / 639)
....E.............................................   (600 / 639)
.......................................              (639 / 639)

639 mutations were generated:
     638 mutants were killed
       0 mutants were configured to be ignored
       0 mutants were not covered by tests
       0 covered mutants were not detected
       1 errors were encountered
       0 syntax errors were encountered
       0 time outs were encountered
       0 mutants required more time than configured

Metrics:
         Mutation Score Indicator (MSI): 100%
         Mutation Code Coverage: 100%
         Covered Code MSI: 100%

Note: to see escaped mutants run Infection with "--show-mutations" or configure file loggers.

Please note that some mutants will inevitably be harmless (i.e. false positives).
Escaped mutants:
================

Timed Out mutants:
==================

Skipped mutants:
================

Not Covered mutants:
====================
[warning] Dashboard report has not been sent: The current process is not executed in a CI build
 ! [NOTE] The MSI is 9.9% percentage points over the required MSI. Consider increasing the required MSI percentage the  
 !        next time you run Infection.


Time: 55s. Memory: 0.11GB. Threads: 1
0

[Ocramius]

Notice: You are running Infection with phpdbg enabled.

Perhaps it's using phpdbg for coverage, rather than XDebug

[bdsl]

Notice: You are running Infection with phpdbg enabled.

Perhaps it's using phpdbg for coverage, rather than XDebug

Yes possibly. I'll see if I can it working with phpdbg on my local.

[bdsl]

This is ready for re-review.

[Ocramius]

final to be added, plus some minor adjustments, then this can 🚢 :)

[Ocramius]

All looking quite good!

I'll do a local check of specific mutants before merging :-)

[Ocramius]

Took me a bit (because I just switched my OS / main computer), but this is pristine when running with XDebug!

692 mutations were generated:
     691 mutants were killed
       0 mutants were configured to be ignored
       0 mutants were not covered by tests
       0 covered mutants were not detected
       1 errors were encountered
       0 syntax errors were encountered
       0 time outs were encountered
       0 mutants required more time than configured

Metrics:
         Mutation Score Indicator (MSI): 100%
         Mutation Code Coverage: 100%
         Covered Code MSI: 100%

Note: to see escaped mutants run Infection with "--show-mutations" or configure file loggers.

Please note that some mutants will inevitably be harmless (i.e. false positives).
Escaped mutants:
================

Timed Out mutants:
==================

Skipped mutants:
================

Not Covered mutants:
====================
[warning] Dashboard report has not been sent: The current process is not executed in a CI build
 ! [NOTE] The MSI is 9.9% percentage points over the required MSI. Consider increasing the required MSI percentage the  
 !        next time you run Infection.

I will lower the MT requirements to get this merged, meanwhile :-)

[bdsl]

Took me a bit (because I just switched my OS / main computer), but this is pristine when running with XDebug!

Yep, I ran infection on my local when I saw that it was failing on the build, so then killed all the mutants.

There are some mutations I can imagine easily that are not tested, like deleting the untested "return trait" here or equivalently replacing the if condition with false. But I think enough is tested.

[Ocramius]

Meanwhile, 🚢

Thanks @bdsl!