refactor!: Throw exception for unkown field method

Author: distantnative

src/Cms/Collection.php

@@ -7,6 +7,7 @@
 use Kirby\Toolkit\Collection as BaseCollection;
 use Kirby\Toolkit\Str;
 use Kirby\Uuid\Uuid;
+use Throwable;
 
 /**
  * The Collection class serves as foundation
@@ -126,7 +127,13 @@ public function append(...$args): static
 				is_object($args[0]) === true &&
 				is_callable([$args[0], 'id']) === true
 			) {
-				return parent::append($args[0]->id(), $args[0]);
+				try {
+					return parent::append($args[0]->id(), $args[0]);
+				} catch (Throwable) {
+					// is_callable might be true when the object implements
+					// the magic __call method, but __call can still throw
+					// an exception
+				}
 			}
 
 			return parent::append($args[0]);

src/Content/Field.php

@@ -20,6 +20,7 @@
 use Kirby\Cms\User;
 use Kirby\Cms\Users;
 use Kirby\Data\Data;
+use Kirby\Exception\BadMethodCallException;
 use Kirby\Exception\InvalidArgumentException;
 use Kirby\Exception\NotFoundException;
 use Kirby\Image\QrCode;
@@ -76,13 +77,14 @@ public function __call(string $method, array $arguments = []): mixed
 	{
 		$method = strtolower($method);
 
+		// call custom field method
 		if ($this->hasMethod($method) === true) {
 			return $this->callMethod($method, [clone $this, ...$arguments]);
 		}
 
-		// TODO: throw deprecation, then exception
-		// when unknown method is called
-		return $this;
+		throw new BadMethodCallException(
+			message: 'Field method "' . $method . '" does not exist'
+		);
 	}
 
 	/**

src/Panel/Ui/FilePreviews/ImageFilePreview.php

@@ -3,6 +3,7 @@
 namespace Kirby\Panel\Ui\FilePreviews;
 
 use Kirby\Cms\File;
+use Kirby\Image\Dimensions;
 use Kirby\Panel\Ui\FilePreview;
 use Kirby\Toolkit\I18n;
 
@@ -30,17 +31,22 @@ public static function accepts(File $file): bool
 
 	public function details(): array
 	{
-		return [
-			...parent::details(),
-			[
+		$details    = parent::details();
+		$dimensions = $this->file->dimensions();
+
+		if ($dimensions instanceof Dimensions) {
+			$details[] = [
 				'title' => I18n::translate('dimensions'),
-				'text'  => $this->file->dimensions() . ' ' . I18n::translate('pixel')
-			],
-			[
+				'text'  => $dimensions . ' ' . I18n::translate('pixel')
+			];
+
+			$details[] = [
 				'title' => I18n::translate('orientation'),
-				'text'  => I18n::translate('orientation.' . $this->file->dimensions()->orientation())
-			]
-		];
+				'text'  => I18n::translate('orientation.' . $dimensions->orientation())
+			];
+		}
+
+		return $details;
 	}
 
 	public function props(): array

tests/Cms/Collections/CollectionTest.php

@@ -40,6 +40,14 @@ public function uuid(): string|Field
 	}
 }
 
+class MockObjectWith__CallException
+{
+	public function __call($name, $arguments)
+	{
+		throw new Exception('Test exception');
+	}
+}
+
 #[CoversClass(Collection::class)]
 class CollectionTest extends TestCase
 {
@@ -145,6 +153,14 @@ public function testAppend()
 		$this->assertSame([$a, $b, $c, $d, 'a simple string'], $collection->values());
 	}
 
+	public function testAppendWith__CallException(): void
+	{
+		$collection = new Collection();
+		$obj        = new MockObjectWith__CallException();
+		$collection = $collection->append($obj);
+		$this->assertSame([0], $collection->keys());
+	}
+
 	public function testFindByUuid()
 	{
 		$collection = new Collection([

tests/Content/FieldTest.php

@@ -14,6 +14,7 @@
 use Kirby\Data\Data;
 use Kirby\Data\Json;
 use Kirby\Data\Yaml;
+use Kirby\Exception\BadMethodCallException;
 use Kirby\Exception\InvalidArgumentException;
 use Kirby\Filesystem\Dir;
 use Kirby\Image\QrCode;
@@ -68,10 +69,11 @@ public function test__call(): void
 
 	public function test__callNonExistingMethod(): void
 	{
-		$field  = $this->field('value');
-		$result = $field->methodDoesNotExist();
+		$this->expectException(BadMethodCallException::class);
+		$this->expectExceptionMessage('Field method "methoddoesnotexist" does not exist');
 
-		$this->assertSame($field, $result);
+		$field  = $this->field('value');
+		$field->methodDoesNotExist();
 	}
 
 	public function test__debugInfo(): void

tests/Panel/Ui/FilePreviews/ImageFilePreviewTest.php

@@ -6,16 +6,12 @@
 use Kirby\Cms\Page;
 use Kirby\Panel\Ui\FilePreview;
 use Kirby\TestCase;
+use PHPUnit\Framework\Attributes\CoversClass;
 
-/**
- * @coversDefaultClass \Kirby\Panel\Ui\FilePreviews\ImageFilePreview
- */
+#[CoversClass(ImageFilePreview::class)]
 class ImageFilePreviewTest extends TestCase
 {
-	/**
-	 * @covers ::accepts
-	 */
-	public function testAccepts()
+	public function testAccepts(): void
 	{
 		$page = new Page(['slug' => 'test']);
 
@@ -26,10 +22,7 @@ public function testAccepts()
 		$this->assertFalse(ImageFilePreview::accepts($file));
 	}
 
-	/**
-	 * @covers ::details
-	 */
-	public function testDetails()
+	public function testDetails(): void
 	{
 		$page    = new Page(['slug' => 'test']);
 		$file    = new File(['filename' => 'test.jpg', 'parent' => $page]);
@@ -43,10 +36,7 @@ public function testDetails()
 		$this->assertSame('Dimensions', $detail['title']);
 	}
 
-	/**
-	 * @covers ::__construct
-	 */
-	public function testFactory()
+	public function testFactory(): void
 	{
 		$page    = new Page(['slug' => 'test']);
 		$file    = new File(['filename' => 'test.jpg', 'parent' => $page]);
@@ -56,10 +46,7 @@ public function testFactory()
 		$this->assertSame('k-image-file-preview', $preview->component);
 	}
 
-	/**
-	 * @covers ::props
-	 */
-	public function testProps()
+	public function testProps(): void
 	{
 		$page    = new Page(['slug' => 'test']);
 		$file    = new File(['filename' => 'test.xls', 'parent' => $page]);

tests/Toolkit/VTest.php

@@ -600,18 +600,18 @@ public function testSize()
 		$this->assertFalse(V::size(7, 5, '<'));
 
 		$field = new Field(null, 'foo', 2);
-		$this->assertTrue(V::size($field->foo(), 2));
-		$this->assertTrue(V::size($field->foo(), 3, '<'));
-		$this->assertTrue(V::size($field->foo(), 1, '>'));
-		$this->assertFalse(V::size($field->foo(), 1, '<'));
-		$this->assertFalse(V::size($field->foo(), 3, '>'));
+		$this->assertTrue(V::size($field, 2));
+		$this->assertTrue(V::size($field, 3, '<'));
+		$this->assertTrue(V::size($field, 1, '>'));
+		$this->assertFalse(V::size($field, 1, '<'));
+		$this->assertFalse(V::size($field, 3, '>'));
 
 		$field = new Field(null, 'foo', 'hello');
-		$this->assertTrue(V::size($field->foo(), 5));
-		$this->assertTrue(V::size($field->foo(), 6, '<'));
-		$this->assertTrue(V::size($field->foo(), 4, '>'));
-		$this->assertFalse(V::size($field->foo(), 4, '<'));
-		$this->assertFalse(V::size($field->foo(), 6, '>'));
+		$this->assertTrue(V::size($field, 5));
+		$this->assertTrue(V::size($field, 6, '<'));
+		$this->assertTrue(V::size($field, 4, '>'));
+		$this->assertFalse(V::size($field, 4, '<'));
+		$this->assertFalse(V::size($field, 6, '>'));
 	}
 
 	public function testSizeInvalid()