Skip to content

Commit 6455dd1

Browse files
authored
[TASK] Have comsumeWhiteSpace() return the consumed (#1477)
Instead, have the comments array passed by reference to be appended to. This avoids various methods having to do an `array_merge`. Also, knowing if any whitespace was consumed will be needed for #1471.
1 parent 0135077 commit 6455dd1

5 files changed

Lines changed: 163 additions & 10 deletions

File tree

src/CSSList/CSSList.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public static function parseList(ParserState $parserState, CSSList $list): void
6868
$usesLenientParsing = $parserState->getSettings()->usesLenientParsing();
6969
$comments = [];
7070
while (!$parserState->isEnd()) {
71-
$comments = \array_merge($comments, $parserState->consumeWhiteSpace());
71+
$parserState->consumeWhiteSpace($comments);
7272
$listItem = null;
7373
if ($usesLenientParsing) {
7474
try {
@@ -93,7 +93,8 @@ public static function parseList(ParserState $parserState, CSSList $list): void
9393
$listItem->addComments($comments);
9494
$list->append($listItem);
9595
}
96-
$comments = $parserState->consumeWhiteSpace();
96+
$comments = [];
97+
$parserState->consumeWhiteSpace($comments);
9798
}
9899
$list->addComments($comments);
99100
if (!$isRoot && !$usesLenientParsing) {

src/Parsing/ParserState.php

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,11 @@ public function parseCharacter(bool $isForIdentifier): ?string
188188
}
189189

190190
/**
191-
* @return list<Comment>
191+
* Consumes whitespace and/or comments until the next non-whitespace character that isn't a slash opening a comment.
192+
*
193+
* @param list<Comment> $comments Any comments consumed will be appended to this array.
194+
*
195+
* @return string the whitespace consumed, without the comments
192196
*
193197
* @throws UnexpectedEOFException
194198
* @throws UnexpectedTokenException
@@ -197,12 +201,12 @@ public function parseCharacter(bool $isForIdentifier): ?string
197201
* This method may change the state of the object by advancing the internal position;
198202
* it does not simply 'get' a value.
199203
*/
200-
public function consumeWhiteSpace(): array
204+
public function consumeWhiteSpace(array &$comments = []): string
201205
{
202-
$comments = [];
206+
$consumed = '';
203207
do {
204208
while (preg_match('/\\s/isSu', $this->peek()) === 1) {
205-
$this->consume(1);
209+
$consumed .= $this->consume(1);
206210
}
207211
if ($this->parserSettings->usesLenientParsing()) {
208212
try {
@@ -219,7 +223,7 @@ public function consumeWhiteSpace(): array
219223
}
220224
} while ($comment instanceof Comment);
221225

222-
return $comments;
226+
return $consumed;
223227
}
224228

225229
/**

src/Rule/Rule.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,14 +65,15 @@ public function __construct(string $rule, ?int $lineNumber = null, ?int $columnN
6565
*/
6666
public static function parse(ParserState $parserState, array $commentsBeforeRule = []): Rule
6767
{
68-
$comments = \array_merge($commentsBeforeRule, $parserState->consumeWhiteSpace());
68+
$comments = $commentsBeforeRule;
69+
$parserState->consumeWhiteSpace($comments);
6970
$rule = new Rule(
7071
$parserState->parseIdentifier(!$parserState->comes('--')),
7172
$parserState->currentLine(),
7273
$parserState->currentColumn()
7374
);
75+
$parserState->consumeWhiteSpace($comments);
7476
$rule->setComments($comments);
75-
$rule->addComments($parserState->consumeWhiteSpace());
7677
$parserState->consume(':');
7778
$value = Value::parseValue($parserState, self::listDelimiterForRule($rule->getRule()));
7879
$rule->setValue($value);

src/RuleSet/RuleSet.php

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,8 @@ public static function parseRuleSet(ParserState $parserState, RuleSet $ruleSet):
5959
$parserState->consume(';');
6060
}
6161
while (true) {
62-
$commentsBeforeRule = $parserState->consumeWhiteSpace();
62+
$commentsBeforeRule = [];
63+
$parserState->consumeWhiteSpace($commentsBeforeRule);
6364
if ($parserState->comes('}')) {
6465
break;
6566
}

tests/Unit/Parsing/ParserStateTest.php

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use Sabberworm\CSS\Comment\Comment;
99
use Sabberworm\CSS\Parsing\ParserState;
1010
use Sabberworm\CSS\Settings;
11+
use TRegx\PhpUnit\DataProviders\DataProvider;
1112

1213
/**
1314
* @covers \Sabberworm\CSS\Parsing\ParserState
@@ -157,4 +158,149 @@ public function consumeIfComesUpdatesLineNumber(): void
157158

158159
self::assertSame(2, $subject->currentLine());
159160
}
161+
162+
/**
163+
* @return array<non-empty-string, array{0: string, 1: string}>
164+
*/
165+
public static function provideContentWhichMayHaveWhitespaceOrCommentsAndExpectedConsumption(): array
166+
{
167+
return [
168+
'nothing' => ['', ''],
169+
'space' => [' ', ' '],
170+
'tab' => ["\t", "\t"],
171+
'line feed' => ["\n", "\n"],
172+
'carriage return' => ["\r", "\r"],
173+
'two spaces' => [' ', ' '],
174+
'comment' => ['/*hello*/', ''],
175+
'comment with space to the left' => [' /*hello*/', ' '],
176+
'comment with space to the right' => ['/*hello*/ ', ' '],
177+
'two comments' => ['/*hello*//*bye*/', ''],
178+
'two comments with space between' => ['/*hello*/ /*bye*/', ' '],
179+
'two comments with line feed between' => ["/*hello*/\n/*bye*/", "\n"],
180+
];
181+
}
182+
183+
/**
184+
* @test
185+
*
186+
* @dataProvider provideContentWhichMayHaveWhitespaceOrCommentsAndExpectedConsumption
187+
*/
188+
public function consumeWhiteSpaceReturnsTheConsumed(
189+
string $whitespaceMaybeWithComments,
190+
string $expectedConsumption
191+
): void {
192+
$subject = new ParserState($whitespaceMaybeWithComments, Settings::create());
193+
194+
$result = $subject->consumeWhiteSpace();
195+
196+
self::assertSame($expectedConsumption, $result);
197+
}
198+
199+
/**
200+
* @test
201+
*/
202+
public function consumeWhiteSpaceExtractsComment(): void
203+
{
204+
$commentText = 'Did they get you to trade your heroes for ghosts?';
205+
$subject = new ParserState('/*' . $commentText . '*/', Settings::create());
206+
207+
$result = [];
208+
$subject->consumeWhiteSpace($result);
209+
210+
self::assertInstanceOf(Comment::class, $result[0]);
211+
self::assertSame($commentText, $result[0]->getComment());
212+
}
213+
214+
/**
215+
* @test
216+
*/
217+
public function consumeWhiteSpaceExtractsTwoComments(): void
218+
{
219+
$commentText1 = 'Hot ashes for trees? Hot air for a cool breeze?';
220+
$commentText2 = 'Cold comfort for change? Did you exchange';
221+
$subject = new ParserState('/*' . $commentText1 . '*//*' . $commentText2 . '*/', Settings::create());
222+
223+
$result = [];
224+
$subject->consumeWhiteSpace($result);
225+
226+
self::assertInstanceOf(Comment::class, $result[0]);
227+
self::assertSame($commentText1, $result[0]->getComment());
228+
self::assertInstanceOf(Comment::class, $result[1]);
229+
self::assertSame($commentText2, $result[1]->getComment());
230+
}
231+
232+
/**
233+
* @return array<non-empty-string, array{0: non-empty-string}>
234+
*/
235+
public static function provideWhitespace(): array
236+
{
237+
return [
238+
'space' => [' '],
239+
'tab' => ["\t"],
240+
'line feed' => ["\n"],
241+
'carriage return' => ["\r"],
242+
'two spaces' => [' '],
243+
];
244+
}
245+
246+
/**
247+
* @test
248+
*
249+
* @param non-empty-string $whitespace
250+
*
251+
* @dataProvider provideWhitespace
252+
*/
253+
public function consumeWhiteSpaceExtractsCommentWithSurroundingWhitespace(string $whitespace): void
254+
{
255+
$commentText = 'A walk-on part in the war for a lead role in a cage?';
256+
$subject = new ParserState($whitespace . '/*' . $commentText . '*/' . $whitespace, Settings::create());
257+
258+
$result = [];
259+
$subject->consumeWhiteSpace($result);
260+
261+
self::assertInstanceOf(Comment::class, $result[0]);
262+
self::assertSame($commentText, $result[0]->getComment());
263+
}
264+
265+
/**
266+
* @return array<non-empty-string, array{0: non-empty-string}>
267+
*/
268+
public static function provideNonWhitespace(): array
269+
{
270+
return [
271+
'number' => ['7'],
272+
'uppercase letter' => ['B'],
273+
'lowercase letter' => ['c'],
274+
'symbol' => ['@'],
275+
'sequence of non-whitespace characters' => ['Hello!'],
276+
'sequence of characters including space which isn\'t first' => ['Oh no!'],
277+
];
278+
}
279+
280+
/**
281+
* @return DataProvider<non-empty-string, array{0: non-empty-string, 1: string}>
282+
*/
283+
public function provideNonWhitespaceAndContentWithPossibleWhitespaceOrComments(): DataProvider
284+
{
285+
return DataProvider::cross(
286+
self::provideNonWhitespace(),
287+
self::provideContentWhichMayHaveWhitespaceOrCommentsAndExpectedConsumption()
288+
);
289+
}
290+
291+
/**
292+
* @test
293+
*
294+
* @param non-empty-string $nonWhitespace
295+
*
296+
* @dataProvider provideNonWhitespaceAndContentWithPossibleWhitespaceOrComments
297+
*/
298+
public function consumeWhiteSpaceStopsAtNonWhitespace(string $nonWhitespace, string $whitespace): void
299+
{
300+
$subject = new ParserState($whitespace . $nonWhitespace, Settings::create());
301+
302+
$subject->consumeWhiteSpace();
303+
304+
self::assertTrue($subject->comes($nonWhitespace));
305+
}
160306
}

0 commit comments

Comments
 (0)