Skip to content

Laravel App Analyzer #328

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

Merged
merged 4 commits into from
Apr 12, 2025
Merged

Laravel App Analyzer #328

merged 4 commits into from
Apr 12, 2025

Conversation

peterfox
Copy link
Collaborator

@peterfox peterfox commented Apr 8, 2025

Changes

  • Adds an App analyzer that manages finding the version in Laravel
  • Updates AddGenericReturnTypeToRelationsRector
  • Updates AddGenericReturnTypeToRelationsRector tests
  • Adds a InteractsWithLaravelVersion trait to use with tests
  • Removes some files used to control app version within tests

Why

Overall this is just a way to simplify how we make rules that might rely on the Laravel version.

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

@GeniJaho GeniJaho left a comment

Choose a reason for hiding this comment

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

Great addition 💪

@peterfox peterfox merged commit ed0cb99 into main Apr 12, 2025
5 checks passed
@peterfox peterfox deleted the feature/laravel-app-analyzer branch April 12, 2025 19:22
@brunogaspar
Copy link

Hello guys, sorry to bother, but something on these changes is causing the following regression:

    ---------- begin diff ----------
@@ @@
     }

     /**
-     * @return BelongsTo<User, $this>
+     * @return \Illuminate\Database\Eloquent\Relations\BelongsTo<\<REDACTED>\Users\Models\User, $this>
      */
     public function user(): BelongsTo
     {
    ----------- end diff -----------

Applied rules:
 * AddGenericReturnTypeToRelationsRector

I've downgraded to 2.0.3 (just to double check) and it did not happen there, then updated again to 2.0.4 and it yielded the above result sadly.

@peterfox
Copy link
Collaborator Author

Just to clarify but is the only difference is that you're getting the fully qualified name instead of the short model?

Might be the PR before this that caused the break by not checking if the tag is already set.

@brunogaspar
Copy link

Thanks for the reply @peterfox

Yes exactly, it sets the FQCN all the time now. I use PHP CS Fixer to "remove" the FQCN but rector puts it back.

You're referring to #324 ? I can try to revert the changes locally to check

@peterfox
Copy link
Collaborator Author

@brunogaspar no, more likely #316

@brunogaspar
Copy link

ah i see..

i've reverted the file to this version and the issue is gone, so you're correct, it's not your PR.

@peterfox
Copy link
Collaborator Author

I've got a test that covers your scenario and confirmed it. I just need to do a bit of digging to fix it.

@brunogaspar
Copy link

Thank you @peterfox !

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.

3 participants