Skip to content

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

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

konradoboza merged 4 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, ...) with self::/ClassName:: calls: 23 calls in 6 files. The staticMethod.dynamicCall strict rule itself is enabled in the linked ibexa/phpstan PR; this PR keeps CI green once that is released.
Also fixes what the existing ibexa/phpstan rules report: 24 closures/arrow functions got return types (Ibexa.requireClosureReturnType), 3 converter constructors (Html5, Html5Edit, Html5Input) no longer read ConfigResolver parameters (Ibexa.noConfigResolverParametersInConstructor) — the resolver is now kept as a property and the custom stylesheets are resolved lazily on first use, with Xslt::addCustomStylesheets() extracted so the subclasses can call it after construction.
While fixing a pre-existing PHPStan argument.type finding in RichTextTransformerTest (honestly, not by baselining), one baseline entry became unmatched and was removed.
A follow-up commit extracts the shared logic of Html5, Html5Edit and Html5Input into a new ConfigResolverAwareXsltConverter base class: the initial Ibexa.noConfigResolverParametersInConstructor fix made those three classes near-identical, which SonarCloud correctly flagged as new duplicated code (67% on new lines, gate is 3%); this was genuinely new duplication introduced by this PR, not the known pre-existing-test-fixture false positive, so it was fixed rather than worked around.

Known CI failure (SonarCloud duplication gate): after that fix, SonarCloud still fails the "duplication on new code" gate (14.2%, threshold 3%), now entirely attributed to tests/lib/RichText/RendererTest.php (64% on its 28 new lines). This is the documented false positive: the flagged hasParameter/getParameter/warning/error/__get mock closures are pre-existing, near-identical blocks that already existed 2-3 times in this file on 6.0 before this branch; this PR only added a return-type declaration (and one self::/static conversion in commit 2) inside each occurrence, which makes SonarCloud attribute the whole pre-existing duplicated block as "new code". No new duplication was introduced. Per the established policy for this pattern, the test fixtures were intentionally left untouched rather than refactored to dodge the gate; all other checks (PHPStan, code style, unit, integration, browser tests) are green.

For QA:

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

Documentation:

No client-facing impact.

@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

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

See analysis details on SonarQube Cloud

@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.

@bnowak do you feel sanities are needed here given refactor caused by ConfigResolver call in constructors?

@konradoboza

Copy link
Copy Markdown
Contributor

After internal sync we decided to merge this one as-is to unblock further progress.

@konradoboza
konradoboza merged commit d4a6c64 into 6.0 Sep 23, 2026
19 of 20 checks passed
@konradoboza
konradoboza deleted the IBX-12654-added-ibexa-phpstan-and-fixed-dynamic-calls-to-static-methods branch September 23, 2026 12:56
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