Skip to content

Modernize code#611

Open
jrauh01 wants to merge 41 commits intomainfrom
modernize
Open

Modernize code#611
jrauh01 wants to merge 41 commits intomainfrom
modernize

Conversation

@jrauh01
Copy link
Copy Markdown
Contributor

@jrauh01 jrauh01 commented Feb 20, 2026

This PR is a broad modernization and cleanup sweep across the whole codebase. The goal is to move the project toward more idiomatic modern PHP, improve type safety, reduce deprecated usages, and improve readability.

Because the vast amount of changes PR explicitly makes no claim to completeness. It is a best effort refactor pass: there may still be remaining places that could benefit from the same transformations. So this should be treated as first big step in an incremental modernization rather than a finished “cleanup of everything”.

Typing and signatures

  • Add/strengthen parameter, return, and property types throughout the codebase.
  • Align fluent APIs and inheritance signatures (static/self, consistent return types).
  • Improve PHPDoc accuracy and consistency, including proper @deprecated tags.
  • Use ?Type shorthand for nullable PHPDoc annotations.

Modern PHP language features

Replace older constructs with modern equivalents where appropriate:

  • match, nullsafe ?->, null coalescing ??/??=, ternary, arrow functions fn() =>, first-class callables.
  • Modern string helpers (str_starts_with, str_ends_with, str_contains) instead of substr/strpos/regex for simple checks.
  • Variadic arguments instead of func_get_args().

Consistency / formatting / readability

  • Formattings: imports sorted, redundant else branches removed, small temporary variables inlined, unreachable code commented out.
  • Standardize array formatting in some places (not all places; requires further discussions about a unified standard) and removes trailing commas where used in multi-line arrays.
  • Rework query builder method chains for consistent indentation.
  • Refactor table column creation blocks to a uniform style (chaining, spacing, renderer callbacks).
  • Remove PHPDoc annotation alignment.

Internal structure improvements

  • Introduces small abstractions to reduce duplication (for example a shared table base for percent/usage patterns).
  • Converts some "const string" typed concepts into enums ( ´ObjectType, MonitoringStateTrigger, CheckPluginState, and ResultStatus`).

@cla-bot cla-bot Bot added the cla/signed label Feb 20, 2026
@jrauh01 jrauh01 self-assigned this Feb 23, 2026
@jrauh01 jrauh01 requested a review from lippserd February 24, 2026 14:18
@jrauh01 jrauh01 force-pushed the support-php-8.5 branch 2 times, most recently from bdf6581 to db4e52f Compare February 26, 2026 12:11
Base automatically changed from support-php-8.5 to main February 26, 2026 12:29
Comment thread library/Vspheredb/Web/Form/VCenterShipMetricsForm.php Outdated
Comment thread library/Vspheredb/Web/Form/PerfdataConsumerForm.php Outdated
@jrauh01 jrauh01 force-pushed the modernize branch 6 times, most recently from 03fb727 to 4d6f364 Compare March 4, 2026 13:10
@jrauh01
Copy link
Copy Markdown
Contributor Author

jrauh01 commented Mar 4, 2026

The last two force pushes fixed the failed jobs.

jrauh01 added 11 commits March 20, 2026 08:41
- Add explicit parameter, return, and property types.
- Remove runtime type checks where signatures now enforce valid input.
- Change `self` return type to `static` in most cases.
- Added phpdocs. (not for all because the commit grew to much)
- Format existing phpdocs.
- Use `use` statements to import classes instead of using namespaced references.
- Drop unnecessary global prefixes for built-in function calls. (for example
  `\strlen`, `\sprintf` -> ´strlen`, `sprintf`)
Use match espressions where possible.
Replace old function patterns by more modern functions.

- `strpos(...) !== false` with `str_contains(...)`
- `substr(..., 0, N) === '...'` and `/^.../` regex checks with
  `str_starts_with(...)`
- `substr(..., -N) === '...'` and `/...$/` regex checks with
  `str_ends_with(...)`
Refactor a few simple `function (...) { return ...; }` closures over to
`fn (...) => ...` where the callback is a single expression.
Modernize IcingaCliRunner::command() by switching from func_get_args() to a
variadic signature.

Preserves backwards compatibility with callers that pass a single array: if the
method receives exactly one argument and it’s an array, it unwraps it.
Replace passing raw arrays into `addAttributes()` with
`Attributes::create([...])`.
Remove obsolete PHP < 8.1 compatibility code
- Remove stored closure workaround for normalizeBatchResult, decodeResponse,
and requireValueProperty
- Remove related comments referencing future PHP 8.1 changes
Add PHPDoc `@deprecated` annotations so IDEs and static analysis tools recognize
them as deprecated.
jrauh01 added 24 commits March 20, 2026 08:43
- Drop a bunch of temporary variables that were only used once, replacing them
  with direct expressions.
- Replace small loops used only for editing each entry of  an array with
  `array_map()`.
- Remove unused variables:
  - `$e`/`$_` Exception variables.
  - `foreach` keys.
  - Other
Remove trailing commas from multi-line arrays for consistency.
Rewrite many `$db->select()->from()->join()->where();` calls to a consistent
“one method per line” style. Split longer `from()`/`join()`/`where()` argument
lists across lines and align indentation so chained queries are easier to scan
and edit.
This commit does **not** format all array to the same format. It formats all
keys of each array in the same way so they are uniform in themselves.

- If array keys are aligned partly now the full array is aligned.
- Remove trailing spaces in inline arrays `[ 'a', 'b' ]` -> `['a', 'b']`.
- Use arrow functions for simple renderers and keep full closures only when
  multi step logic is needed.
- Consistent method chaining layout.
- Add empty lines between column objects.
Mixed commit that reformats code for readability.

- Reflow a few multi-line calls into a clearer style.
- Add spaces after `!` and type casts.
- Override `protected $method` instead of setting it in the constructor.
- Add empty lines.
Comment out...

- code below a return statement,
- code inside of an `if (false)` block,
- variable assignments directly before the variable is overwritten,
- empty `if` block

 ...to improve readability by reducing noise.
Sort imports alphabetically and remove unused imports.
The DB readiness check was incorrect due to operator precedence.

The logical NOT operator (`!`) was applied before the strict comparison
(`===`), causing the component state to be negated and then compared to
`STATE_READY`. This resulted in the condition never evaluating as intended.

Replace the negated comparison with an explicit `!== STATE_READY` check
to correctly detect when the DB component is not ready.
`updateDb()` returns either an **integer** or **true**, but never **false**.
Because the check works with a strict comparison, this means it will never be
**false**. Thus the strict `!== false` comparison is replaced by a truthy check
to correctly handle integer return values and failures.
For the methods `Form::onSuccess()` and `HtmlDocument::assemble()`.
This reverts commit e77d6eead174169719298ba2c2123c7f07c8516e.

This fix caused the daemon to freeze.
Comment thread library/Vspheredb/Web/Form/VCenterShipMetricsForm.php Outdated
@lippserd lippserd added this to the v1.9.0 milestone Mar 26, 2026
Remove all types from `isValidEvent()`
the forward compatibility change is moved to a different PR

This commit will be squashed into the original commit later.
@BastianLedererIcinga
Copy link
Copy Markdown

I moved return type additions for ipl forward compatibilty to a different PR.

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.

4 participants