Skip to content

Commit 098f8d8

Browse files
committed
[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.
1 parent 7a469b2 commit 098f8d8

File tree

3 files changed

+15
-13
lines changed

3 files changed

+15
-13
lines changed

CHANGELOG.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).
1515
- Methods `getLineNumber` and `getColumnNumber` which return a nullable `int`
1616
for the following classes:
1717
`Comment`, `CSSList`, `SourceException`, `Charset`, `CSSNamespace`, `Import`,
18-
`Rule`, `DeclarationBlock`, `RuleSet`, `CSSFunction`, `Value` (#1225)
18+
`Rule`, `DeclarationBlock`, `RuleSet`, `CSSFunction`, `Value` (#1225, #1263)
1919
- `Positionable` interface for CSS items that may have a position
2020
(line and perhaps column number) in the parsed CSS (#1221)
2121

@@ -55,6 +55,10 @@ This project adheres to [Semantic Versioning](https://semver.org/).
5555

5656
### Fixed
5757

58+
- Set line number when `RuleSet::addRule()` called with only column number set
59+
(#1265)
60+
- Ensure first rule added with `RuleSet::addRule()` has valid position (#1262)
61+
5862
## 8.8.0: Bug fixes and deprecations
5963

6064
### Added

src/RuleSet/RuleSet.php

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,14 +121,19 @@ public function addRule(Rule $oRule, $oSibling = null)
121121
$oRule->setPosition($oSibling->getLineNo(), $oSibling->getColNo() - 1);
122122
}
123123
}
124-
if ($oRule->getLineNo() === 0 && $oRule->getColNo() === 0) {
124+
if ($oRule->getLineNumber() === null) {
125125
//this node is added manually, give it the next best line
126+
$columnNumber = $ruleToAdd->getColumnNumber() ?? 0;
126127
$rules = $this->getRules();
127128
$pos = count($rules);
128129
if ($pos > 0) {
129130
$last = $rules[$pos - 1];
130-
$oRule->setPosition($last->getLineNo() + 1, 0);
131+
$oRule->setPosition($last->getLineNo() + 1, $columnNumber);
132+
} else {
133+
$oRule->setPosition(1, $columnNumber);
131134
}
135+
} elseif ($ruleToAdd->getColumnNumber() === null) {
136+
$ruleToAdd->setPosition($ruleToAdd->getLineNumber(), 0);
132137
}
133138

134139
array_splice($this->aRules[$sRule], $iPosition, 0, [$oRule]);

tests/Unit/RuleSet/RuleSetTest.php

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,6 @@ public function addRuleWithoutSiblingAddsRuleAfterInitialRulesAndSetsValidLineAn
7777
array $initialPropertyNames,
7878
string $propertyNameToAdd
7979
) {
80-
if ($initialPropertyNames === []) {
81-
self::markTestSkipped('currently broken - first rule added does not have valid line number set');
82-
}
83-
8480
$subject = new ConcreteRuleSet();
8581
$ruleToAdd = new Rule($propertyNameToAdd);
8682
self::setRulesFromPropertyNames($subject, $initialPropertyNames);
@@ -106,8 +102,6 @@ public function addRuleWithOnlyLineNumberAddsRuleAndSetsColumnNumberPreservingLi
106102
array $initialPropertyNames,
107103
string $propertyNameToAdd
108104
) {
109-
self::markTestSkipped('currently broken - does not set column number');
110-
111105
$subject = new ConcreteRuleSet();
112106
$ruleToAdd = new Rule($propertyNameToAdd);
113107
$ruleToAdd->setPosition(42);
@@ -128,20 +122,19 @@ public function addRuleWithOnlyLineNumberAddsRuleAndSetsColumnNumberPreservingLi
128122
*
129123
* @param list<string> $initialPropertyNames
130124
*/
131-
public function addRuleWithOnlyColumnNumberAddsRuleAndSetsLineNumberPreservingColumnNumber(
125+
public function addRuleWithOnlyColumnNumberAddsRuleAfterInitialRulesAndSetsLineNumberPreservingColumnNumber(
132126
array $initialPropertyNames,
133127
string $propertyNameToAdd
134128
) {
135-
self::markTestSkipped('currently broken - does not preserve column number');
136-
137129
$subject = new ConcreteRuleSet();
138130
$ruleToAdd = new Rule($propertyNameToAdd);
139131
$ruleToAdd->setPosition(null, 42);
140132
self::setRulesFromPropertyNames($subject, $initialPropertyNames);
141133

142134
$subject->addRule($ruleToAdd);
143135

144-
self::assertContains($ruleToAdd, $subject->getRules());
136+
$rules = $subject->getRules();
137+
self::assertSame($ruleToAdd, \end($rules));
145138
self::assertInternalType('int', $ruleToAdd->getLineNumber(), 'line number not set');
146139
self::assertGreaterThanOrEqual(1, $ruleToAdd->getLineNumber(), 'line number not valid');
147140
self::assertSame(42, $ruleToAdd->getColumnNumber(), 'column number not preserved');

0 commit comments

Comments
 (0)