From aeead1a07dba3b9f21e103b215375952d5a64947 Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Mon, 29 Dec 2025 15:14:38 +0100 Subject: [PATCH 1/4] fixes for phpcs bonus levels --- examples/bootstrap_examples.php | 2 +- examples/collection.php | 2 +- phpcs.bonus.xml | 4 +++- phpstan.bonus.neon | 4 ++-- src/objects/LinkObject.php | 14 ++++++++------ tests/example_output/collection/collection.php | 2 +- 6 files changed, 16 insertions(+), 12 deletions(-) diff --git a/examples/bootstrap_examples.php b/examples/bootstrap_examples.php index acf810cc..92348d3b 100644 --- a/examples/bootstrap_examples.php +++ b/examples/bootstrap_examples.php @@ -59,7 +59,7 @@ class ExampleDataset { ]; public static function getRecord($type, $id) { - if (!isset(self::$records[$type][$id])) { + if (isset(self::$records[$type][$id]) === false) { throw new \Exception('sorry, we have a limited dataset'); } diff --git a/examples/collection.php b/examples/collection.php index cdd5a38c..ab11fc8a 100644 --- a/examples/collection.php +++ b/examples/collection.php @@ -18,7 +18,7 @@ foreach ($users as $user) { $resource = ResourceObject::fromObject($user, type: 'user', id: $user->id); - if ($user->id == 42) { + if ($user->id === 42) { $ship = new ResourceObject('ship', 5); $ship->add('name', 'Heart of Gold'); $resource->addRelationship('ship', $ship); diff --git a/phpcs.bonus.xml b/phpcs.bonus.xml index ee7fe9e0..5cb3deca 100644 --- a/phpcs.bonus.xml +++ b/phpcs.bonus.xml @@ -21,5 +21,7 @@ - + + + diff --git a/phpstan.bonus.neon b/phpstan.bonus.neon index ae9db4e9..169fed18 100644 --- a/phpstan.bonus.neon +++ b/phpstan.bonus.neon @@ -4,9 +4,9 @@ includes: parameters: level: 10 - + treatPhpDocTypesAsCertain: true - + # @see https://github.com/phpstan/phpstan-strict-rules strictRules: allRules: true diff --git a/src/objects/LinkObject.php b/src/objects/LinkObject.php index 5a13d2fb..7fbb4fdb 100644 --- a/src/objects/LinkObject.php +++ b/src/objects/LinkObject.php @@ -152,12 +152,7 @@ public function toArray(): array { $array['type'] = $this->type; } if ($this->hreflang !== []) { - if (count($this->hreflang) === 1) { - $array['hreflang'] = $this->hreflang[0]; - } - else { - $array['hreflang'] = $this->hreflang; - } + $array['hreflang'] = $this->getHrefLanguages(); } if (isset($this->describedby) && $this->describedby->isEmpty() === false) { $array['describedby'] = $this->describedby->toArray(); @@ -168,4 +163,11 @@ public function toArray(): array { return $array; } + + /** + * @return string|string[] + */ + private function getHrefLanguages(): string|array { + return (count($this->hreflang) === 1) ? $this->hreflang[0] : $this->hreflang; + } } diff --git a/tests/example_output/collection/collection.php b/tests/example_output/collection/collection.php index 991fbaee..a3a02659 100644 --- a/tests/example_output/collection/collection.php +++ b/tests/example_output/collection/collection.php @@ -29,7 +29,7 @@ public static function createJsonapiDocument() { foreach ($users as $user) { $resource = ResourceObject::fromObject($user, 'user', $user->id); - if ($user->id == 42) { + if ($user->id === 42) { $ship = new ResourceObject('ship', 5); $ship->add('name', 'Heart of Gold'); $resource->addRelationship('ship', $ship); From 7452fffadd739ba400ee532b53d199e14c92f754 Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Mon, 29 Dec 2025 15:14:52 +0100 Subject: [PATCH 2/4] fix phpstan performance issues --- .../profiles/CursorPaginationProfileTest.php | 39 ++++++++++++------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/tests/profiles/CursorPaginationProfileTest.php b/tests/profiles/CursorPaginationProfileTest.php index 42250a6d..9d67e331 100644 --- a/tests/profiles/CursorPaginationProfileTest.php +++ b/tests/profiles/CursorPaginationProfileTest.php @@ -76,21 +76,30 @@ public function test_WithRelationship(): void { parent::assertArrayHasKey('data', $array); parent::assertArrayHasKey('relationships', $array['data']); parent::assertArrayHasKey('people', $array['data']['relationships']); - parent::assertArrayHasKey('links', $array['data']['relationships']['people']); - parent::assertArrayHasKey('data', $array['data']['relationships']['people']); - parent::assertArrayHasKey('meta', $array['data']['relationships']['people']); - parent::assertArrayHasKey('prev', $array['data']['relationships']['people']['links']); - parent::assertArrayHasKey('next', $array['data']['relationships']['people']['links']); - parent::assertArrayHasKey('page', $array['data']['relationships']['people']['meta']); - parent::assertArrayHasKey('href', $array['data']['relationships']['people']['links']['prev']); - parent::assertArrayHasKey('href', $array['data']['relationships']['people']['links']['next']); - parent::assertArrayHasKey('total', $array['data']['relationships']['people']['meta']['page']); - parent::assertArrayHasKey('estimatedTotal', $array['data']['relationships']['people']['meta']['page']); - parent::assertArrayHasKey('bestGuess', $array['data']['relationships']['people']['meta']['page']['estimatedTotal']); - parent::assertCount(3, $array['data']['relationships']['people']['data']); - parent::assertArrayHasKey('meta', $array['data']['relationships']['people']['data'][0]); - parent::assertArrayHasKey('page', $array['data']['relationships']['people']['data'][0]['meta']); - parent::assertArrayHasKey('cursor', $array['data']['relationships']['people']['data'][0]['meta']['page']); + + // re-map nested arrays to variables to speed up phpstan + // without it, this file takes 10 seconds (!) more to process + + $people = $array['data']['relationships']['people']; + parent::assertArrayHasKey('links', $people); + parent::assertArrayHasKey('data', $people); + parent::assertArrayHasKey('meta', $people); + parent::assertArrayHasKey('prev', $people['links']); + parent::assertArrayHasKey('next', $people['links']); + parent::assertArrayHasKey('page', $people['meta']); + parent::assertArrayHasKey('href', $people['links']['prev']); + parent::assertArrayHasKey('href', $people['links']['next']); + + $peopleMeta = $people['meta']; + parent::assertArrayHasKey('total', $peopleMeta['page']); + parent::assertArrayHasKey('estimatedTotal', $peopleMeta['page']); + parent::assertArrayHasKey('bestGuess', $peopleMeta['page']['estimatedTotal']); + parent::assertCount(3, $people['data']); + + $firstPerson = $people['data'][0]; + parent::assertArrayHasKey('meta', $firstPerson); + parent::assertArrayHasKey('page', $firstPerson['meta']); + parent::assertArrayHasKey('cursor', $firstPerson['meta']['page']); } public function testSetLinksFirstPage_HappyPath(): void { From 0a998dc0b6ef10465104fd489b3f507569859c7a Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Mon, 29 Dec 2025 15:19:27 +0100 Subject: [PATCH 3/4] cleanup relaxed type checking by phpdoc --- examples/bootstrap_examples.php | 13 +++++-------- phpstan.bonus.neon | 2 -- phpstan.neon | 4 ++-- src/helpers/RequestParser.php | 2 +- src/objects/RelationshipObject.php | 1 + tests/example_output/ExampleTimestampsProfile.php | 13 +++++-------- 6 files changed, 14 insertions(+), 21 deletions(-) diff --git a/examples/bootstrap_examples.php b/examples/bootstrap_examples.php index 92348d3b..c18cf55c 100644 --- a/examples/bootstrap_examples.php +++ b/examples/bootstrap_examples.php @@ -151,14 +151,11 @@ public function getOfficialLink(): string { * optionally helpers for the specific profile */ - /** - * @param ResourceInterface&HasAttributesInterface $resource - */ - public function setTimestamps(ResourceInterface $resource, ?\DateTimeInterface $created=null, ?\DateTimeInterface $updated=null) { - if ($resource instanceof HasAttributesInterface === false) { - throw new \Exception('cannot add attributes to identifier objects'); - } - + public function setTimestamps( + ResourceInterface & HasAttributesInterface $resource, + ?\DateTimeInterface $created=null, + ?\DateTimeInterface $updated=null, + ) { $timestamps = []; if ($created !== null) { $timestamps['created'] = $created->format(\DateTime::ISO8601); diff --git a/phpstan.bonus.neon b/phpstan.bonus.neon index 169fed18..ddc4b487 100644 --- a/phpstan.bonus.neon +++ b/phpstan.bonus.neon @@ -5,8 +5,6 @@ includes: parameters: level: 10 - treatPhpDocTypesAsCertain: true - # @see https://github.com/phpstan/phpstan-strict-rules strictRules: allRules: true diff --git a/phpstan.neon b/phpstan.neon index a5a5f410..d0cf43ae 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -5,11 +5,11 @@ parameters: - src/ - tests/ - examples/ - + typeAliases: PHPStanTypeAlias_InternalOptions: 'array' - treatPhpDocTypesAsCertain: false + treatPhpDocTypesAsCertain: true strictRules: allRules: false diff --git a/src/helpers/RequestParser.php b/src/helpers/RequestParser.php index 0863c306..24918ca8 100644 --- a/src/helpers/RequestParser.php +++ b/src/helpers/RequestParser.php @@ -112,7 +112,7 @@ public function hasIncludePaths(): bool { * the raw format allows for custom processing * * @param PHPStanTypeAlias_InternalOptions $options {@see RequestParser::$defaults} - * @return string[]|array + * @return array|array */ public function getIncludePaths(array $options=[]): array { if ($this->queryParameters['include'] === '') { diff --git a/src/objects/RelationshipObject.php b/src/objects/RelationshipObject.php index e44ece9a..51668997 100644 --- a/src/objects/RelationshipObject.php +++ b/src/objects/RelationshipObject.php @@ -316,6 +316,7 @@ public function getNestedContainedResourceObjects(): array { $resourceObjects = []; foreach ($resources as $resource) { + // @phpstan-ignore instanceof.alwaysTrue, identical.alwaysFalse (we _can_ have both ResourceObject and ResourceIdentifierObject here) if ($resource->getResource() instanceof ResourceObject === false) { continue; } diff --git a/tests/example_output/ExampleTimestampsProfile.php b/tests/example_output/ExampleTimestampsProfile.php index 210af099..4129a35f 100644 --- a/tests/example_output/ExampleTimestampsProfile.php +++ b/tests/example_output/ExampleTimestampsProfile.php @@ -14,14 +14,11 @@ public function getOfficialLink(): string { return 'https://jsonapi.org/recommendations/#authoring-profiles'; } - /** - * @param ResourceInterface&HasAttributesInterface $resource - */ - public function setTimestamps(ResourceInterface $resource, ?\DateTimeInterface $created=null, ?\DateTimeInterface $updated=null) { - if ($resource instanceof HasAttributesInterface === false) { - throw new InputException('cannot add attributes to identifier objects'); - } - + public function setTimestamps( + ResourceInterface & HasAttributesInterface $resource, + ?\DateTimeInterface $created=null, + ?\DateTimeInterface $updated=null, + ) { $timestamps = []; if ($created !== null) { $timestamps['created'] = $created->format(\DateTime::ISO8601); From 7d4b8515a566df31bfca36cddc5a3ab33c200707 Mon Sep 17 00:00:00 2001 From: Lode Claassen Date: Mon, 29 Dec 2025 15:53:12 +0100 Subject: [PATCH 4/4] improve test coverage --- src/Document.php | 4 -- src/objects/RelationshipObject.php | 5 -- src/objects/ResourceIdentifierObject.php | 2 +- tests/ResourceDocumentTest.php | 61 +++++++++++++++++++ tests/helpers/RequestParserTest.php | 7 +-- .../objects/ResourceIdentifierObjectTest.php | 48 +++++++++++++++ 6 files changed, 112 insertions(+), 15 deletions(-) diff --git a/src/Document.php b/src/Document.php index 0c2dfcdd..f1d599bb 100644 --- a/src/Document.php +++ b/src/Document.php @@ -140,7 +140,6 @@ public function setDescribedByLink(string $href, array $meta=[]): void { } /** - * @throws InputException if the $level is unknown * @throws InputException if the $level is DocumentLevelEnum::Resource */ public function addMeta(string $key, mixed $value, DocumentLevelEnum $level=DocumentLevelEnum::Root): void { @@ -163,9 +162,6 @@ public function addMeta(string $key, mixed $value, DocumentLevelEnum $level=Docu case DocumentLevelEnum::Resource: throw new InputException('level "resource" can only be set on a ResourceDocument'); - - default: - throw new InputException('unknown level "'.$level->value.'"'); } } diff --git a/src/objects/RelationshipObject.php b/src/objects/RelationshipObject.php index 51668997..6d72df4b 100644 --- a/src/objects/RelationshipObject.php +++ b/src/objects/RelationshipObject.php @@ -45,8 +45,6 @@ public function __construct( * @param CollectionDocument|ResourceInterface|ResourceInterface[]|null $relation * @param array $links * @param array $meta - * - * @throws InputException if $relation is not one of the supported formats */ public static function fromAnything( array|CollectionDocument|ResourceInterface|null $relation, @@ -66,9 +64,6 @@ public static function fromAnything( elseif ($relation === null) { $relationshipObject = new RelationshipObject(RelationshipTypeEnum::ToOne); } - else { - throw new InputException('unknown format of relation "'.get_debug_type($relation).'"'); - } return $relationshipObject; } diff --git a/src/objects/ResourceIdentifierObject.php b/src/objects/ResourceIdentifierObject.php index 5f09cd4d..b45be053 100644 --- a/src/objects/ResourceIdentifierObject.php +++ b/src/objects/ResourceIdentifierObject.php @@ -112,7 +112,7 @@ public function setMetaObject(MetaObject $metaObject): void { */ public static function fromResourceObject(ResourceObject $resourceObject): static { if ($resourceObject->hasIdentification() === false) { - throw new InputException('resource has no identification yet<'); + throw new InputException('resource has no identification yet'); } $resourceIdentifierObject = new static($resourceObject->type, $resourceObject->primaryId()); diff --git a/tests/ResourceDocumentTest.php b/tests/ResourceDocumentTest.php index 144d574c..f9a2276c 100644 --- a/tests/ResourceDocumentTest.php +++ b/tests/ResourceDocumentTest.php @@ -6,6 +6,7 @@ use alsvanzelf\jsonapi\ResourceDocument; use alsvanzelf\jsonapi\enums\DocumentLevelEnum; +use alsvanzelf\jsonapi\enums\RelationshipTypeEnum; use alsvanzelf\jsonapi\exceptions\Exception; use alsvanzelf\jsonapi\exceptions\InputException; use alsvanzelf\jsonapi\interfaces\ExtensionInterface; @@ -100,6 +101,66 @@ public function testAddRelationship_DoNotIncludeContainedResources(): void { parent::assertArrayNotHasKey('included', $array); } + public function testAddRelationship_IdentifierOnlyObject(): void { + $document = new ResourceDocument(); + $document->setPrimaryResource(new ResourceIdentifierObject('user', 42)); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('the resource is an identifier-only object'); + + $document->addRelationship('foo', null); + } + + public function testAddLink_IdentifierOnlyObject(): void { + $document = new ResourceDocument(); + $document->setPrimaryResource(new ResourceIdentifierObject('user', 42)); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('the resource is an identifier-only object'); + + $document->addLink('foo', null); + } + + public function testSetSelfLink_IdentifierOnlyObject(): void { + $document = new ResourceDocument(); + $document->setPrimaryResource(new ResourceIdentifierObject('user', 42)); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('the resource is an identifier-only object'); + + $document->setSelfLink('https://jsonapi.org'); + } + + public function testSetAttributesObject_IdentifierOnlyObject(): void { + $document = new ResourceDocument(); + $document->setPrimaryResource(new ResourceIdentifierObject('user', 42)); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('the resource is an identifier-only object'); + + $document->setAttributesObject(new AttributesObject()); + } + + public function testAddRelationshipObject_IdentifierOnlyObject(): void { + $document = new ResourceDocument(); + $document->setPrimaryResource(new ResourceIdentifierObject('user', 42)); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('the resource is an identifier-only object'); + + $document->addRelationshipObject('foo', new RelationshipObject(RelationshipTypeEnum::ToOne)); + } + + public function testSetRelationshipsObject_IdentifierOnlyObject(): void { + $document = new ResourceDocument(); + $document->setPrimaryResource(new ResourceIdentifierObject('user', 42)); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('the resource is an identifier-only object'); + + $document->setRelationshipsObject(new RelationshipsObject()); + } + public function testAddMeta_HappyPath(): void { $document = new ResourceDocument(); $document->addMeta('foo', 'root', DocumentLevelEnum::Root); diff --git a/tests/helpers/RequestParserTest.php b/tests/helpers/RequestParserTest.php index 2f03689c..fa2120f9 100644 --- a/tests/helpers/RequestParserTest.php +++ b/tests/helpers/RequestParserTest.php @@ -87,12 +87,9 @@ public function testFromSuperglobals_WithPhpInputStream(): void { $_SERVER['REQUEST_URI'] = '/'; $_SERVER['CONTENT_TYPE'] = ContentTypeEnum::Official->value; + // empty $_POST so we get a bit more test coverage for input stream processing $_GET = []; - $_POST = [ - 'meta' => [ - 'foo' => 'bar', - ], - ]; + $_POST = []; $requestParser = RequestParser::fromSuperglobals(); diff --git a/tests/objects/ResourceIdentifierObjectTest.php b/tests/objects/ResourceIdentifierObjectTest.php index 64e65ff2..89539196 100644 --- a/tests/objects/ResourceIdentifierObjectTest.php +++ b/tests/objects/ResourceIdentifierObjectTest.php @@ -6,8 +6,10 @@ use alsvanzelf\jsonapi\exceptions\DuplicateException; use alsvanzelf\jsonapi\exceptions\Exception; +use alsvanzelf\jsonapi\exceptions\InputException; use alsvanzelf\jsonapi\interfaces\ExtensionInterface; use alsvanzelf\jsonapi\objects\ResourceIdentifierObject; +use alsvanzelf\jsonapi\objects\ResourceObject; use PHPUnit\Framework\TestCase; class ResourceIdentifierObjectTest extends TestCase { @@ -55,6 +57,35 @@ public function testSetLocalId_WithIdAlreadySet(): void { $resourceIdentifierObject->setLocalId('uuid-1'); } + public function testFromResourceObject_HappyPath(): void { + $resource = new ResourceObject('test', 1); + $resource->addAttribute('foo', 'bar'); + + $array = $resource->toArray(); + + parent::assertSame('test', $array['type']); + parent::assertSame('1', $array['id']); + parent::assertArrayHasKey('attributes', $array); + + $resourceIdentifierObject = ResourceIdentifierObject::fromResourceObject($resource); + + $array = $resourceIdentifierObject->toArray(); + + parent::assertSame('test', $array['type']); + parent::assertSame('1', $array['id']); + parent::assertArrayNotHasKey('attributes', $array); + } + + public function testFromResourceObject_NoFullIdentification(): void { + $resource = new ResourceObject(); + $array = $resource->toArray(); + + $this->expectException(InputException::class); + $this->expectExceptionMessage('resource has no identification yet'); + + ResourceIdentifierObject::fromResourceObject($resource); + } + public function testEquals_HappyPath(): void { $one = new ResourceIdentifierObject('test', 1); $two = new ResourceIdentifierObject('test', 2); @@ -168,6 +199,13 @@ public function testGetIdentificationKey_NoFullIdentification(): void { $resourceIdentifierObject->getIdentificationKey(); } + public function testIsEmpty_IdWithoutType(): void { + $resourceIdentifierObject = new ResourceIdentifierObject(); + $resourceIdentifierObject->setId(42); + + parent::assertFalse($resourceIdentifierObject->isEmpty()); + } + public function testIsEmpty_WithAtMembers(): void { $resourceIdentifierObject = new ResourceIdentifierObject(); @@ -190,4 +228,14 @@ public function testIsEmpty_WithExtensionMembers(): void { parent::assertFalse($resourceIdentifierObject->isEmpty()); } + + public function testPrimaryId_NoFullIdentification(): void { + $resourceIdentifierObject = new ResourceIdentifierObject(); + $primaryIdMethod = new \ReflectionMethod($resourceIdentifierObject, 'primaryId'); + + $this->expectException(Exception::class); + $this->expectExceptionMessage('resource has no identification yet'); + + $primaryIdMethod->invoke($resourceIdentifierObject); + } }