Skip to content

Conversation

@alongosz
Copy link
Member

@alongosz alongosz commented Mar 1, 2024

🎫 Issue n/a
BC breaks yes, for 5.0.0 major

Description:

\Ibexa\Contracts\Core\Repository\Values\Translation class has no __toString() method but all two implementations: Plural, and Message - have it. Given we operate on the common abstract contract, it causes static analysis issue:

Error: Cannot cast Ibexa\Contracts\Core\Repository\Values\Translation to string.

This method should be declared on the Translation abstract. The easiest way to solve it is to implement \Stringable on the abstract Translation class, forcing all its children to actually implement public function __toString(): string.

abstract class Translation extends ValueObject implements \Stringable

For QA:

No QA required, regressions are enough.

Documentation:

Should be documented as a breaking change. I'll update proper internal document that will be used when documenting all 5.0 breaks, so for this PR no "doc needed" label.

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (main for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review.

@alongosz alongosz force-pushed the fix-missing-translation-cls-stringable branch from 3394277 to af6c78b Compare March 1, 2024 15:49
@alongosz alongosz requested a review from a team March 1, 2024 15:52
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 1, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@konradoboza konradoboza requested a review from a team March 1, 2024 22:57
@adamwojs
Copy link
Member

adamwojs commented Mar 7, 2024

Can we implement \Stringable on \Ibexa\Contracts\Core\Repository\Values\Translation ? It should be covered by php 8.x polyfill package.

@alongosz alongosz force-pushed the fix-missing-translation-cls-stringable branch from af6c78b to 2c79176 Compare April 24, 2024 13:56
@sonarqubecloud
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@alongosz alongosz changed the title Defined fallback Translation::__toString implementation [BC break] Added implements \Stringable to Translation Value Object Apr 24, 2024
@alongosz alongosz requested review from a team, Steveb-p, ViniTou and konradoboza April 24, 2024 14:11
@alongosz alongosz merged commit 669c300 into main Apr 25, 2024
@alongosz alongosz deleted the fix-missing-translation-cls-stringable branch April 25, 2024 08:09
barw4 pushed a commit that referenced this pull request Oct 17, 2024
…ct (#344)

For more details see #344

Breaking changes:

Any class extending `\Ibexa\Contracts\Core\Repository\Values\Translation` will need to implement `\Stringable`.
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.

8 participants