Skip to content

Stop overriding previous tags #331

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

peterfox
Copy link
Collaborator

@peterfox peterfox commented Apr 19, 2025

Changes

  • AddGenericReturnTypeToRelationsRector updates
  • additional test scenario for AddGenericReturnTypeToRelationsRector

Why

Reported by @brunogaspar that the rule was working okay but would overwrite tags with a fully qualified name even when the tag was already correct.

This PR seeks to change the situation so if the tag exists, then it will not apply any changes.

Looking at this rule. There's a few changes that might be required when it comes to the pivot changes that were added in #316. I have a feeling relationships like MorphToMany haven't been considered and it should probably assign the Pivot class if the relationship uses one.

@peterfox peterfox self-assigned this Apr 19, 2025
@peterfox
Copy link
Collaborator Author

@brunogaspar if you're able to check this branch out and try it, that would be very useful.

This should fix the issue you've been having.

@brunogaspar
Copy link

@peterfox That seems to do it, thank you 🙏

@peterfox peterfox marked this pull request as ready for review April 19, 2025 16:27
@peterfox peterfox added the bug Something isn't working label Apr 19, 2025
@GeniJaho
Copy link
Collaborator

There's a new warning that popped up in tests after the latest core Rector changes:

ClassMethod/AddGenericReturnTypeToRelationsRector.php:393 - Undefined array key 1

It might have been there all along and not related to changes in this PR though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants