Modernize code by BastianLedererIcinga · Pull Request #155 · Icinga/ipl-orm

BastianLedererIcinga · 2025-12-09T09:41:28Z

Add property, parameter and return types.
Replace return type self with static.
Some InvalidArgumentExceptions became obsolete by the new parameter types, unit tests have been adjusted to expect a TypeError instead.

Code that requires compatibility changes in modules is not modernized by this PR.

src/Query.php

src/Relation/BelongsToMany.php

src/Behaviors.php

src/Compat/FilterProcessor.php

src/ResolvedExpression.php

src/Query.php

The base branch was changed.

lippserd · 2026-01-14T16:19:58Z

src/Query.php

     * @return $this
     */
-    public function peekAhead($peekAhead = true)
+    public function peekAhead($peekAhead = true): static


Can this be bool $peekAhead?

I've checked out all usages, the only cases where this could end up not being a bool are in some controllers like this one:
https://github.com/Icinga/icingaweb2-module-x509/blob/81b7e5b6611e5a0c09bb19faca194583a79b462c/application/controllers/UsageController.php#L88
Where view->compact is passed, which could technically return null or some other type. The ActionController always initializes this property with a bool in the constructor and all reassignments also use bool values. When testing the affected controllers in the UI everything worked fine aswell, so I suppose we can type the parameter as bool.

lippserd · 2026-01-14T16:20:23Z

src/Query.php

@@ -704,7 +700,7 @@ public function peekAhead($peekAhead = true)
     *
     * @return \Generator


You can simplify this to Generator.

lippserd · 2026-01-14T16:23:43Z

src/Relation.php

     * @return \Generator
     */
-    public function resolve()
+    public function resolve(): \Generator


Add use statement and simplify both PHPDoc and return type to Generator.

lippserd · 2026-01-14T16:25:11Z

src/Relation/BelongsToMany.php

    }

-    public function resolve()
+    public function resolve(): \Generator


Add use statement.

lippserd · 2026-01-14T16:28:14Z

src/UnionQuery.php

+     * @return ?Query[]
     */
-    public function getUnions()
+    public function getUnions(): ?array


This method does not return null, does it?

lippserd · 2026-01-14T16:30:06Z

tests/RelationTest.php

        $this->assertSame($targetClass, $relation->getTargetClass());
    }

-    public function testSetTargetClassThrowsInvalidArgumentExceptionIfNotString()


This test can be removed because we are no longer testing an implementation detail, but PHP itself.

lippserd · 2026-01-15T09:29:27Z

src/Exception/InvalidRelationException.php

-    /** @var Model The target model */
-    protected $model;
+    /** @var ?Model The target model */
+    protected ?Model $model;


I think it always makes sense to initialize nullable properties with null, as you do everywhere else. Even here, where the constructor sets the property.

lippserd · 2026-01-15T10:39:33Z

src/Exception/InvalidRelationException.php

     * @return Model
     */
-    public function getModel()
+    public function getModel(): Model


This method may return null.

sukhwinder33445

I think we can define return types for PersistBehavior|RetrieveBehavior interfaces, as they are not overridden by any other module.
Please use [...] instead of list()
Please import the class instead of using qualifier in PropertiesWithDefaults (line 10-11).
Please remove the unused variable $err in MillisecondTimestamp.

Note for myself:
The following classes and interfaces have not yet been modernized, as they are overridden by some modules (TBD):
Classes:

PropertyBehavior
Binary|BoolCast|MillisecondTimestamp
Model
UnionModel
ResultSet
Relation (partially)
Query (Partially)

Interfaces:

QueryAwareBehavior
RewriteColumnBehavior
RewriteFilterBehavior

src/Behaviors.php

src/Exception/InvalidRelationException.php

src/Exception/InvalidColumnException.php

sukhwinder33445

Looks good.
Please use backticks in the commits to highlight function, class, interface, etc. names.

Add types and missing return statements for implementations of: * `QueryAwarebehavior` * `RewriteFilterbehavior` To ensure compatibility with Icinga/ipl-orm#155. Add types to `ContactForm::isValidEent()` as required by Icinga/ipl-stdlib#68

Add strict type declarations to properties, function/method signatures, where types are unambiguous and no inheritance or interface contract is affected.

sukhwinder33445

Looks good to me.
@lippserd, please take a look.

src/ColumnDefinition.php

src/Hydrator.php

src/Relation.php

src/Resolver.php

Where functions are already adjusted and `throws` tags are missing they are added

cla-bot bot added the cla/signed label Dec 9, 2025

BastianLedererIcinga changed the base branch from main to 8.4/8.5-compatibility December 9, 2025 13:28

BastianLedererIcinga marked this pull request as ready for review December 9, 2025 13:30

BastianLedererIcinga requested a review from Jan-Schuppik December 9, 2025 13:30

Jan-Schuppik reviewed Dec 9, 2025

View reviewed changes

src/Query.php Outdated Show resolved Hide resolved

Jan-Schuppik reviewed Dec 9, 2025

View reviewed changes

src/Query.php Show resolved Hide resolved

BastianLedererIcinga force-pushed the modernize-code branch from a48b99b to 7d98bbe Compare December 9, 2025 15:11

BastianLedererIcinga requested a review from Jan-Schuppik December 9, 2025 15:12

BastianLedererIcinga self-assigned this Dec 10, 2025

Jan-Schuppik previously approved these changes Dec 10, 2025

View reviewed changes

BastianLedererIcinga force-pushed the modernize-code branch from 7d98bbe to e194919 Compare December 10, 2025 14:47

sukhwinder33445 requested changes Dec 11, 2025

View reviewed changes

BastianLedererIcinga requested a review from sukhwinder33445 December 11, 2025 10:49

BastianLedererIcinga force-pushed the modernize-code branch from 87bf2cd to a83714b Compare December 11, 2025 14:23

sukhwinder33445 previously approved these changes Dec 11, 2025

View reviewed changes

BastianLedererIcinga force-pushed the 8.4/8.5-compatibility branch 3 times, most recently from 40c7f0b to be36f28 Compare December 18, 2025 14:17

Base automatically changed from 8.4/8.5-compatibility to main January 7, 2026 12:55

BastianLedererIcinga force-pushed the modernize-code branch from a83714b to 1538b72 Compare January 8, 2026 07:43

sukhwinder33445 previously approved these changes Jan 13, 2026

View reviewed changes

lippserd requested changes Jan 15, 2026

View reviewed changes

BastianLedererIcinga dismissed sukhwinder33445’s stale review via 6a3071e January 16, 2026 08:48

BastianLedererIcinga requested a review from lippserd January 16, 2026 08:49

BastianLedererIcinga force-pushed the modernize-code branch from 6a3071e to 45aa085 Compare February 27, 2026 08:48

sukhwinder33445 reviewed Feb 27, 2026

View reviewed changes

src/Behaviors.php Outdated Show resolved Hide resolved

src/Exception/InvalidRelationException.php Outdated Show resolved Hide resolved

src/Exception/InvalidColumnException.php Outdated Show resolved Hide resolved

Jan-Schuppik mentioned this pull request Mar 2, 2026

Adjust modules for compatibility with ipl-stdlib modernization Icinga/ipl-stdlib#68

Open

5 tasks

BastianLedererIcinga requested a review from sukhwinder33445 March 2, 2026 11:45

BastianLedererIcinga force-pushed the modernize-code branch from ad48a0e to 25b8768 Compare March 2, 2026 13:45

sukhwinder33445 reviewed Mar 3, 2026

View reviewed changes

BastianLedererIcinga force-pushed the modernize-code branch 2 times, most recently from 28bd4f2 to 1a6a329 Compare March 3, 2026 13:46

BastianLedererIcinga force-pushed the modernize-code branch 4 times, most recently from e1d1c36 to 9734f88 Compare March 4, 2026 12:42

BastianLedererIcinga added 3 commits March 4, 2026 16:37

Add non-breaking strict type declarations

4f90d69

Add strict type declarations to properties, function/method signatures, where types are unambiguous and no inheritance or interface contract is affected.

Replace return type self with static

5e20d35

Optimize imports

a04c17f

BastianLedererIcinga force-pushed the modernize-code branch 2 times, most recently from 59f2fe9 to 9b944e8 Compare March 5, 2026 08:11

sukhwinder33445 previously approved these changes Mar 5, 2026

View reviewed changes

sukhwinder33445 reviewed Mar 5, 2026

View reviewed changes

src/ColumnDefinition.php Show resolved Hide resolved

src/Hydrator.php Outdated Show resolved Hide resolved

src/Relation.php Outdated Show resolved Hide resolved

src/Resolver.php Show resolved Hide resolved

BastianLedererIcinga added 2 commits March 5, 2026 11:24

Simplify FQN

92c3a4a

Use array destructuring instead of list()

0ca568a

BastianLedererIcinga dismissed sukhwinder33445’s stale review via 2bcbda8 March 5, 2026 10:27

BastianLedererIcinga force-pushed the modernize-code branch from 9b944e8 to 2bcbda8 Compare March 5, 2026 10:27

sukhwinder33445 previously approved these changes Mar 5, 2026

View reviewed changes

Add missing @throws tags

cee39f3

Where functions are already adjusted and `throws` tags are missing they are added

BastianLedererIcinga dismissed sukhwinder33445’s stale review via cee39f3 March 13, 2026 08:22

BastianLedererIcinga force-pushed the modernize-code branch from 2bcbda8 to cee39f3 Compare March 13, 2026 08:22

		@@ -704,7 +700,7 @@ public function peekAhead($peekAhead = true)
		*
		* @return \Generator

Conversation

BastianLedererIcinga commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sukhwinder33445 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sukhwinder33445 left a comment

Choose a reason for hiding this comment

Uh oh!

sukhwinder33445 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

BastianLedererIcinga commented Dec 9, 2025 •

edited

Loading

sukhwinder33445 left a comment •

edited

Loading