From 7642f68b3425fa08fc46b14683b2ad94f839010d Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Wed, 4 Jun 2025 00:10:23 +0100 Subject: [PATCH 1/3] [BUGFIX] `AddRule` before sibling with different property name Part of #974. --- CHANGELOG.md | 2 ++ src/RuleSet/RuleSet.php | 33 ++++++++++++++++++++++++++++-- tests/Unit/RuleSet/RuleSetTest.php | 2 -- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5255f87e..7f69624e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -105,6 +105,8 @@ Please also have a look at our ### Fixed +- Insert `Rule` before sibling even with different property name + (in `RuleSet::addRule()`) (#1270) - Ensure `RuleSet::addRule()` sets non-negative column number when sibling provided (#1268) - Set line number when `RuleSet::addRule()` called with only column number set diff --git a/src/RuleSet/RuleSet.php b/src/RuleSet/RuleSet.php index b24eefab..5793e7f5 100644 --- a/src/RuleSet/RuleSet.php +++ b/src/RuleSet/RuleSet.php @@ -104,12 +104,29 @@ public function addRule(Rule $ruleToAdd, ?Rule $sibling = null): void $position = \count($this->rules[$propertyName]); if ($sibling !== null) { + $siblingLineNumber = $sibling->getLineNumber(); + $siblingColumnNumber = $sibling->getColumnNumber(); + $siblingIsInSet = false; $siblingPosition = \array_search($sibling, $this->rules[$propertyName], true); if ($siblingPosition !== false) { + $siblingIsInSet = true; $position = $siblingPosition; + } elseif ($siblingIsInSet = $this->hasRule($sibling)) { + // Maintain ordering within `$this->rules[$propertyName]` + // by inserting before first `Rule` with a same-or-later position than the sibling. + foreach ($this->rules[$propertyName] as $index => $rule) { + if ( + $rule->getLineNumber() > $siblingLineNumber || + $rule->getLineNumber() === $siblingLineNumber && + $rule->getColumnNumber() >= $siblingColumnNumber + ) { + $position = $index; + break; + } + } + } + if ($siblingIsInSet) { // Increment column number of all existing rules on same line, starting at sibling - $siblingLineNumber = $sibling->getLineNumber(); - $siblingColumnNumber = $sibling->getColumnNumber(); foreach ($this->rules as $rulesForAProperty) { foreach ($rulesForAProperty as $rule) { if ( @@ -123,6 +140,7 @@ public function addRule(Rule $ruleToAdd, ?Rule $sibling = null): void $ruleToAdd->setPosition($siblingLineNumber, $siblingColumnNumber); } } + if ($ruleToAdd->getLineNumber() === null) { //this node is added manually, give it the next best line $columnNumber = $ruleToAdd->getColumnNumber() ?? 0; @@ -305,4 +323,15 @@ private static function comparePositionable(Positionable $first, Positionable $s } return $first->getLineNo() - $second->getLineNo(); } + + private function hasRule(Rule $rule): bool + { + foreach ($this->rules as $rulesForAProperty) { + if (\in_array($rule, $rulesForAProperty, true)) { + return true; + } + } + + return false; + } } diff --git a/tests/Unit/RuleSet/RuleSetTest.php b/tests/Unit/RuleSet/RuleSetTest.php index 5b2bfa7a..03cacd85 100644 --- a/tests/Unit/RuleSet/RuleSetTest.php +++ b/tests/Unit/RuleSet/RuleSetTest.php @@ -221,8 +221,6 @@ public function addRuleWithSiblingInsertsRuleBeforeSibling( int $siblingIndex, string $propertyNameToAdd ): void { - self::markTestSkipped('tofix: if property names don\'t match, sibling is ignored'); - $ruleToAdd = new Rule($propertyNameToAdd); $this->setRulesFromPropertyNames($initialPropertyNames); $sibling = $this->subject->getRules()[$siblingIndex]; From 459e4033ea64e7c92cc0ef09912450fdfdcaa954 Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Wed, 4 Jun 2025 22:29:54 +0100 Subject: [PATCH 2/3] Use `comparePositionable()` --- src/RuleSet/RuleSet.php | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/RuleSet/RuleSet.php b/src/RuleSet/RuleSet.php index 5793e7f5..67505b5e 100644 --- a/src/RuleSet/RuleSet.php +++ b/src/RuleSet/RuleSet.php @@ -104,8 +104,6 @@ public function addRule(Rule $ruleToAdd, ?Rule $sibling = null): void $position = \count($this->rules[$propertyName]); if ($sibling !== null) { - $siblingLineNumber = $sibling->getLineNumber(); - $siblingColumnNumber = $sibling->getColumnNumber(); $siblingIsInSet = false; $siblingPosition = \array_search($sibling, $this->rules[$propertyName], true); if ($siblingPosition !== false) { @@ -115,11 +113,7 @@ public function addRule(Rule $ruleToAdd, ?Rule $sibling = null): void // Maintain ordering within `$this->rules[$propertyName]` // by inserting before first `Rule` with a same-or-later position than the sibling. foreach ($this->rules[$propertyName] as $index => $rule) { - if ( - $rule->getLineNumber() > $siblingLineNumber || - $rule->getLineNumber() === $siblingLineNumber && - $rule->getColumnNumber() >= $siblingColumnNumber - ) { + if (self::comparePositionable($rule, $sibling) >= 0) { $position = $index; break; } @@ -127,6 +121,8 @@ public function addRule(Rule $ruleToAdd, ?Rule $sibling = null): void } if ($siblingIsInSet) { // Increment column number of all existing rules on same line, starting at sibling + $siblingLineNumber = $sibling->getLineNumber(); + $siblingColumnNumber = $sibling->getColumnNumber(); foreach ($this->rules as $rulesForAProperty) { foreach ($rulesForAProperty as $rule) { if ( From 3b7f793fb9bfe5a33207b0e29b4abd67c6432bff Mon Sep 17 00:00:00 2001 From: Jake Hotson Date: Thu, 5 Jun 2025 15:02:09 +0100 Subject: [PATCH 3/3] Separate conditional from assignment --- src/RuleSet/RuleSet.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/RuleSet/RuleSet.php b/src/RuleSet/RuleSet.php index 67505b5e..8e0e8ae5 100644 --- a/src/RuleSet/RuleSet.php +++ b/src/RuleSet/RuleSet.php @@ -109,13 +109,16 @@ public function addRule(Rule $ruleToAdd, ?Rule $sibling = null): void if ($siblingPosition !== false) { $siblingIsInSet = true; $position = $siblingPosition; - } elseif ($siblingIsInSet = $this->hasRule($sibling)) { - // Maintain ordering within `$this->rules[$propertyName]` - // by inserting before first `Rule` with a same-or-later position than the sibling. - foreach ($this->rules[$propertyName] as $index => $rule) { - if (self::comparePositionable($rule, $sibling) >= 0) { - $position = $index; - break; + } else { + $siblingIsInSet = $this->hasRule($sibling); + if ($siblingIsInSet) { + // Maintain ordering within `$this->rules[$propertyName]` + // by inserting before first `Rule` with a same-or-later position than the sibling. + foreach ($this->rules[$propertyName] as $index => $rule) { + if (self::comparePositionable($rule, $sibling) >= 0) { + $position = $index; + break; + } } } }