Skip to content

Conversation

@barw4
Copy link
Contributor

@barw4 barw4 commented Jul 7, 2025

🎫 Issue IBX-9727

Description:

For QA:

Documentation:

@barw4 barw4 requested a review from a team July 9, 2025 09:14
@ezrobot ezrobot requested review from Steveb-p, ViniTou, adamwojs, alongosz, ciastektk, konradoboza, mikadamczyk, tbialcz and wiewiurdp and removed request for a team July 9, 2025 09:14
Copy link
Member

@adamwojs adamwojs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following symbols are missing type hints:

  • \Ibexa\Rest\Input\FieldTypeParser::parseValidatorConfiguration
  • \Ibexa\Rest\Input\Handler\Xml::$fieldTypeHashElements
  • \Ibexa\Rest\Server\Service\RootResourceBuilderInterface::buildRootResource
  • \Ibexa\Rest\Server\Output\ValueObjectVisitor\Section::visitSectionAttributes
  • \Ibexa\Rest\Server\Output\ValueObjectVisitor\UserSession::visitUserSessionAttributes
  • \Ibexa\Bundle\Rest\DependencyInjection\Compiler\InputHandlerPass::INPUT_HANDLER_SERVICE_TAG

@barw4 barw4 changed the title Added missing strict types IBX-9727: Added missing strict types Jul 9, 2025
@barw4
Copy link
Contributor Author

barw4 commented Jul 9, 2025

Resolved in cfe0d72

@barw4 barw4 requested a review from adamwojs July 9, 2025 09:29

/** @var mixed|null */
private $valueObject = null;
private mixed $valueObject = null;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Q: Is it really mixed, not object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines 23 to +25
* @param \Ibexa\Rest\Server\Values\CreatedPolicy $data
*/
public function visit(Visitor $visitor, Generator $generator, $data): void
public function visit(Visitor $visitor, Generator $generator, mixed $data): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So after my suggestion for src/lib/Server/Output/ValueObjectVisitor/Policy.php this one pops up for PHPStan, but I think strict types help us here uncover composition bug. CreatedPolicy extends ValueObject and has inner Policy object as a composition. Therefore CreatedPolicy extends Policy is incorrect - Policy visitor should also be a composition.

See also that in src/bundle/Resources/config/value_object_visitors.yml:431 parent for CreatePolicy is set to ValueObjectVisitor, not Policy - which IMO is correct, the implementation needs not to extend Policy.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, should be better now: 213a5f9

@barw4 barw4 requested a review from alongosz July 9, 2025 13:22
@sonarqubecloud
Copy link

sonarqubecloud bot commented Jul 9, 2025

Quality Gate Failed Quality Gate failed

Failed conditions
4.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@adamwojs adamwojs merged commit 36bdd3c into main Jul 9, 2025
9 of 10 checks passed
@adamwojs adamwojs deleted the strict-types branch July 9, 2025 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants