Merge branch '5' into 6

src/ORM/ArrayList.php

@@ -137,6 +137,8 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
 
     /**
      * Return an array of the actual items that this ArrayList contains.
+     *
+     * @return array<T>
      */
     public function toArray()
     {
@@ -477,6 +479,8 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
      * @example $list->sort('Name DESC'); // DESC sorting
      * @example $list->sort('Name', 'ASC');
      * @example $list->sort(array('Name'=>'ASC,'Age'=>'DESC'));
+     *
+     * @return static<T>
      */
     public function sort()
     {
@@ -589,6 +593,7 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
      *
      * @param string $key
      * @param mixed $value
+     * @return T|null
      */
     public function find($key, $value)
     {
@@ -609,6 +614,8 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
      * Also supports SearchFilter syntax
      * @example // include anyone with "sam" anywhere in their name
      *          $list = $list->filter('Name:PartialMatch', 'sam');
+     *
+     * @return static<T>
      */
     public function filter()
     {
@@ -635,6 +642,7 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
      *          $list = $list->filterAny('Name:PartialMatch', 'sam');
      *
      * @param string|array See {@link filter()}
+     * @return static<T>
      */
     public function filterAny()
     {
@@ -656,6 +664,8 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
      * Also supports SearchFilter syntax
      * @example // everyone except anyone with "sam" anywhere in their name
      *          $list = $list->exclude('Name:PartialMatch', 'sam');
+     *
+     * @return static<T>
      */
     public function exclude()
     {
@@ -829,6 +839,8 @@ class ArrayList extends ViewableData implements SS_List, Filterable, Sortable, L
      * Filter this list to only contain the given Primary IDs
      *
      * @param array $ids Array of integers, will be automatically cast/escaped.
+     *
+     * @return static<T>
      */
     public function byIDs($ids)
     {

src/ORM/DataList.php

@@ -322,6 +322,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
     /**
      * Return a new DataList instance with the records returned in this query
      * restricted by a limit clause.
+     * @return static<T>
      */
     public function limit(?int $length, int $offset = 0): static
     {
@@ -364,6 +365,8 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * @example $list = $list->sort(['Name' => 'ASC', 'Age' => 'DESC']);
      * @example $list = $list->sort('MyRelation.MyColumn ASC')
      * @example $list->sort(null); // wipe any existing sort
+     *
+     * @return static<T>
      */
     public function sort(...$args): static
     {
@@ -480,6 +483,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * ->filter('Field:not', null) will generate '"Field" IS NOT NULL'
      *
      * @param string|array Escaped SQL statement. If passed as array, all keys and values will be escaped internally
+     * @return static<T>
      */
     public function filter()
     {
@@ -541,6 +545,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      *          // SQL: WHERE (("Name" IN ('bob', 'phil')) OR ("Age" IN ('21', '43'))
      *
      * @param string|array See {@link filter()}
+     * @return static<T>
      */
     public function filterAny()
     {
@@ -681,6 +686,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      *
      * @param string|array
      * @param string [optional]
+     * @return static<T>
      */
     public function exclude()
     {
@@ -824,6 +830,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
     /**
      * Return an array of the actual items that this DataList contains at this stage.
      * This is when the query is actually executed.
+     * @return array<T>
      */
     public function toArray()
     {
@@ -1658,6 +1665,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * Returns the first item in this DataList
      *
      * The object returned is not cached, unlike {@link DataObject::get_one()}
+     * @return T|null
      */
     public function first()
     {
@@ -1676,6 +1684,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * Returns the last item in this DataList
      *
      * The object returned is not cached, unlike {@link DataObject::get_one()}
+     * @return T|null
      */
     public function last()
     {
@@ -1708,6 +1717,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      *
      * @param string $key
      * @param string $value
+     * @return T|null
      */
     public function find($key, $value)
     {
@@ -1736,6 +1746,7 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
      * Return the first DataObject with the given ID
      *
      * The object returned is not cached, unlike {@link DataObject::get_by_id()}
+     * @return T|null
      */
     public function byID($id)
     {

src/View/AttributesHTML.php

@@ -105,9 +105,9 @@ trait AttributesHTML
 
         $attributes = (array) $attributes;
 
-        $attributes = array_filter($attributes ?? [], function ($v) {
+        $attributes = array_filter($attributes ?? [], function ($v, $k) {
-            return ($v || $v === 0 || $v === '0');
+            return ($k === 'alt' || $v || $v === 0 || $v === '0');
-        });
+        }, ARRAY_FILTER_USE_BOTH);
 
         if ($exclude) {
             $attributes = array_diff_key(

tests/php/View/AttributesHTMLTest.php

@@ -0,0 +1,290 @@
+<?php
+
+namespace SilverStripe\View\Tests;
+
+use SilverStripe\Dev\SapphireTest;
+use SilverStripe\View\Tests\AttributesHTMLTest\DummyAttributesHTML;
+
+class AttributesHTMLTest extends SapphireTest
+{
+
+    public function provideGetAttribute(): array
+    {
+        return [
+            'empty string' => ['test', '', 'Empty string is not converted to a different falsy value'],
+            'Zero' => ['test', 0, 'Zero is not converted to a different falsy value'],
+            'Null' => ['test', 0, 'Null is not converted to a different falsy value'],
+            'False' => ['test', false, 'False is not converted to a different falsy value'],
+            'Empty array' => ['test', [], 'Empty array is not converted to a different falsy value'],
+            'True' => ['test', true, 'True is stored properly as an attribute'],
+            'String' => ['test', 'test', 'String is stored properly as an attribute'],
+            'Int' => ['test', -1, 'Int is stored properly as an attribute'],
+            'Array' => ['test', ['foo' => 'bar'], 'Array is stored properly as an attribute'],
+        ];
+    }
+
+    /** @dataProvider provideGetAttribute */
+    public function testGetAttribute($name, $value, $message): void
+    {
+        $dummy = new DummyAttributesHTML();
+        $this->assertNull(
+            $dummy->getAttribute('non-existent attribute'),
+            'Trying to access a non-existent attribute should return null'
+        );
+
+        $dummy->setAttribute($name, $value);
+        $this->assertSame(
+            $value,
+            $dummy->getAttribute($name),
+            $message
+        );
+    }
+
+    public function testGetAttributes(): void
+    {
+        $dummy = new DummyAttributesHTML();
+        $dummy->setDefaultAttributes([]);
+        $this->assertSame(
+            [],
+            $dummy->getAttributes(),
+            'When no attributes are set and the default attributes are empty, an empty array should be returned'
+        );
+
+        $dummy->setAttribute('empty', '');
+        $dummy->setAttribute('foo', 'bar');
+        $dummy->setAttribute('Number', 123);
+        $dummy->setAttribute('Array', ['foo' => 'bar']);
+
+        $this->assertSame(
+            [
+                'empty' => '',
+                'foo' => 'bar',
+                'Number' => 123,
+                'Array' => ['foo' => 'bar'],
+            ],
+            $dummy->getAttributes(),
+            'All explicitly defined attributes should be returned'
+        );
+
+        $dummy = new DummyAttributesHTML();
+        $dummy->setDefaultAttributes([
+            'foo' => 'Will be overridden',
+            'bar' => 'Not overridden',
+        ]);
+        $this->assertSame(
+            [
+                'foo' => 'Will be overridden',
+                'bar' => 'Not overridden',
+            ],
+            $dummy->getAttributes(),
+            'When no attributes are set and the default attributes are used'
+        );
+
+        $dummy->setAttribute('empty', '');
+        $dummy->setAttribute('foo', 'bar');
+        $dummy->setAttribute('Number', 123);
+        $dummy->setAttribute('Array', ['foo' => 'bar']);
+
+        $this->assertSame(
+            [
+                'foo' => 'bar',
+                'bar' => 'Not overridden',
+                'empty' => '',
+                'Number' => 123,
+                'Array' => ['foo' => 'bar'],
+            ],
+            $dummy->getAttributes(),
+            'Explicitly defined attributes overrides default ones'
+        );
+    }
+
+    public function testAttributesHTML(): void
+    {
+        $dummy = new DummyAttributesHTML();
+
+        $dummy->setAttribute('emptystring', '');
+        $dummy->setAttribute('nullvalue', null);
+        $dummy->setAttribute('false', false);
+        $dummy->setAttribute('emptyarray', []);
+        $dummy->setAttribute('zeroint', 0);
+        $dummy->setAttribute('zerostring', '0');
+        $dummy->setAttribute('alt', '');
+        $dummy->setAttribute('array', ['foo' => 'bar']);
+        $dummy->setAttribute('width', 123);
+        $dummy->setAttribute('hack>', '">hack');
+
+        $html = $dummy->getAttributesHTML();
+
+        $this->assertStringNotContainsString(
+            'emptystring',
+            $html,
+            'Attribute with empty string are not rendered'
+        );
+
+        $this->assertStringNotContainsString(
+            'nullvalue',
+            $html,
+            'Attribute with null are not rendered'
+        );
+
+        $this->assertStringNotContainsString(
+            'false',
+            $html,
+            'Attribute with false are not rendered'
+        );
+
+        $this->assertStringNotContainsString(
+            'emptyarray',
+            $html,
+            'Attribute with empty array are not rendered'
+        );
+
+        $this->assertStringContainsString(
+            'zeroint="0"',
+            $html,
+            'Attribute with a zero int value are rendered'
+        );
+
+        $this->assertStringContainsString(
+            'zerostring="0"',
+            $html,
+            'Attribute with a zerostring value are rendered'
+        );
+
+        $this->assertStringContainsString(
+            'alt=""',
+            $html,
+            'alt attribute is rendered even when empty set to an empty string'
+        );
+
+        $this->assertStringContainsString(
+            'array="{&quot;foo&quot;:&quot;bar&quot;}"',
+            $html,
+            'Array attribute is converted to JSON'
+        );
+
+        $this->assertStringContainsString(
+            'width="123"',
+            $html,
+            'Numeric values are rendered with quotes'
+        );
+
+        $this->assertStringNotContainsString(
+            'hack&quot;&gt;="&quot;&gt;hack"',
+            $html,
+            'Attribute names and value are escaped'
+        );
+
+        $html = $dummy->getAttributesHTML('zeroint', 'array');
+
+        $this->assertStringNotContainsString(
+            'zeroint="0"',
+            $html,
+            'Excluded attributes are not rendered'
+        );
+
+        $this->assertStringContainsString(
+            'zerostring="0"',
+            $html,
+            'Attribute not excluded still render'
+        );
+
+        $this->assertStringContainsString(
+            'alt=""',
+            $html,
+            'Attribute not excluded still render'
+        );
+
+        $this->assertStringNotContainsString(
+            'array',
+            $html,
+            'Excluded attributes are not rendered'
+        );
+    }
+
+    public function testAttributesHTMLwithExplicitAttr(): void
+    {
+        $dummy = new DummyAttributesHTML();
+
+        $this->assertEmpty(
+            '',
+            $dummy->getAttributesHTML(),
+            'If no attributes are provided, an empty string should be returned'
+        );
+
+        $attributes = [
+            'emptystring' => '',
+            'nullvalue' => null,
+            'false' => false,
+            'emptyarray' => [],
+            'zeroint' => 0,
+            'zerostring' => '0',
+            'alt' => '',
+            'array' => ['foo' => 'bar'],
+            'width' => 123,
+            'hack>' => '">hack',
+        ];
+
+        $html = $dummy->getAttributesHTML($attributes);
+
+        $this->assertStringNotContainsString(
+            'emptystring',
+            $html,
+            'Attribute with empty string are not rendered'
+        );
+
+        $this->assertStringNotContainsString(
+            'nullvalue',
+            $html,
+            'Attribute with null are not rendered'
+        );
+
+        $this->assertStringNotContainsString(
+            'false',
+            $html,
+            'Attribute with false are not rendered'
+        );
+
+        $this->assertStringNotContainsString(
+            'emptyarray',
+            $html,
+            'Attribute with empty array are not rendered'
+        );
+
+        $this->assertStringContainsString(
+            'zeroint="0"',
+            $html,
+            'Attribute with a zero int value are rendered'
+        );
+
+        $this->assertStringContainsString(
+            'zerostring="0"',
+            $html,
+            'Attribute with a zerostring value are rendered'
+        );
+
+        $this->assertStringContainsString(
+            'alt=""',
+            $html,
+            'alt attribute is rendered even when empty set to an empty string'
+        );
+
+        $this->assertStringContainsString(
+            'array="{&quot;foo&quot;:&quot;bar&quot;}"',
+            $html,
+            'Array attribute is converted to JSON'
+        );
+
+        $this->assertStringContainsString(
+            'width="123"',
+            $html,
+            'Numeric values are rendered with quotes'
+        );
+
+        $this->assertStringNotContainsString(
+            'hack&quot;&gt;="&quot;&gt;hack"',
+            $html,
+            'Attribute names and value are escaped'
+        );
+    }
+}

tests/php/View/AttributesHTMLTest/DummyAttributesHTML.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace SilverStripe\View\Tests\AttributesHTMLTest;
+
+use SilverStripe\View\AttributesHTML;
+use SilverStripe\Dev\TestOnly;
+
+/**
+ * This call is used to test the AttributesHTML trait
+ */
+class DummyAttributesHTML implements TestOnly
+{
+    use AttributesHTML;
+
+    private array $defaultAttributes = [];
+
+    /**
+     * Trait requires this method to prepopulate the attributes
+     */
+    protected function getDefaultAttributes(): array
+    {
+        return $this->defaultAttributes;
+    }
+
+    /**
+     * This method is only there to allow to explicitly set the default attributes in the test.
+     */
+    public function setDefaultAttributes(array $attributes): void
+    {
+        $this->defaultAttributes = $attributes;
+    }
+}