Skip to content

IBX-12654: Added ibexa/phpstan and fixed dynamic calls to static methods - #839

Merged
wiewiurdp merged 6 commits into
6.0from
IBX-12654-added-ibexa-phpstan-and-fixed-dynamic-calls-to-static-methods
Sep 23, 2026
Merged

wiewiurdp merged 6 commits into
6.0from
IBX-12654-added-ibexa-phpstan-and-fixed-dynamic-calls-to-static-methods

Conversation

@bnowak

@bnowak bnowak commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor
🎫 Issue IBX-12654

Related PRs:

Description:

Adds ibexa/phpstan as a dev dependency and includes its extension in the PHPStan configuration.

Replaces dynamic calls to static methods ($this->assertX(), static test helpers, assertQueryResult, ...) with self::/ClassName:: calls: 463 calls fixed in 74 files. The staticMethod.dynamicCall strict rule itself is enabled in the linked ibexa/phpstan PR; this PR keeps CI green once that is released. One production-code fix: src/lib/Persistence/TransformationProcessor/PcreCompiler.php now calls Utf8Converter::toUnicodeCodepoint()/toUTF8Character() statically instead of through the $converter property inside a closure. Three stale baseline entries were removed (no longer matched after the self:: conversions changed PHPStan's error messages).

Also fixes what the existing ibexa/phpstan rules report: 133 closures/arrow functions got return types (Ibexa.requireClosureReturnType), across both production code (29 hits, e.g. cache decorators in src/lib/Persistence/Cache/*, RegenerateUrlAliasesCommand, Configuration, QueryType normalizers) and tests (104 hits). No Ibexa.noConfigResolverParametersInConstructor hits were reported for this package.

Judgment calls:

  • Where a closure's return value could not be narrowed further (e.g. Ibexa\Contracts\Core\Repository\Values\Content\Search\SearchHit::$valueObject, an unconstrained generic template; or values threaded through an untyped callable property), the closure is typed mixed.
  • Several PHPUnit mock willReturnCallback closures use an if ($matcher->numberOfInvocations() === N) { ...; return X; } chain with no default branch (safe in practice because expects(self::exactly(N)) guarantees the exact call count). These are typed as a nullable/union of the real branch types (e.g. ?string) with an explicit trailing return null;, since PHPStan requires an explicit return on every path regardless of the declared type's nullability.
  • Found and fixed 12 call sites (in BaseNumericValidatorTestCase.php, SearchBaseIntegrationTestCase.php, SearchMultivaluedBaseIntegrationTestCase.php, AbstractServiceTestCase.php) where the dynamic-call fix had used static:: for calls to abstract static methods instead of this codebase's documented convention ($this::method()); corrected in a follow-up commit.
  • .github/workflows/backend-ci.yaml: added the missing ibexa/gh-workflows/actions/setup-composer-root-version@main step (already used by rector.yaml) before composer-install in the cs-fix, deptrac, tests, integration-tests-postgres, integration-tests-mysql and solr-integration jobs. Without it, PR checkouts resolve to a dev-<sha> root version that doesn't satisfy ibexa/phpstan's own ibexa/core: ~6.0.x-dev || ^6.0 requirement, so composer install failed with an unresolvable circular-dependency error the moment ibexa/phpstan became a dependency of this package. This only ever surfaced once this PR introduced the first dependency cycle (core → phpstan → core); it's a genuine CI workflow gap, not something specific to our diff.
  • CI is fully green except SonarCloud's "Duplication on New Code" gate (25.4% vs the 3% threshold). All flagged lines are the one-line : Type closure-return-type insertions landing inside pre-existing, already-duplicated PHPUnit test blocks (e.g. PcreCompiler.php, TrashHandlerTest.php, FieldSortClauseParserTest.php/MapDistanceSortClauseParserTest.php); SonarCloud counts the whole pre-existing duplicated block as "new" whenever any line inside it changes. No new duplication was introduced and the flagged test fixtures are out of scope to refactor here.

For QA:

No functional change. CI (PHPStan, code style, unit tests) is the verification.

Documentation:

No client-facing impact.

@konradoboza konradoboza left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There are tons of FQCNs which need to be fixed by imports to name a few:

Comment thread src/bundle/Core/Features/Context/ContentContext.php Outdated
Comment thread src/bundle/Core/Features/Context/ContentContext.php Outdated
Comment thread src/bundle/Core/Features/Context/ContentContext.php Outdated
Comment thread src/bundle/Core/Features/Context/ContentContext.php Outdated
Comment thread src/lib/FieldType/Author/Type.php Outdated
Comment thread src/lib/Helper/ContentInfoLocationLoader/SudoMainLocationLoader.php Outdated
@bnowak
bnowak force-pushed the IBX-12654-added-ibexa-phpstan-and-fixed-dynamic-calls-to-static-methods branch from e63d529 to 2a669f2 Compare September 23, 2026 12:11
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

@wiewiurdp
wiewiurdp merged commit cc868b2 into 6.0 Sep 23, 2026
12 of 13 checks passed
@wiewiurdp
wiewiurdp deleted the IBX-12654-added-ibexa-phpstan-and-fixed-dynamic-calls-to-static-methods branch September 23, 2026 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants