Skip to content

fix(symfony): remove redundant messenger dispatch docblocks#8410

Open
ousamabenyounes wants to merge 1 commit into
api-platform:4.3from
ousamabenyounes:fix-8312-dispatch-trait-docblock
Open

fix(symfony): remove redundant messenger dispatch docblocks#8410
ousamabenyounes wants to merge 1 commit into
api-platform:4.3from
ousamabenyounes:fix-8312-dispatch-trait-docblock

Conversation

@ousamabenyounes

Copy link
Copy Markdown
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #8312
License MIT
Doc PR n/a

Summary

The two Messenger DispatchTrait implementations declared @param object|Envelope $message. That union is redundant because the method already accepts object, and symfony/type-info rejects unions combining object with a concrete class.

This removes the redundant docblocks and adds a regression test that resolves any future @param $message docblock type from both traits through TypeInfo with the trait import context.

Local review

Reviewed before opening this PR. One test-risk finding was fixed before commit: the new regression test now records an assertion when a trait has no docblock or no @param $message, avoiding a future risky-test path.

Test verification (RED → GREEN)

RED, after adding the regression test on upstream main:

EE                                                                  2 / 2 (100%)

1) ApiPlatform\Symfony\Tests\Messenger\DispatchTraitTest::testDispatchMessageParamDocblockUsesTypeInfoCompatibleType@doctrine common messenger dispatch trait
Symfony\Component\TypeInfo\Exception\InvalidArgumentException: Cannot create union with both "object" and class type.

2) ApiPlatform\Symfony\Tests\Messenger\DispatchTraitTest::testDispatchMessageParamDocblockUsesTypeInfoCompatibleType@symfony messenger dispatch trait
Symfony\Component\TypeInfo\Exception\InvalidArgumentException: Cannot create union with both "object" and class type.

ERRORS!
Tests: 2, Assertions: 2, Errors: 2.

GREEN, with the fix:

..                                                                  2 / 2 (100%)

OK (2 tests, 2 assertions)

Fix-reverted regression check:

EE                                                                  2 / 2 (100%)

ApiPlatform\Symfony\Tests\Messenger\DispatchTraitTest::testDispatchMessageParamDocblockUsesTypeInfoCompatibleType
Symfony\Component\TypeInfo\Exception\InvalidArgumentException: Cannot create union with both "object" and class type.

ERRORS!
Tests: 2, Assertions: 0, Errors: 2.

Additional checks:

php vendor/bin/php-cs-fixer fix --config=.php-cs-fixer.dist.php --allow-unsupported-php-version=yes --dry-run --diff src/Symfony/Messenger/DispatchTrait.php src/Doctrine/Common/Messenger/DispatchTrait.php src/Symfony/Tests/Messenger/DispatchTraitTest.php
Found 0 of 3 files that can be fixed

php vendor/bin/phpstan analyse src/Symfony/Messenger/DispatchTrait.php src/Doctrine/Common/Messenger/DispatchTrait.php src/Symfony/Tests/Messenger/DispatchTraitTest.php
[OK] No errors

All commands were run inside ephemeral Docker containers with docker run --rm -u "$(id -u):$(id -g)" -v "$PWD":/app -w /app composer:2 ....

@soyuka

soyuka commented Jul 22, 2026

Copy link
Copy Markdown
Member

can this target 4.3 ? Not sure the dispatch trait test is useful drop it please.

@ousamabenyounes
ousamabenyounes force-pushed the fix-8312-dispatch-trait-docblock branch from 9de133a to 771c789 Compare July 23, 2026 11:15
@ousamabenyounes
ousamabenyounes changed the base branch from main to 4.3 July 23, 2026 11:15
@ousamabenyounes

Copy link
Copy Markdown
Author

Addressed this in 771c78919715269933a501fe14e8536be1f7da9c.

Changes:

  • retargeted the PR to 4.3
  • rebuilt the branch from 4.3 so the diff is only the two Messenger dispatch trait docblock removals
  • dropped the added DispatchTraitTest

Validation:

  • php -l src/Symfony/Messenger/DispatchTrait.php via Docker PHP 8.4
  • php -l src/Doctrine/Common/Messenger/DispatchTrait.php via Docker PHP 8.4
  • GitHub commitlint is green on the pushed SHA.

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.

2 participants