Skip to content

Fix some new phpstan issues in new packages #7783

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

alexander-schranz
Copy link
Member

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Fixed tickets fixes #
Related issues/PRs #
License MIT
Documentation PR sulu/sulu-docs#

What's in this PR?

Fix some bew phpstan issues in new packages.

Why?

After upgrade to PHPStan 2.0 we got some new issues on the 3.0 branch we tackle them here before doing a baseline.

@alexander-schranz alexander-schranz added the DX Affecting the end developer label Feb 4, 2025
@alexander-schranz alexander-schranz changed the title Fix some bew phpstan issues in new packages Fix some new phpstan issues in new packages Feb 4, 2025
*/
private function setExcerptData(ExcerptInterface $dimensionContent, array $data): void
{
if (\array_key_exists('excerptTitle', $data)) {
Assert::nullOrString($data['excerptTitle']);
Copy link
Member Author

Choose a reason for hiding this comment

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

not sure about this "runtime" checks maybe we should go with assert instead which is compiled away on prod for better performance in cases used for imports.

$dimensionContent->setSeoHideInSitemap($data['seoHideInSitemap'] ?? false);
$dimensionContent->setSeoNoFollow($data['seoNoFollow'] ?? false);
$dimensionContent->setSeoNoIndex($data['seoNoIndex'] ?? false);
if (\array_key_exists('seoTitle', $data)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

this was missing. See excerpt as or other data mapper as reference only set data if send else stay untouched.

Copy link
Member Author

Choose a reason for hiding this comment

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

moved to the TestBundle

@alexander-schranz
Copy link
Member Author

@Prokyonn would be good if you could target the ContentResolver part in a PR:

Bildschirmfoto 2025-02-05 um 13 41 28

@alexander-schranz alexander-schranz merged commit 8b67447 into sulu:3.0 Feb 5, 2025
9 checks passed
@alexander-schranz alexander-schranz deleted the enhancement/phpstan-issues-packages branch February 5, 2025 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Affecting the end developer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants