Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parse simple expressions #389

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion src/Value/CSSFunction.php
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ private static function parseName(ParserState $oParserState, bool $bIgnoreCase =
* @throws UnexpectedEOFException
* @throws UnexpectedTokenException
*/
private static function parseArguments(ParserState $oParserState)
protected static function parseArguments(ParserState $oParserState)
{
return Value::parseValue($oParserState, ['=', ' ', ',']);
}
Expand Down
27 changes: 27 additions & 0 deletions src/Value/Expression.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

namespace Sabberworm\CSS\Value;

use Sabberworm\CSS\OutputFormat;
use Sabberworm\CSS\Parsing\ParserState;

/**
* An `Expression` represents a special kind of value that is comprised of multiple components wrapped in parenthesis.
* Examle `height: (vh - 10);`.
*/
class Expression extends CSSFunction
{
/**
* @throws SourceException
* @throws UnexpectedEOFException
* @throws UnexpectedTokenException
*/
public static function parse(ParserState $oParserState, bool $bIgnoreCase = false): Expression
{
$oParserState->consume('(');
$aArguments = parent::parseArguments($oParserState);
$mResult = new Expression("", $aArguments, ',', $oParserState->currentLine());
$oParserState->consume(')');
return $mResult;
}
}
2 changes: 2 additions & 0 deletions src/Value/Value.php
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,8 @@ public static function parsePrimitiveValue(ParserState $oParserState)
$oValue = LineName::parse($oParserState);
} elseif ($oParserState->comes('U+')) {
$oValue = self::parseUnicodeRangeValue($oParserState);
} elseif ($oParserState->comes("(")) {
$oValue = Expression::parse($oParserState);
} else {
$sNextChar = $oParserState->peek(1);
try {
Expand Down
31 changes: 29 additions & 2 deletions tests/ParserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,20 @@ public function expandShorthands(): void
self::assertSame($sExpected, $oDoc->render());
}

/**
* @test
*/
public function parseExpressions(): void
{
$oDoc = self::parsedStructureForFile('expressions');
$sExpected = 'div {height: (vh - 10);}'
. "\n"
. 'div {height: (vh - 10)/2;}'
. "\n"
. 'div {height: max(5,(vh - 10));}';
self::assertSame($sExpected, $oDoc->render());
}

/**
* @test
*/
Expand Down Expand Up @@ -689,8 +703,8 @@ public function calcNestedInFile(): void
public function invalidCalcInFile(): void
{
$oDoc = self::parsedStructureForFile('calc-invalid', Settings::create()->withMultibyteSupport(true));
$sExpected = 'div {}
div {}
$sExpected = 'div {height: calc (25% - 1em);}
div {height: calc (25% - 1em);}
div {}
div {height: -moz-calc;}
div {height: calc;}';
Expand Down Expand Up @@ -1239,6 +1253,19 @@ public function lonelyImport(): void
self::assertSame($sExpected, $oDoc->render());
}

/**
* @test
*/
public function functionArithmeticInFile(): void
{
$oDoc = self::parsedStructureForFile('function-arithmetic', Settings::create()->withMultibyteSupport(true));
$sExpected = 'div {height: max(300,vh + 10);}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAICT, vh without a number in front is still not valid and I’d prefer it would throw an error in strict mode.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, it is invalid, however this seems to be outside of the scope of these changes. I guess we need a separate issue for this. Also is it a concern of the parser to validate the semantic meaning of identifiers it encounters?

Copy link
Contributor

@JakeQZ JakeQZ Feb 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also is it a concern of the parser to validate the semantic meaning of identifiers it encounters?

I agree it's not.

Moving forwards, we've agreed to replace strict mode parsing with a logging mechanism. And try to replicate what most browsers do when encountering invalid syntax. A unit with no number in front should therefore probably be parsed as zero of that unit. And, as a bonus, a separate PR would log the lack of a number.

So, for now, we don't need to worry about strict mode, but should create issues for things that could be logged in future.

div {height: max(300,vh - 10);}
div {height: max(300,vh * 10);}
div {height: max(300,vh / 10);}';
self::assertSame($sExpected, $oDoc->render());
}

public function escapedSpecialCaseTokens(): void
{
$oDoc = $this->parsedStructureForFile('escaped-tokens');
Expand Down
63 changes: 63 additions & 0 deletions tests/Value/ExpressionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
<?php

namespace Sabberworm\CSS\Tests\Value;

use PHPUnit\Framework\TestCase;
use Sabberworm\CSS\Parsing\ParserState;
use Sabberworm\CSS\Settings;
use Sabberworm\CSS\Value\Value;
use Sabberworm\CSS\Value\ValueList;
use Sabberworm\CSS\Value\Expression;
use Sabberworm\CSS\Rule\Rule;

/**
* @covers \Sabberworm\CSS\Value\Expression
*/
final class ExpressionTest extends TestCase
{
/**
* @return array<0, array{string: string}>
*/
public static function provideExpressions(): array
{
return [
[
'input' => '(vh - 10) / 2',
'expected_output' => '(vh - 10)/2',
'expression_index' => 0,
],
[
'input' => 'max(5, (vh - 10))',
'expected_output' => 'max(5,(vh - 10))',
'expression_index' => 1
],
];
}

/**
* @test
*
* @dataProvider provideExpressions
*/
public function parseExpressions(string $input, string $expected, int $expression_index): void
{
$val = Value::parseValue(
new ParserState($input, Settings::create()),
$this->getDelimiters('height')
);

self::assertInstanceOf(ValueList::class, $val);
self::assertInstanceOf(Expression::class, $val->getListComponents()[$expression_index]);
self::assertSame($expected, (string) $val);
}

private function getDelimiters(string $rule): array
{
$closure = function($rule) {
return self::listDelimiterForRule($rule);
};

$getter = $closure->bindTo(null, Rule::class);
return $getter($rule);
}
}
11 changes: 11 additions & 0 deletions tests/fixtures/expressions.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
div {
height: (vh - 10);
}

div {
height: (vh - 10) / 2;
}

div {
height: max(5, (vh - 10));
}
12 changes: 12 additions & 0 deletions tests/fixtures/function-arithmetic.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
div {
height: max(300, vh + 10);
}
div {
height: max(300, vh - 10);
}
div {
height: max(300, vh * 10);
}
div {
height: max(300, vh / 10);
}
Loading