Add check for adding param to method

[AydinHassan]

Heyy,

We recently encountered #750 and I had a quick stab at implementing it, to start the conversation off. I guess we should also support removing. Feedback welcome!

Fixes #750

[Ocramius]

Let's add the "why" in this comment, and then we can merge :-)

[AydinHassan]

Nice! Updated :)

[Ocramius]

🚢 thanks @AydinHassan!

[Ocramius]

@AydinHassan uh-oh, some more tests to add to cover the last introduced mutations

Warning: Escaped Mutant for Mutator "LogicalOr":

--- Original
+++ New
@@ @@
 {
     public function __invoke(ReflectionMethod $fromMethod, ReflectionMethod $toMethod) : Changes
     {
-        if ($fromMethod->getDeclaringClass()->isFinal() || $fromMethod->isPrivate()) {
+        if ($fromMethod->getDeclaringClass()->isFinal() && $fromMethod->isPrivate()) {
             return Changes::empty();
         }
         $added = array_diff(array_map(static fn(ReflectionParameter $param) => $param->getName(), $toMethod->getParameters()), array_map(static fn(ReflectionParameter $param) => $param->getName(), $fromMethod->getParameters()));


Warning: Escaped Mutant for Mutator "UnwrapArrayDiff":

--- Original
+++ New
@@ @@
         if ($fromMethod->getDeclaringClass()->isFinal() || $fromMethod->isPrivate()) {
             return Changes::empty();
         }
-        $added = array_diff(array_map(static fn(ReflectionParameter $param) => $param->getName(), $toMethod->getParameters()), array_map(static fn(ReflectionParameter $param) => $param->getName(), $fromMethod->getParameters()));
+        $added = array_map(static fn(ReflectionParameter $param) => $param->getName(), $toMethod->getParameters());
         if ($toMethod->getNumberOfParameters() > $fromMethod->getNumberOfParameters()) {
             return Changes::fromList(...array_map(static fn(string $paramName): Change => Change::added(Str\format('Parameter %s was added to Method %s() of class %s', $paramName, $fromMethod->getName(), $fromMethod->getDeclaringClass()->getName())), $added));
         }


Warning: Escaped Mutant for Mutator "UnwrapArrayMap":

--- Original
+++ New
@@ @@
         if ($fromMethod->getDeclaringClass()->isFinal() || $fromMethod->isPrivate()) {
             return Changes::empty();
         }
-        $added = array_diff(array_map(static fn(ReflectionParameter $param) => $param->getName(), $toMethod->getParameters()), array_map(static fn(ReflectionParameter $param) => $param->getName(), $fromMethod->getParameters()));
+        $added = array_diff(array_map(static fn(ReflectionParameter $param) => $param->getName(), $toMethod->getParameters()), $fromMethod->getParameters());
         if ($toMethod->getNumberOfParameters() > $fromMethod->getNumberOfParameters()) {
             return Changes::fromList(...array_map(static fn(string $paramName): Change => Change::added(Str\format('Parameter %s was added to Method %s() of class %s', $paramName, $fromMethod->getName(), $fromMethod->getDeclaringClass()->getName())), $added));
         }


Warning: Escaped Mutant for Mutator "GreaterThan":

--- Original
+++ New
@@ @@
             return Changes::empty();
         }
         $added = array_diff(array_map(static fn(ReflectionParameter $param) => $param->getName(), $toMethod->getParameters()), array_map(static fn(ReflectionParameter $param) => $param->getName(), $fromMethod->getParameters()));
-        if ($toMethod->getNumberOfParameters() > $fromMethod->getNumberOfParameters()) {
+        if ($toMethod->getNumberOfParameters() >= $fromMethod->getNumberOfParameters()) {
             return Changes::fromList(...array_map(static fn(string $paramName): Change => Change::added(Str\format('Parameter %s was added to Method %s() of class %s', $paramName, $fromMethod->getName(), $fromMethod->getDeclaringClass()->getName())), $added));
         }
         return Changes::empty();
     }
 }

Warning:  Dashboard report has not been sent: The current process is a pull request build

This can be tested by expanding your data provider:

• add examples that remove parameters -> shouldn't trip this comparator
• add examples with private methods -> shouldn't trip this comparator

[AydinHassan]

@Ocramius I tried adding a few cases which I've just pushed. I also added some other tests that I since deleted but it doesn't seem to fix the escaped mutants. Sorry I don't have any experience with Infection and looks like I am fundamentally missing something. Whatever I change seems to have no effect :D

edit: i'm a dumbass missed adding the provider cases

edit 2: Ok I managed to kill a few but I'm a bit lost with the GreaterThan mutant

edit 3: Fixed the last one by deleting code 🔪

@Ocramius could you so kindly approve the workflow :)

[Ocramius]

@Ocramius I tried adding a few cases which I've just pushed. I also added some other tests that I since deleted but it doesn't seem to fix the escaped mutants. Sorry I don't have any experience with Infection and looks like I am fundamentally missing something. Whatever I change seems to have no effect :D

edit: i'm a dumbass missed adding the provider cases

edit 2: Ok I managed to kill a few but I'm a bit lost with the GreaterThan mutant

edit 3: Fixed the last one by deleting code 🔪

@Ocramius could you so kindly approve the workflow :)

and now you know how it works :-)

[Ocramius]

🚢 excellent!

[AydinHassan]

thanks for the guidance and speed @Ocramius ! 🏎️