From cbe313364c5410ab32de55ac497501a96d6367cc Mon Sep 17 00:00:00 2001
From: Jake Hotson <jake.github@qzdesign.co.uk>
Date: Fri, 23 May 2025 00:39:07 +0100
Subject: [PATCH] [BUGFIX] Don't return objects from data providers

The same objects may be provided to multiple tests.
If a test manipulates an object,
it will no longer be in the initial expected state for other tests.
---
 tests/Unit/RuleSet/RuleSetTest.php | 89 +++++++++++++++++-------------
 1 file changed, 51 insertions(+), 38 deletions(-)

diff --git a/tests/Unit/RuleSet/RuleSetTest.php b/tests/Unit/RuleSet/RuleSetTest.php
index 03783b8e..a9a33773 100644
--- a/tests/Unit/RuleSet/RuleSetTest.php
+++ b/tests/Unit/RuleSet/RuleSetTest.php
@@ -51,38 +51,38 @@ public function implementsRuleContainer(): void
     }
 
     /**
-     * @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
+     * @return array<string, array{0: list<string>, 1: string, 2: list<string>}>
      */
-    public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames(): array
+    public static function providePropertyNamesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames(): array
     {
         return [
             'removing single rule' => [
-                [new Rule('color')],
+                ['color'],
                 'color',
                 [],
             ],
             'removing first rule' => [
-                [new Rule('color'), new Rule('display')],
+                ['color', 'display'],
                 'color',
                 ['display'],
             ],
             'removing last rule' => [
-                [new Rule('color'), new Rule('display')],
+                ['color', 'display'],
                 'display',
                 ['color'],
             ],
             'removing middle rule' => [
-                [new Rule('color'), new Rule('display'), new Rule('width')],
+                ['color', 'display', 'width'],
                 'display',
                 ['color', 'width'],
             ],
             'removing multiple rules' => [
-                [new Rule('color'), new Rule('color')],
+                ['color', 'color'],
                 'color',
                 [],
             ],
             'removing multiple rules with another kept' => [
-                [new Rule('color'), new Rule('color'), new Rule('display')],
+                ['color', 'color', 'display'],
                 'color',
                 ['display'],
             ],
@@ -92,7 +92,7 @@ public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPr
                 [],
             ],
             'removing nonexistent rule from nonempty list' => [
-                [new Rule('color'), new Rule('display')],
+                ['color', 'display'],
                 'width',
                 ['color', 'display'],
             ],
@@ -102,60 +102,60 @@ public static function provideRulesAndPropertyNameToRemoveAndExpectedRemainingPr
     /**
      * @test
      *
-     * @param list<Rule> $rules
+     * @param list<string> $initialPropertyNames
      * @param list<string> $expectedRemainingPropertyNames
      *
-     * @dataProvider provideRulesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
+     * @dataProvider providePropertyNamesAndPropertyNameToRemoveAndExpectedRemainingPropertyNames
      */
     public function removeMatchingRulesRemovesRulesByPropertyNameAndKeepsOthers(
-        array $rules,
-        string $propertyName,
+        array $initialPropertyNames,
+        string $propertyNameToRemove,
         array $expectedRemainingPropertyNames
     ): void {
-        $this->subject->setRules($rules);
+        $this->setRulesFromPropertyNames($initialPropertyNames);
 
-        $this->subject->removeMatchingRules($propertyName);
+        $this->subject->removeMatchingRules($propertyNameToRemove);
 
         $remainingRules = $this->subject->getRulesAssoc();
-        self::assertArrayNotHasKey($propertyName, $remainingRules);
+        self::assertArrayNotHasKey($propertyNameToRemove, $remainingRules);
         foreach ($expectedRemainingPropertyNames as $expectedPropertyName) {
             self::assertArrayHasKey($expectedPropertyName, $remainingRules);
         }
     }
 
     /**
-     * @return array<string, array{0: list<Rule>, 1: string, 2: list<string>}>
+     * @return array<string, array{0: list<string>, 1: string, 2: list<string>}>
      */
-    public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames(): array
+    public static function providePropertyNamesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames(): array
     {
         return [
             'removing shorthand rule' => [
-                [new Rule('font')],
+                ['font'],
                 'font',
                 [],
             ],
             'removing longhand rule' => [
-                [new Rule('font-size')],
+                ['font-size'],
                 'font',
                 [],
             ],
             'removing shorthand and longhand rule' => [
-                [new Rule('font'), new Rule('font-size')],
+                ['font', 'font-size'],
                 'font',
                 [],
             ],
             'removing shorthand rule with another kept' => [
-                [new Rule('font'), new Rule('color')],
+                ['font', 'color'],
                 'font',
                 ['color'],
             ],
             'removing longhand rule with another kept' => [
-                [new Rule('font-size'), new Rule('color')],
+                ['font-size', 'color'],
                 'font',
                 ['color'],
             ],
             'keeping other rules whose property names begin with the same characters' => [
-                [new Rule('contain'), new Rule('container'), new Rule('container-type')],
+                ['contain', 'container', 'container-type'],
                 'contain',
                 ['container', 'container-type'],
             ],
@@ -165,18 +165,18 @@ public static function provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemai
     /**
      * @test
      *
-     * @param list<Rule> $rules
+     * @param list<string> $initialPropertyNames
      * @param list<string> $expectedRemainingPropertyNames
      *
-     * @dataProvider provideRulesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
+     * @dataProvider providePropertyNamesAndPropertyNamePrefixToRemoveAndExpectedRemainingPropertyNames
      */
     public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOthers(
-        array $rules,
+        array $initialPropertyNames,
         string $propertyNamePrefix,
         array $expectedRemainingPropertyNames
     ): void {
         $propertyNamePrefixWithHyphen = $propertyNamePrefix . '-';
-        $this->subject->setRules($rules);
+        $this->setRulesFromPropertyNames($initialPropertyNames);
 
         $this->subject->removeMatchingRules($propertyNamePrefixWithHyphen);
 
@@ -191,31 +191,44 @@ public function removeMatchingRulesRemovesRulesByPropertyNamePrefixAndKeepsOther
     }
 
     /**
-     * @return array<string, array{0: list<Rule>}>
+     * @return array<string, array{0: list<string>}>
      */
-    public static function provideRulesToRemove(): array
+    public static function providePropertyNamesToRemove(): array
     {
         return [
-            'no rules' => [[]],
-            'one rule' => [[new Rule('color')]],
-            'two rules for different properties' => [[new Rule('color'), new Rule('display')]],
-            'two rules for the same property' => [[new Rule('color'), new Rule('color')]],
+            'no properties' => [[]],
+            'one property' => [['color']],
+            'two different properties' => [['color', 'display']],
+            'two of the same property' => [['color', 'color']],
         ];
     }
 
     /**
      * @test
      *
-     * @param list<Rule> $rules
+     * @param list<string> $propertyNamesToRemove
      *
-     * @dataProvider provideRulesToRemove
+     * @dataProvider providePropertyNamesToRemove
      */
-    public function removeAllRulesRemovesAllRules(array $rules): void
+    public function removeAllRulesRemovesAllRules(array $propertyNamesToRemove): void
     {
-        $this->subject->setRules($rules);
+        $this->setRulesFromPropertyNames($propertyNamesToRemove);
 
         $this->subject->removeAllRules();
 
         self::assertSame([], $this->subject->getRules());
     }
+
+    /**
+     * @param list<string> $propertyNames
+     */
+    private function setRulesFromPropertyNames(array $propertyNames): void
+    {
+        $this->subject->setRules(\array_map(
+            static function (string $propertyName): Rule {
+                return new Rule($propertyName);
+            },
+            $propertyNames
+        ));
+    }
 }