Skip to content

Commit 354026c

Browse files
vjikvjik
authored
Fix empty string and "0" keys usage for CSS/JS in WebView + Improve psalm (#273)
1 parent 3af9738 commit 354026c

5 files changed

Lines changed: 156 additions & 27 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,10 @@
66
- Chg #271: Remove deprecated methods `withDefaultExtension()` and `getDefaultExtension()` from `ViewInterface` (@vjik)
77
- Chg #271: Rename configuration parameter `defaultExtension` to `fallbackExtension` (@vjik)
88
- Chg #272: Add variadic parameter `$default` to `ViewInterface::getParameter()` (@vjik)
9+
- Bug #273: Fix empty string and "0" keys in `WebView` methods: `registerCss()`, `registerStyleTag()`,
10+
`registerCssFile()`, `registerJs()`, `registerScriptTag()` and `registerJsFile()` (@vjik)
11+
- Enh #273: Use more specific psalm types in results of `WebView` methods: `getLinkTags()`, `getCss()`, `getCssFiles()`,
12+
`getJs()` and `getJsFiles()` (@vjik)
913

1014
## 10.0.0 June 28, 2024
1115

‎psalm.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,5 @@
1515
</projectFiles>
1616
<issueHandlers>
1717
<MixedAssignment errorLevel="suppress" />
18-
<RiskyTruthyFalsyComparison errorLevel="suppress" />
1918
</issueHandlers>
2019
</psalm>

‎psalm83.xml‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
</projectFiles>
1616
<issueHandlers>
1717
<MixedAssignment errorLevel="suppress" />
18-
<RiskyTruthyFalsyComparison errorLevel="suppress" />
1918
<MissingClassConstType errorLevel="suppress" />
2019
</issueHandlers>
2120
</psalm>

‎src/State/WebViewState.php‎

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ final class WebViewState
4141

4242
/**
4343
* @var array The registered link tags.
44-
* @psalm-var array<int, Link[]>
44+
* @psalm-var array<int, non-empty-array<array-key, Link>>
4545
*
4646
* @see registerLink()
4747
* @see registerLinkTag()
@@ -50,31 +50,31 @@ final class WebViewState
5050

5151
/**
5252
* @var array The registered CSS code blocks.
53-
* @psalm-var array<int, string[]|Style[]>
53+
* @psalm-var array<int, non-empty-array<array-key, string|Style>>
5454
*
5555
* {@see registerCss()}
5656
*/
5757
private array $css = [];
5858

5959
/**
6060
* @var array The registered CSS files.
61-
* @psalm-var array<int, string[]>
61+
* @psalm-var array<int, non-empty-array<array-key, string>>
6262
*
6363
* {@see registerCssFile()}
6464
*/
6565
private array $cssFiles = [];
6666

6767
/**
6868
* @var array The registered JS code blocks
69-
* @psalm-var array<int, string[]|Script[]>
69+
* @psalm-var array<int, non-empty-array<array-key, string|Script>>
7070
*
7171
* {@see registerJs()}
7272
*/
7373
private array $js = [];
7474

7575
/**
7676
* @var array The registered JS files.
77-
* @psalm-var array<int, string[]>
77+
* @psalm-var array<int, non-empty-array<array-key, string>>
7878
*
7979
* {@see registerJsFile()}
8080
*/
@@ -98,7 +98,7 @@ public function getMetaTags(): array
9898

9999
/**
100100
* @return array The registered link tags.
101-
* @psalm-return array<int, Link[]>
101+
* @psalm-return array<int, non-empty-array<array-key, Link>>
102102
*/
103103
public function getLinkTags(): array
104104
{
@@ -107,7 +107,7 @@ public function getLinkTags(): array
107107

108108
/**
109109
* @return array The registered CSS code blocks.
110-
* @psalm-return array<int, string[]|Style[]>
110+
* @psalm-return array<int, non-empty-array<array-key, string|Style>>
111111
*/
112112
public function getCss(): array
113113
{
@@ -116,7 +116,7 @@ public function getCss(): array
116116

117117
/**
118118
* @return array The registered CSS files.
119-
* @psalm-return array<int, string[]>
119+
* @psalm-return array<int, non-empty-array<array-key, string>>
120120
*/
121121
public function getCssFiles(): array
122122
{
@@ -125,7 +125,7 @@ public function getCssFiles(): array
125125

126126
/**
127127
* @return array The registered JS code blocks
128-
* @psalm-return array<int, string[]|Script[]>
128+
* @psalm-return array<int, non-empty-array<array-key, string|Script>>
129129
*/
130130
public function getJs(): array
131131
{
@@ -134,7 +134,7 @@ public function getJs(): array
134134

135135
/**
136136
* @return array The registered JS files.
137-
* @psalm-return array<int, string[]>
137+
* @psalm-return array<int, non-empty-array<array-key, string>>
138138
*/
139139
public function getJsFiles(): array
140140
{
@@ -224,18 +224,17 @@ public function registerLinkTag(Link $link, int $position = WebView::POSITION_HE
224224
* Registers a CSS code block.
225225
*
226226
* @param string $css The content of the CSS code block to be registered.
227-
* @param string|null $key The key that identifies the CSS code block. If null, it will use $css as the key.
228-
* If two CSS code blocks are registered with the same key, the latter will overwrite the former.
229227
* @param array $attributes The HTML attributes for the {@see Style} tag.
228+
* @param string|null $key The key that identifies the CSS code block. If `null`, it will use `$css` as the key.
229+
* If two CSS code blocks are registered with the same key, the latter will overwrite the former.
230230
*/
231231
public function registerCss(
232232
string $css,
233233
int $position = WebView::POSITION_HEAD,
234234
array $attributes = [],
235235
?string $key = null
236236
): void {
237-
$key = $key ?: md5($css);
238-
$this->css[$position][$key] = $attributes === [] ? $css : Html::style($css, $attributes);
237+
$this->css[$position][$key ?? md5($css)] = $attributes === [] ? $css : Html::style($css, $attributes);
239238
}
240239

241240
/**
@@ -266,8 +265,7 @@ public function registerCssFromFile(
266265
*/
267266
public function registerStyleTag(Style $style, int $position = WebView::POSITION_HEAD, ?string $key = null): void
268267
{
269-
$key = $key ?: md5($style->render());
270-
$this->css[$position][$key] = $style;
268+
$this->css[$position][$key ?? md5($style->render())] = $style;
271269
}
272270

273271
/**
@@ -280,20 +278,20 @@ public function registerStyleTag(Style $style, int $position = WebView::POSITION
280278
* @param string $url The CSS file to be registered.
281279
* @param array $options the HTML attributes for the link tag. Please refer to {@see \Yiisoft\Html\Html::cssFile()}
282280
* for the supported options.
283-
* @param string|null $key The key that identifies the CSS script file. If null, it will use $url as the key.
281+
* @param string|null $key The key that identifies the CSS script file. If `null`, it will use `$url` as the key.
284282
* If two CSS files are registered with the same key, the latter will overwrite the former.
285283
*/
286284
public function registerCssFile(
287285
string $url,
288286
int $position = WebView::POSITION_HEAD,
289287
array $options = [],
290-
string $key = null
288+
?string $key = null
291289
): void {
292290
if (!$this->isValidCssPosition($position)) {
293291
throw new InvalidArgumentException('Invalid position of CSS file.');
294292
}
295293

296-
$this->cssFiles[$position][$key ?: $url] = Html::cssFile($url, $options)->render();
294+
$this->cssFiles[$position][$key ?? $url] = Html::cssFile($url, $options)->render();
297295
}
298296

299297
/**
@@ -337,13 +335,12 @@ public function addCssStrings(array $cssStrings): void
337335
* - {@see WebView::POSITION_END}: at the end of the body section. This is the default value.
338336
* - {@see WebView::POSITION_LOAD}: executed when HTML page is completely loaded.
339337
* - {@see WebView::POSITION_READY}: executed when HTML document composition is ready.
340-
* @param string|null $key The key that identifies the JS code block. If null, it will use $js as the key.
338+
* @param string|null $key The key that identifies the JS code block. If `null`, it will use `$js` as the key.
341339
* If two JS code blocks are registered with the same key, the latter will overwrite the former.
342340
*/
343341
public function registerJs(string $js, int $position = WebView::POSITION_END, ?string $key = null): void
344342
{
345-
$key = $key ?: md5($js);
346-
$this->js[$position][$key] = $js;
343+
$this->js[$position][$key ?? md5($js)] = $js;
347344
}
348345

349346
/**
@@ -353,8 +350,7 @@ public function registerJs(string $js, int $position = WebView::POSITION_END, ?s
353350
*/
354351
public function registerScriptTag(Script $script, int $position = WebView::POSITION_END, ?string $key = null): void
355352
{
356-
$key = $key ?: md5($script->render());
357-
$this->js[$position][$key] = $script;
353+
$this->js[$position][$key ?? md5($script->render())] = $script;
358354
}
359355

360356
/**
@@ -389,7 +385,7 @@ public function registerJsFile(
389385
throw new InvalidArgumentException('Invalid position of JS file.');
390386
}
391387

392-
$this->jsFiles[$position][$key ?: $url] = Html::javaScriptFile($url, $options)->render();
388+
$this->jsFiles[$position][$key ?? $url] = Html::javaScriptFile($url, $options)->render();
393389
}
394390

395391
/**

‎tests/WebViewTest.php‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,27 @@ public function testRegisterJsFileWithPosition(string $expected, int $position):
118118
$this->assertStringContainsString($expected, $html);
119119
}
120120

121+
public function testRegisterJsFileWithKeyEdgeCase(): void
122+
{
123+
$webView = TestHelper::createWebView()
124+
->registerJsFile('main.js')
125+
->registerJsFile('main.js', key: '')
126+
->registerJsFile('main.js', key: '0')
127+
->registerJsFile('main.js', key: 'four');
128+
129+
$html = $webView->render('positions.php');
130+
131+
$this->assertStringContainsString(
132+
<<<HTML
133+
[ENDBODY]<script src="main.js"></script>
134+
<script src="main.js"></script>
135+
<script src="main.js"></script>
136+
<script src="main.js"></script>[/ENDBODY]
137+
HTML,
138+
$html
139+
);
140+
}
141+
121142
public function testRegisterStyleTag(): void
122143
{
123144
$webView = TestHelper::createWebView();
@@ -136,6 +157,31 @@ public function testRegisterStyleTag(): void
136157
$this->assertSame($expected, $html);
137158
}
138159

160+
public function testRegisterStyleTagEdgeCase(): void
161+
{
162+
$style = Html::style('H1 { color: red; }');
163+
$webView = TestHelper::createWebView()
164+
->registerStyleTag($style)
165+
->registerStyleTag($style, key: '')
166+
->registerStyleTag($style, key: '0')
167+
->registerStyleTag($style, key: 'test');
168+
169+
$html = $webView->render('positions.php');
170+
171+
$expected = <<<CODE
172+
[BEGINPAGE][/BEGINPAGE]
173+
[HEAD]{$style->render()}
174+
{$style->render()}
175+
{$style->render()}
176+
{$style->render()}[/HEAD]
177+
[BEGINBODY][/BEGINBODY]
178+
[ENDBODY][/ENDBODY]
179+
[ENDPAGE][/ENDPAGE]
180+
CODE;
181+
182+
$this->assertSame($expected, $html);
183+
}
184+
139185
public static function dataRegisterCssFile(): array
140186
{
141187
return [
@@ -159,6 +205,25 @@ public function testRegisterCssFile(string $url): void
159205
$this->assertStringContainsString('[HEAD]<link href="' . $url . '" rel="stylesheet">[/HEAD]', $html);
160206
}
161207

208+
public function testRegisterCssFileEdgeCase(): void
209+
{
210+
$webView = TestHelper::createWebView()
211+
->registerCssFile('main.css')
212+
->registerCssFile('main.css', options: ['data-x' => '1'], key: '')
213+
->registerCssFile('main.css', options: ['data-x' => '2'], key: '0');
214+
215+
$html = $webView->render('positions.php');
216+
217+
$this->assertStringContainsString(
218+
<<<HTML
219+
[HEAD]<link href="main.css" rel="stylesheet">
220+
<link href="main.css" rel="stylesheet" data-x="1">
221+
<link href="main.css" rel="stylesheet" data-x="2">[/HEAD]
222+
HTML,
223+
$html
224+
);
225+
}
226+
162227
public function testRegisterCssFileWithInvalidPosition(): void
163228
{
164229
$webView = TestHelper::createWebView();
@@ -329,6 +394,26 @@ public function testRegisterCss(string $expected, ?int $position): void
329394
$this->assertStringContainsString($expected, $html);
330395
}
331396

397+
public function testRegisterCssWithKeyEdgeCase(): void
398+
{
399+
$webView = TestHelper::createWebView()
400+
->registerCss('.red{color:red;}')
401+
->registerCss('.red{color:red;}', key: '')
402+
->registerCss('.red{color:red;}', key: '0')
403+
->registerCss('.red{color:red;}', key: 'red');
404+
405+
$html = $webView->render('positions.php');
406+
407+
$this->assertStringContainsString(
408+
<<<CSS
409+
.red{color:red;}
410+
.red{color:red;}
411+
.red{color:red;}
412+
CSS,
413+
$html
414+
);
415+
}
416+
332417
public function testRegisterCssWithAttributes(): void
333418
{
334419
$webView = TestHelper::createWebView();
@@ -447,6 +532,31 @@ public function testRegisterScriptTag(): void
447532
$this->assertSame($expected, $html);
448533
}
449534

535+
public function testRegisterScriptTagWithKeyEdgeCase(): void
536+
{
537+
$script = Html::script('alert(1);');
538+
$webView = TestHelper::createWebView()
539+
->registerScriptTag($script)
540+
->registerScriptTag($script, key: '')
541+
->registerScriptTag($script, key: '0')
542+
->registerScriptTag($script, key: 'four');
543+
544+
$html = $webView->render('positions.php');
545+
546+
$expected = <<<HTML
547+
[BEGINPAGE][/BEGINPAGE]
548+
[HEAD][/HEAD]
549+
[BEGINBODY][/BEGINBODY]
550+
[ENDBODY]{$script->render()}
551+
{$script->render()}
552+
{$script->render()}
553+
{$script->render()}[/ENDBODY]
554+
[ENDPAGE][/ENDPAGE]
555+
HTML;
556+
557+
$this->assertSame($expected, $html);
558+
}
559+
450560
public function testRegisterJsAndRegisterScriptTag(): void
451561
{
452562
$webView = TestHelper::createWebView();
@@ -525,6 +635,27 @@ public function testRegisterJsAndRegisterScriptTagWithAjax(): void
525635
$this->assertSame($expected, $html);
526636
}
527637

638+
public function testRegisterJsWithKeyEdgeCase(): void
639+
{
640+
$webView = TestHelper::createWebView()
641+
->registerJs('alert(1);')
642+
->registerJs('alert(1);', key: '')
643+
->registerJs('alert(1);', key: '0')
644+
->registerJs('alert(1);', key: 'four');
645+
646+
$html = $webView->render('positions.php');
647+
648+
$this->assertStringContainsString(
649+
<<<JS
650+
<script>alert(1);
651+
alert(1);
652+
alert(1);
653+
alert(1);</script>
654+
JS,
655+
$html
656+
);
657+
}
658+
528659
public function testAddCssFiles(): void
529660
{
530661
$webView = TestHelper::createWebView();

0 commit comments

Comments
 (0)