From 93ab0177fbbb118ba3d9b60a806f167e6994bcd6 Mon Sep 17 00:00:00 2001
From: Jake Hotson <jake.github@qzdesign.co.uk>
Date: Wed, 4 Jun 2025 00:58:51 +0100
Subject: [PATCH] [BUGFIX] Ensure valid position after `AddRule`

This is the backport of #1263, #1262 and #1265.

Since `getLineNo()` and `getColNo()`, which always returned an `int`, are
deprecated, using their replacements, which may return `null`, for `Rule`s in
a `RuleSet` may cause issues.
The fixes ensure that such `Rule`s will always have a valid position, so the
new methods will not return `null` in that situation, and a straightforward
replacement can be done.

Part of #974.
---
 CHANGELOG.md                       |  6 +++++-
 src/RuleSet/RuleSet.php            |  9 +++++++--
 tests/Unit/RuleSet/RuleSetTest.php | 13 +++----------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index c1535719..74afb6a6 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,7 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 - Methods `getLineNumber` and `getColumnNumber` which return a nullable `int`
   for the following classes:
   `Comment`, `CSSList`, `SourceException`, `Charset`, `CSSNamespace`, `Import`,
-  `Rule`, `DeclarationBlock`, `RuleSet`, `CSSFunction`, `Value` (#1225)
+  `Rule`, `DeclarationBlock`, `RuleSet`, `CSSFunction`, `Value` (#1225, #1263)
 - `Positionable` interface for CSS items that may have a position
   (line and perhaps column number) in the parsed CSS (#1221)
 
@@ -55,6 +55,10 @@ This project adheres to [Semantic Versioning](https://semver.org/).
 
 ### Fixed
 
+- Set line number when `RuleSet::addRule()` called with only column number set
+  (#1265)
+- Ensure first rule added with `RuleSet::addRule()` has valid position (#1262)
+
 ## 8.8.0: Bug fixes and deprecations
 
 ### Added
diff --git a/src/RuleSet/RuleSet.php b/src/RuleSet/RuleSet.php
index 1658154c..0110f50e 100644
--- a/src/RuleSet/RuleSet.php
+++ b/src/RuleSet/RuleSet.php
@@ -121,14 +121,19 @@ public function addRule(Rule $oRule, $oSibling = null)
                 $oRule->setPosition($oSibling->getLineNo(), $oSibling->getColNo() - 1);
             }
         }
-        if ($oRule->getLineNo() === 0 && $oRule->getColNo() === 0) {
+        if ($oRule->getLineNumber() === null) {
             //this node is added manually, give it the next best line
+            $columnNumber = $oRule->getColNo();
             $rules = $this->getRules();
             $pos = count($rules);
             if ($pos > 0) {
                 $last = $rules[$pos - 1];
-                $oRule->setPosition($last->getLineNo() + 1, 0);
+                $oRule->setPosition($last->getLineNo() + 1, $columnNumber);
+            } else {
+                $oRule->setPosition(1, $columnNumber);
             }
+        } elseif ($oRule->getColumnNumber() === null) {
+            $oRule->setPosition($oRule->getLineNumber(), 0);
         }
 
         array_splice($this->aRules[$sRule], $iPosition, 0, [$oRule]);
diff --git a/tests/Unit/RuleSet/RuleSetTest.php b/tests/Unit/RuleSet/RuleSetTest.php
index 303e718b..baa7d316 100644
--- a/tests/Unit/RuleSet/RuleSetTest.php
+++ b/tests/Unit/RuleSet/RuleSetTest.php
@@ -77,10 +77,6 @@ public function addRuleWithoutSiblingAddsRuleAfterInitialRulesAndSetsValidLineAn
         array $initialPropertyNames,
         string $propertyNameToAdd
     ) {
-        if ($initialPropertyNames === []) {
-            self::markTestSkipped('currently broken - first rule added does not have valid line number set');
-        }
-
         $subject = new ConcreteRuleSet();
         $ruleToAdd = new Rule($propertyNameToAdd);
         self::setRulesFromPropertyNames($subject, $initialPropertyNames);
@@ -106,8 +102,6 @@ public function addRuleWithOnlyLineNumberAddsRuleAndSetsColumnNumberPreservingLi
         array $initialPropertyNames,
         string $propertyNameToAdd
     ) {
-        self::markTestSkipped('currently broken - does not set column number');
-
         $subject = new ConcreteRuleSet();
         $ruleToAdd = new Rule($propertyNameToAdd);
         $ruleToAdd->setPosition(42);
@@ -128,12 +122,10 @@ public function addRuleWithOnlyLineNumberAddsRuleAndSetsColumnNumberPreservingLi
      *
      * @param list<string> $initialPropertyNames
      */
-    public function addRuleWithOnlyColumnNumberAddsRuleAndSetsLineNumberPreservingColumnNumber(
+    public function addRuleWithOnlyColumnNumberAddsRuleAfterInitialRulesAndSetsLineNumberPreservingColumnNumber(
         array $initialPropertyNames,
         string $propertyNameToAdd
     ) {
-        self::markTestSkipped('currently broken - does not preserve column number');
-
         $subject = new ConcreteRuleSet();
         $ruleToAdd = new Rule($propertyNameToAdd);
         $ruleToAdd->setPosition(null, 42);
@@ -141,7 +133,8 @@ public function addRuleWithOnlyColumnNumberAddsRuleAndSetsLineNumberPreservingCo
 
         $subject->addRule($ruleToAdd);
 
-        self::assertContains($ruleToAdd, $subject->getRules());
+        $rules = $subject->getRules();
+        self::assertSame($ruleToAdd, \end($rules));
         self::assertInternalType('int', $ruleToAdd->getLineNumber(), 'line number not set');
         self::assertGreaterThanOrEqual(1, $ruleToAdd->getLineNumber(), 'line number not valid');
         self::assertSame(42, $ruleToAdd->getColumnNumber(), 'column number not preserved');