Skip to content

Conversation

@yoeunes
Copy link
Contributor

@yoeunes yoeunes commented Nov 13, 2025

Q A
Branch? 7.3
Bug fix? yes
New feature? no
Deprecations? no
Issues Fix #62374
License MIT

This PR fixes an issue where PriorityTaggedServiceTrait would call the defaultIndexMethod (like getDefaultName()) even when:

  1. The iterator was not indexed at all.
  2. The index was already provided by the tag attribute.

This caused an InvalidArgumentException for services with a non-static method matching the defaultIndexMethod name.

The logic is now corrected to only call the defaultIndexMethod as a fallback when an index is needed but not provided by the tag.

@carsonbot carsonbot added this to the 7.3 milestone Nov 13, 2025
@yoeunes yoeunes force-pushed the di-trait-default-index-lookup-7.3 branch 3 times, most recently from 637ba9d to 23d25c8 Compare November 13, 2025 23:29
@yoeunes yoeunes changed the title [DependencyInjection] Fix default index method call in PriorityTaggedServiceTrait [Dependencyinjection] Call default index method when index is not provided by tag Nov 13, 2025
@nicolas-grekas nicolas-grekas force-pushed the di-trait-default-index-lookup-7.3 branch from 23d25c8 to 66ca601 Compare November 14, 2025 08:22
Copy link
Member

@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I've updated the code to make it closer to how it was before #62329
This helps with merging fixes up and also reducing the risk of unattended regressions.

@nicolas-grekas
Copy link
Member

Thank you @yoeunes.

@carsonbot carsonbot changed the title [Dependencyinjection] Call default index method when index is not provided by tag [DependencyInjection] Call default index method when index is not provided by tag Nov 14, 2025
@nicolas-grekas nicolas-grekas merged commit b9a4552 into symfony:7.3 Nov 14, 2025
10 of 11 checks passed
@alexander-schranz
Copy link
Contributor

alexander-schranz commented Nov 14, 2025

Can confirm it fixes my issue. #62374 (7.3-dev works, think 7.4-dev fails as not yet backmerged) Thx 👍

@dmaicher
Copy link
Contributor

Same here 👍 Thanks @yoeunes @nicolas-grekas

This was referenced Nov 16, 2025
@fabpot fabpot mentioned this pull request Dec 7, 2025
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.

5 participants