Skip to content

Commit 8ef921e

Browse files
vitormattosvitormattos
authored
Merge pull request #12 from LibreSign/feat/quality-hardening-layout-tests
Quality hardening: parser robustness and mutation coverage improvements
2 parents ecc3f69 + dc4eaf7 commit 8ef921e

11 files changed

Lines changed: 574 additions & 67 deletions

‎infection.json5‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@
1212
"phpUnit": {
1313
"customPath": "vendor-bin/phpunit/vendor/bin/phpunit"
1414
},
15-
"minMsi": 70,
16-
"minCoveredMsi": 80,
15+
"minMsi": 75,
16+
"minCoveredMsi": 82,
1717
"testFramework": "phpunit",
1818
"timeout": 10
1919
}

‎phpmd.xml‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@
66
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
77
xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 https://pmd.github.io/pmd-6.55.0/ruleset_xml_schema.xsd">
88
<description>PHPMD baseline ruleset.</description>
9-
<rule ref="rulesets/cleancode.xml">
10-
<exclude name="ErrorControlOperator"/>
11-
</rule>
12-
<rule ref="rulesets/codesize.xml">
13-
<exclude name="CyclomaticComplexity"/>
14-
<exclude name="NPathComplexity"/>
15-
</rule>
9+
<rule ref="rulesets/cleancode.xml"/>
10+
<rule ref="rulesets/codesize.xml"/>
11+
<exclude-pattern>*tests/*</exclude-pattern>
1612
<rule ref="rulesets/design.xml"/>
1713
<rule ref="rulesets/naming.xml">
1814
<exclude name="ShortVariable"/>
1915
</rule>
16+
<rule ref="rulesets/naming.xml/ShortVariable">
17+
<properties>
18+
<property name="exceptions" value="x,y"/>
19+
</properties>
20+
</rule>
2021
</ruleset>

‎src/Html/SubsetHtmlParser.php‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@
1414

1515
final class SubsetHtmlParser
1616
{
17+
private const HTML_WRAPPER = '<?xml encoding="utf-8" ?><body>%s</body>';
18+
private const LIBXML_HTML_PARSE_FLAGS = 96; // LIBXML_NOERROR | LIBXML_NOWARNING
19+
1720
/** @var array<string, true> */
1821
private array $allowedTags = [
1922
'div' => true,
@@ -31,14 +34,14 @@ public function parse(string $html): array
3134
$dom = new DOMDocument('1.0', 'UTF-8');
3235
$prevLibxmlErrors = libxml_use_internal_errors(true);
3336
$dom->loadHTML(
34-
'<?xml encoding="utf-8" ?><body>' . $html . '</body>',
35-
LIBXML_NOERROR | LIBXML_NOWARNING,
37+
sprintf(self::HTML_WRAPPER, $html),
38+
self::LIBXML_HTML_PARSE_FLAGS,
3639
);
3740
libxml_clear_errors();
3841
libxml_use_internal_errors($prevLibxmlErrors);
3942

4043
$body = $dom->getElementsByTagName('body')->item(0);
41-
if (!$body instanceof DOMElement) {
44+
if ($body === null) {
4245
return [];
4346
}
4447

@@ -64,13 +67,14 @@ private function parseDomNode(DOMNode $node, string $inheritedStyle): ?Node
6467

6568
private function parseElementNode(DOMElement $node, string $inheritedStyle): Node
6669
{
67-
$tag = strtolower($node->tagName);
70+
$tag = $node->tagName;
6871
if (!isset($this->allowedTags[$tag])) {
6972
throw new UnsupportedSubsetException(sprintf('Tag <%s> is not supported.', $tag));
7073
}
7174

7275
$attributes = $this->collectAttributes($node);
7376
$effectiveStyle = $this->mergeStyle($inheritedStyle, $attributes['style'] ?? '');
77+
unset($attributes['style']);
7478
if ($effectiveStyle !== '') {
7579
$attributes['style'] = $effectiveStyle;
7680
}
@@ -112,22 +116,18 @@ private function parseTextNode(DOMNode $node, string $inheritedStyle): ?Node
112116
private function collectAttributes(DOMElement $node): array
113117
{
114118
$attributes = [];
115-
if ($node->attributes === null) {
116-
return $attributes;
117-
}
118-
119-
foreach ($node->attributes as $attribute) {
120-
$attributes[strtolower($attribute->name)] = $attribute->value;
119+
$nodeAttrs = $node->attributes;
120+
if ($nodeAttrs !== null) {
121+
foreach ($nodeAttrs as $attribute) {
122+
$attributes[$attribute->name] = trim($attribute->value);
123+
}
121124
}
122125

123126
return $attributes;
124127
}
125128

126129
private function mergeStyle(string $inheritedStyle, string $ownStyle): string
127130
{
128-
$inheritedStyle = trim($inheritedStyle);
129-
$ownStyle = trim($ownStyle);
130-
131131
if ($inheritedStyle === '') {
132132
return $ownStyle;
133133
}

‎src/Layout/LinearLayoutEngine.php‎

Lines changed: 52 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -33,31 +33,28 @@ public function layout(array $nodes, float $width, float $height): LayoutResult
3333

3434
foreach ($this->walk($nodes) as $node) {
3535
$style = $this->styleParser->parse($node->attributes['style'] ?? '');
36-
$margin = $this->parseBoxSpacing((string) $style->get('margin', '0'));
37-
$padding = $this->parseBoxSpacing((string) $style->get('padding', '0'));
36+
$margin = $this->parseBoxSpacing($this->styleValue($style, 'margin', '0'));
37+
$padding = $this->parseBoxSpacing($this->styleValue($style, 'padding', '0'));
3838

3939
$cursorY -= ($margin['top'] + $padding['top']);
4040

41-
$fontSize = $this->toPoints((string) $style->get('font-size', '10'));
42-
$lineHeight = max(
43-
$fontSize * 1.2,
44-
$this->toPoints((string) $style->get('line-height', (string) ($fontSize * 1.2))),
45-
);
41+
$fontSize = $this->toPoints($this->styleValue($style, 'font-size', '10'));
42+
$lineHeight = $this->resolveLineHeight($style, $fontSize);
4643
$fontAlias = $this->resolveFontAlias(
47-
(string) $style->get('font-family', 'helvetica'),
48-
(string) $style->get('font-weight', 'normal'),
44+
$this->styleValue($style, 'font-family', 'helvetica'),
45+
$this->styleValue($style, 'font-weight', 'normal'),
4946
);
5047

51-
$boxWidth = $this->toPoints((string) $style->get('width', '0'));
48+
$boxWidth = $this->toPoints($this->styleValue($style, 'width', '0'));
5249
if ($boxWidth <= 0) {
5350
$boxWidth = max($width - $margin['left'] - $margin['right'] - $padding['left'] - $padding['right'], 0);
5451
}
5552
$leftBase = $margin['left'] + $padding['left'];
5653
$rightBase = $leftBase + $boxWidth;
5754

5855
if ($node->tag === 'img') {
59-
$imgWidth = $this->toPoints((string) $style->get('width', '32'));
60-
$imgHeight = $this->toPoints((string) $style->get('height', '32'));
56+
$imgWidth = $this->toPoints($this->styleValue($style, 'width', '32'));
57+
$imgHeight = $this->toPoints($this->styleValue($style, 'height', '32'));
6158
if ($imgWidth <= 0) {
6259
$imgWidth = 32.0;
6360
}
@@ -88,20 +85,20 @@ public function layout(array $nodes, float $width, float $height): LayoutResult
8885
continue;
8986
}
9087

91-
$align = strtolower((string) $style->get('text-align', 'left'));
92-
$x = match ($align) {
88+
$align = strtolower($this->styleValue($style, 'text-align', 'left'));
89+
$lineX = match ($align) {
9390
'center' => $leftBase + ($boxWidth / 2.0),
9491
'right' => max($rightBase - 8.0, 0),
9592
default => $leftBase + 8.0,
9693
};
9794

9895
$lines[] = new LayoutLine(
9996
text: $text,
100-
x: $x,
97+
x: $lineX,
10198
y: max($cursorY, 0),
10299
fontSize: $fontSize,
103100
fontAlias: $fontAlias,
104-
rgbColor: (string) $style->get('color', '#000000'),
101+
rgbColor: $this->styleValue($style, 'color', '#000000'),
105102
);
106103

107104
$cursorY -= ($lineHeight + $margin['bottom'] + $padding['bottom']);
@@ -110,17 +107,33 @@ public function layout(array $nodes, float $width, float $height): LayoutResult
110107
return new LayoutResult(lines: $lines, images: $images);
111108
}
112109

110+
private function styleValue(
111+
\LibreSign\XObjectTemplate\Css\StyleMap $style,
112+
string $property,
113+
string $default,
114+
): string {
115+
return $style->get($property, $default) ?? $default;
116+
}
117+
113118
/**
114119
* @param list<Node> $nodes
115120
* @return list<Node>
116121
*/
117122
private function walk(array $nodes): array
118123
{
119124
$result = [];
120-
foreach ($nodes as $node) {
125+
$stack = array_reverse($nodes);
126+
127+
while ($stack !== []) {
128+
$node = array_pop($stack);
121129
$result[] = $node;
122-
if ($node->children !== []) {
123-
$result = [...$result, ...$this->walk($node->children)];
130+
131+
if ($node->children === []) {
132+
continue;
133+
}
134+
135+
foreach (array_reverse($node->children) as $child) {
136+
$stack[] = $child;
124137
}
125138
}
126139

@@ -129,11 +142,7 @@ private function walk(array $nodes): array
129142

130143
private function toPoints(string $value): float
131144
{
132-
$normalized = trim(strtolower($value));
133-
if ($normalized === '') {
134-
return 0.0;
135-
}
136-
145+
$normalized = strtolower($value);
137146
$number = (float) preg_replace('/[^0-9.\-]/', '', $normalized);
138147
if (str_ends_with($normalized, 'px')) {
139148
return $number * 0.75;
@@ -142,13 +151,27 @@ private function toPoints(string $value): float
142151
return $number;
143152
}
144153

154+
private function resolveLineHeight(
155+
\LibreSign\XObjectTemplate\Css\StyleMap $style,
156+
float $fontSize,
157+
): float {
158+
$defaultLineHeight = $fontSize * 1.2;
159+
$configuredLineHeight = $this->styleValue($style, 'line-height', '');
160+
161+
if ($configuredLineHeight === '') {
162+
return $defaultLineHeight;
163+
}
164+
165+
return max($defaultLineHeight, $this->toPoints($configuredLineHeight));
166+
}
167+
145168
/**
146169
* @return array{top: float, right: float, bottom: float, left: float}
147170
*/
148171
private function parseBoxSpacing(string $value): array
149172
{
150-
$tokens = preg_split('/\s+/', trim($value));
151-
$tokens = array_values(array_filter($tokens ?: [], static fn (string $token): bool => $token !== ''));
173+
preg_match_all('/\S+/', $value, $matches);
174+
$tokens = $matches[0];
152175

153176
if ($tokens === []) {
154177
return ['top' => 0.0, 'right' => 0.0, 'bottom' => 0.0, 'left' => 0.0];
@@ -174,7 +197,7 @@ private function parseBoxSpacing(string $value): array
174197

175198
private function resolveFontAlias(string $fontFamily, string $fontWeight): string
176199
{
177-
$primary = strtolower(trim(explode(',', $fontFamily)[0], " \t\n\r\0\x0B'\""));
200+
$primary = strtolower(explode(',', $fontFamily)[0]);
178201
$isBold = $this->isBoldWeight($fontWeight);
179202

180203
if (str_contains($primary, 'times')) {
@@ -190,13 +213,13 @@ private function resolveFontAlias(string $fontFamily, string $fontWeight): strin
190213

191214
private function isBoldWeight(string $fontWeight): bool
192215
{
193-
$normalized = strtolower(trim($fontWeight));
216+
$normalized = strtolower($fontWeight);
194217
if ($normalized === 'bold' || $normalized === 'bolder') {
195218
return true;
196219
}
197220

198221
if (is_numeric($normalized)) {
199-
return (int) $normalized >= 600;
222+
return $normalized >= 600;
200223
}
201224

202225
return false;

‎src/Pdf/ColorParser.php‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ public function toPdfRgb(string $hexColor): string
2020
return '0 0 0 rg';
2121
}
2222

23-
$r = round(hexdec(substr($hex, 0, 2)) / 255, 4);
24-
$g = round(hexdec(substr($hex, 2, 2)) / 255, 4);
25-
$b = round(hexdec(substr($hex, 4, 2)) / 255, 4);
23+
$channels = str_split($hex, 2);
24+
$redChannel = round(hexdec($channels[0]) / 255, 4);
25+
$greenChannel = round(hexdec($channels[1]) / 255, 4);
26+
$blueChannel = round(hexdec($channels[2]) / 255, 4);
2627

27-
return sprintf('%s %s %s rg', $r, $g, $b);
28+
return sprintf('%s %s %s rg', $redChannel, $greenChannel, $blueChannel);
2829
}
2930
}

‎src/Pdf/TemplateDocumentBuilder.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
namespace LibreSign\XObjectTemplate\Pdf;
99

10+
use Closure;
1011
use LibreSign\XObjectTemplate\Dto\CompileRequest;
1112
use LibreSign\XObjectTemplate\Dto\CompileResult;
1213
use LibreSign\XObjectTemplate\Layout\LayoutImage;
@@ -47,14 +48,18 @@
4748
],
4849
];
4950

51+
private Closure $clock;
52+
5053
/**
5154
* @param array<string, array<string, mixed>> $fontResources
5255
*/
5356
public function __construct(
5457
private PdfEscaper $pdfEscaper = new PdfEscaper(),
5558
private ColorParser $colorParser = new ColorParser(),
5659
private array $fontResources = self::DEFAULT_FONT_RESOURCES,
60+
?Closure $clock = null,
5761
) {
62+
$this->clock = $clock ?? static fn (): int => hrtime(true);
5863
}
5964

6065
public function build(
@@ -121,7 +126,7 @@ public function buildResources(LayoutResult $layout): array
121126
public function buildMetadata(LayoutResult $layout, int $startedAtNs, int $nodeCount = 0): array
122127
{
123128
return [
124-
'render_ms' => round((hrtime(true) - $startedAtNs) / 1_000_000, 3),
129+
'render_ms' => round((($this->clock)() - $startedAtNs) / 1_000_000, 3),
125130
'line_count' => count($layout->lines),
126131
'image_count' => count($layout->images),
127132
'node_count' => $nodeCount,
@@ -130,7 +135,7 @@ public function buildMetadata(LayoutResult $layout, int $startedAtNs, int $nodeC
130135

131136
public function withFontResources(array $fontResources): self
132137
{
133-
return new self($this->pdfEscaper, $this->colorParser, $fontResources);
138+
return new self($this->pdfEscaper, $this->colorParser, $fontResources, $this->clock);
134139
}
135140

136141
private function buildImageCommand(LayoutImage $image): string

‎tests/Unit/Css/InlineStyleParserTest.php‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
namespace LibreSign\XObjectTemplate\Tests\Unit\Css;
99

1010
use LibreSign\XObjectTemplate\Css\InlineStyleParser;
11+
use PHPUnit\Framework\Attributes\DataProvider;
1112
use PHPUnit\Framework\TestCase;
1213

1314
final class InlineStyleParserTest extends TestCase
@@ -42,5 +43,47 @@ public function testParseSkipsEmptyNameOrValueWithoutStoppingTheLoop(): void
4243
self::assertSame('red', $map->get('color'));
4344
self::assertSame('4', $map->get('padding'));
4445
self::assertNull($map->get('font-size'));
46+
self::assertNull($map->get(''));
47+
}
48+
49+
#[DataProvider('invalidChunkProvider')]
50+
public function testParseSkipsEachInvalidChunkVariantAndContinuesParsing(
51+
string $style,
52+
array $expectedPresent,
53+
array $expectedAbsent,
54+
): void {
55+
$parser = new InlineStyleParser();
56+
57+
$map = $parser->parse($style);
58+
59+
foreach ($expectedPresent as $name => $value) {
60+
self::assertSame($value, $map->get($name));
61+
}
62+
63+
foreach ($expectedAbsent as $name) {
64+
self::assertNull($map->get($name));
65+
}
66+
}
67+
68+
/**
69+
* @return iterable<string, array{
70+
* style: string,
71+
* expectedPresent: array<string, string>,
72+
* expectedAbsent: list<string>
73+
* }>
74+
*/
75+
public static function invalidChunkProvider(): iterable
76+
{
77+
yield 'empty name does not stop later declarations' => [
78+
'style' => ':red; color:blue; padding:1',
79+
'expectedPresent' => ['color' => 'blue', 'padding' => '1'],
80+
'expectedAbsent' => [''],
81+
];
82+
83+
yield 'empty value does not stop later declarations' => [
84+
'style' => 'font-size:; color:green; margin:2',
85+
'expectedPresent' => ['color' => 'green', 'margin' => '2'],
86+
'expectedAbsent' => ['font-size'],
87+
];
4588
}
4689
}

0 commit comments

Comments
 (0)