Fix empty string and "0" keys usage for CSS/JS in WebView + Improve psalm

CHANGELOG.md

@@ -6,6 +6,10 @@
 - Chg #271: Remove deprecated methods `withDefaultExtension()` and `getDefaultExtension()` from `ViewInterface` (@vjik)
 - Chg #271: Rename configuration parameter `defaultExtension` to `fallbackExtension` (@vjik)
 - Chg #272: Add variadic parameter `$default` to `ViewInterface::getParameter()` (@vjik)
+- Bug #273: Fix empty string and "0" keys in `WebView` methods: `registerCss()`, `registerStyleTag()`,
+  `registerCssFile()`, `registerJs()`, `registerScriptTag()` and `registerJsFile()` (@vjik)
+- Enh #273: Use more specific psalm types in results of `WebView` methods: `getLinkTags()`, `getCss()`, `getCssFiles()`,
+  `getJs()` and `getJsFiles()` (@vjik) 
 
 ## 10.0.0 June 28, 2024
 

psalm.xml

@@ -15,6 +15,5 @@
     </projectFiles>
     <issueHandlers>
         <MixedAssignment errorLevel="suppress" />
-        <RiskyTruthyFalsyComparison errorLevel="suppress" />
     </issueHandlers>
 </psalm>

psalm83.xml

@@ -15,7 +15,6 @@
     </projectFiles>
     <issueHandlers>
         <MixedAssignment errorLevel="suppress" />
-        <RiskyTruthyFalsyComparison errorLevel="suppress" />
         <MissingClassConstType errorLevel="suppress" />
     </issueHandlers>
 </psalm>

src/State/WebViewState.php

@@ -41,7 +41,7 @@
 
     /**
      * @var array The registered link tags.
-     * @psalm-var array<int, Link[]>
+     * @psalm-var array<int, non-empty-array<array-key, Link>>
      *
      * @see registerLink()
      * @see registerLinkTag()
@@ -50,31 +50,31 @@
 
     /**
      * @var array The registered CSS code blocks.
-     * @psalm-var array<int, string[]|Style[]>
+     * @psalm-var array<int, non-empty-array<array-key, string|Style>>
      *
      * {@see registerCss()}
      */
     private array $css = [];
 
     /**
      * @var array The registered CSS files.
-     * @psalm-var array<int, string[]>
+     * @psalm-var array<int, non-empty-array<array-key, string>>
      *
      * {@see registerCssFile()}
      */
     private array $cssFiles = [];
 
     /**
      * @var array The registered JS code blocks
-     * @psalm-var array<int, string[]|Script[]>
+     * @psalm-var array<int, non-empty-array<array-key, string|Script>>
      *
      * {@see registerJs()}
      */
     private array $js = [];
 
     /**
      * @var array The registered JS files.
-     * @psalm-var array<int, string[]>
+     * @psalm-var array<int, non-empty-array<array-key, string>>
      *
      * {@see registerJsFile()}
      */
@@ -98,7 +98,7 @@
 
     /**
      * @return array The registered link tags.
-     * @psalm-return array<int, Link[]>
+     * @psalm-return array<int, non-empty-array<array-key, Link>>
      */
     public function getLinkTags(): array
     {
@@ -107,7 +107,7 @@
 
     /**
      * @return array The registered CSS code blocks.
-     * @psalm-return array<int, string[]|Style[]>
+     * @psalm-return array<int, non-empty-array<array-key, string|Style>>
      */
     public function getCss(): array
     {
@@ -116,7 +116,7 @@
 
     /**
      * @return array The registered CSS files.
-     * @psalm-return array<int, string[]>
+     * @psalm-return array<int, non-empty-array<array-key, string>>
      */
     public function getCssFiles(): array
     {
@@ -125,7 +125,7 @@
 
     /**
      * @return array The registered JS code blocks
-     * @psalm-return array<int, string[]|Script[]>
+     * @psalm-return array<int, non-empty-array<array-key, string|Script>>
      */
     public function getJs(): array
     {
@@ -134,7 +134,7 @@
 
     /**
      * @return array The registered JS files.
-     * @psalm-return array<int, string[]>
+     * @psalm-return array<int, non-empty-array<array-key, string>>
      */
     public function getJsFiles(): array
     {
@@ -224,18 +224,17 @@
      * Registers a CSS code block.
      *
      * @param string $css The content of the CSS code block to be registered.
-     * @param string|null $key The key that identifies the CSS code block. If null, it will use $css as the key.
-     * If two CSS code blocks are registered with the same key, the latter will overwrite the former.
      * @param array $attributes The HTML attributes for the {@see Style} tag.
+     * @param string|null $key The key that identifies the CSS code block. If `null`, it will use `$css` as the key.
+     * If two CSS code blocks are registered with the same key, the latter will overwrite the former.
      */
     public function registerCss(
         string $css,
         int $position = WebView::POSITION_HEAD,
         array $attributes = [],
         ?string $key = null
     ): void {
-        $key = $key ?: md5($css);
-        $this->css[$position][$key] = $attributes === [] ? $css : Html::style($css, $attributes);
+        $this->css[$position][$key ?? md5($css)] = $attributes === [] ? $css : Html::style($css, $attributes);
     }
 
     /**
@@ -266,8 +265,7 @@
      */
     public function registerStyleTag(Style $style, int $position = WebView::POSITION_HEAD, ?string $key = null): void
     {
-        $key = $key ?: md5($style->render());
-        $this->css[$position][$key] = $style;
+        $this->css[$position][$key ?? md5($style->render())] = $style;
     }
 
     /**
@@ -280,20 +278,20 @@
      * @param string $url The CSS file to be registered.
      * @param array $options the HTML attributes for the link tag. Please refer to {@see \Yiisoft\Html\Html::cssFile()}
      * for the supported options.
-     * @param string|null $key The key that identifies the CSS script file. If null, it will use $url as the key.
+     * @param string|null $key The key that identifies the CSS script file. If `null`, it will use `$url` as the key.
      * If two CSS files are registered with the same key, the latter will overwrite the former.
      */
     public function registerCssFile(
         string $url,
         int $position = WebView::POSITION_HEAD,
         array $options = [],
-        string $key = null
+        ?string $key = null
     ): void {
         if (!$this->isValidCssPosition($position)) {
             throw new InvalidArgumentException('Invalid position of CSS file.');
         }
 
-        $this->cssFiles[$position][$key ?: $url] = Html::cssFile($url, $options)->render();
+        $this->cssFiles[$position][$key ?? $url] = Html::cssFile($url, $options)->render();
     }
 
     /**
@@ -337,13 +335,12 @@
      * - {@see WebView::POSITION_END}: at the end of the body section. This is the default value.
      * - {@see WebView::POSITION_LOAD}: executed when HTML page is completely loaded.
      * - {@see WebView::POSITION_READY}: executed when HTML document composition is ready.
-     * @param string|null $key The key that identifies the JS code block. If null, it will use $js as the key.
+     * @param string|null $key The key that identifies the JS code block. If `null`, it will use `$js` as the key.
      * If two JS code blocks are registered with the same key, the latter will overwrite the former.
      */
     public function registerJs(string $js, int $position = WebView::POSITION_END, ?string $key = null): void
     {
-        $key = $key ?: md5($js);
-        $this->js[$position][$key] = $js;
+        $this->js[$position][$key ?? md5($js)] = $js;
     }
 
     /**
@@ -353,8 +350,7 @@
      */
     public function registerScriptTag(Script $script, int $position = WebView::POSITION_END, ?string $key = null): void
     {
-        $key = $key ?: md5($script->render());
-        $this->js[$position][$key] = $script;
+        $this->js[$position][$key ?? md5($script->render())] = $script;
     }
 
     /**
@@ -389,7 +385,7 @@
             throw new InvalidArgumentException('Invalid position of JS file.');
         }
 
-        $this->jsFiles[$position][$key ?: $url] = Html::javaScriptFile($url, $options)->render();
+        $this->jsFiles[$position][$key ?? $url] = Html::javaScriptFile($url, $options)->render();
     }
 
     /**
@@ -634,7 +630,7 @@
             );
         }
 
         if (!array_key_exists(1, $config)) {
             throw new InvalidArgumentException('Do not set JS variable value.');
         }
         /** @var mixed */

tests/WebViewTest.php

@@ -118,6 +118,27 @@ public function testRegisterJsFileWithPosition(string $expected, int $position):
         $this->assertStringContainsString($expected, $html);
     }
 
+    public function testRegisterJsFileWithKeyEdgeCase(): void
+    {
+        $webView = TestHelper::createWebView()
+            ->registerJsFile('main.js')
+            ->registerJsFile('main.js', key: '')
+            ->registerJsFile('main.js', key: '0')
+            ->registerJsFile('main.js', key: 'four');
+
+        $html = $webView->render('positions.php');
+
+        $this->assertStringContainsString(
+            <<<HTML
+            [ENDBODY]<script src="main.js"></script>
+            <script src="main.js"></script>
+            <script src="main.js"></script>
+            <script src="main.js"></script>[/ENDBODY]
+            HTML,
+            $html
+        );
+    }
+
     public function testRegisterStyleTag(): void
     {
         $webView = TestHelper::createWebView();
@@ -136,6 +157,31 @@ public function testRegisterStyleTag(): void
         $this->assertSame($expected, $html);
     }
 
+    public function testRegisterStyleTagEdgeCase(): void
+    {
+        $style = Html::style('H1 { color: red; }');
+        $webView = TestHelper::createWebView()
+            ->registerStyleTag($style)
+            ->registerStyleTag($style, key: '')
+            ->registerStyleTag($style, key: '0')
+            ->registerStyleTag($style, key: 'test');
+
+        $html = $webView->render('positions.php');
+
+        $expected = <<<CODE
+            [BEGINPAGE][/BEGINPAGE]
+            [HEAD]{$style->render()}
+            {$style->render()}
+            {$style->render()}
+            {$style->render()}[/HEAD]
+            [BEGINBODY][/BEGINBODY]
+            [ENDBODY][/ENDBODY]
+            [ENDPAGE][/ENDPAGE]
+            CODE;
+
+        $this->assertSame($expected, $html);
+    }
+
     public static function dataRegisterCssFile(): array
     {
         return [
@@ -159,6 +205,25 @@ public function testRegisterCssFile(string $url): void
         $this->assertStringContainsString('[HEAD]<link href="' . $url . '" rel="stylesheet">[/HEAD]', $html);
     }
 
+    public function testRegisterCssFileEdgeCase(): void
+    {
+        $webView = TestHelper::createWebView()
+            ->registerCssFile('main.css')
+            ->registerCssFile('main.css', options: ['data-x' => '1'], key: '')
+            ->registerCssFile('main.css', options: ['data-x' => '2'], key: '0');
+
+        $html = $webView->render('positions.php');
+
+        $this->assertStringContainsString(
+            <<<HTML
+            [HEAD]<link href="main.css" rel="stylesheet">
+            <link href="main.css" rel="stylesheet" data-x="1">
+            <link href="main.css" rel="stylesheet" data-x="2">[/HEAD]
+            HTML,
+            $html
+        );
+    }
+
     public function testRegisterCssFileWithInvalidPosition(): void
     {
         $webView = TestHelper::createWebView();
@@ -329,6 +394,26 @@ public function testRegisterCss(string $expected, ?int $position): void
         $this->assertStringContainsString($expected, $html);
     }
 
+    public function testRegisterCssWithKeyEdgeCase(): void
+    {
+        $webView = TestHelper::createWebView()
+            ->registerCss('.red{color:red;}')
+            ->registerCss('.red{color:red;}', key: '')
+            ->registerCss('.red{color:red;}', key: '0')
+            ->registerCss('.red{color:red;}', key: 'red');
+
+        $html = $webView->render('positions.php');
+
+        $this->assertStringContainsString(
+            <<<CSS
+            .red{color:red;}
+            .red{color:red;}
+            .red{color:red;}
+            CSS,
+            $html
+        );
+    }
+
     public function testRegisterCssWithAttributes(): void
     {
         $webView = TestHelper::createWebView();
@@ -447,6 +532,31 @@ public function testRegisterScriptTag(): void
         $this->assertSame($expected, $html);
     }
 
+    public function testRegisterScriptTagWithKeyEdgeCase(): void
+    {
+        $script = Html::script('alert(1);');
+        $webView = TestHelper::createWebView()
+            ->registerScriptTag($script)
+            ->registerScriptTag($script, key: '')
+            ->registerScriptTag($script, key: '0')
+            ->registerScriptTag($script, key: 'four');
+
+        $html = $webView->render('positions.php');
+
+        $expected = <<<HTML
+            [BEGINPAGE][/BEGINPAGE]
+            [HEAD][/HEAD]
+            [BEGINBODY][/BEGINBODY]
+            [ENDBODY]{$script->render()}
+            {$script->render()}
+            {$script->render()}
+            {$script->render()}[/ENDBODY]
+            [ENDPAGE][/ENDPAGE]
+            HTML;
+
+        $this->assertSame($expected, $html);
+    }
+
     public function testRegisterJsAndRegisterScriptTag(): void
     {
         $webView = TestHelper::createWebView();
@@ -525,6 +635,27 @@ public function testRegisterJsAndRegisterScriptTagWithAjax(): void
         $this->assertSame($expected, $html);
     }
 
+    public function testRegisterJsWithKeyEdgeCase(): void
+    {
+        $webView = TestHelper::createWebView()
+            ->registerJs('alert(1);')
+            ->registerJs('alert(1);', key: '')
+            ->registerJs('alert(1);', key: '0')
+            ->registerJs('alert(1);', key: 'four');
+
+        $html = $webView->render('positions.php');
+
+        $this->assertStringContainsString(
+            <<<JS
+            <script>alert(1);
+            alert(1);
+            alert(1);
+            alert(1);</script>
+            JS,
+            $html
+        );
+    }
+
     public function testAddCssFiles(): void
     {
         $webView = TestHelper::createWebView();