Skip to content
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

[10.x] Skip isParameterBackedEnumWithStringBackingType for non ReflectionNamedType #46511

Merged
merged 2 commits into from
Mar 20, 2023
Merged

[10.x] Skip isParameterBackedEnumWithStringBackingType for non ReflectionNamedType #46511

merged 2 commits into from
Mar 20, 2023

Conversation

vladimirmartsul
Copy link
Contributor

#46483 May cause error Call to undefined method ReflectionUnionType::getName() at https://github.com/laravel/framework/blob/10.x/src/Illuminate/Support/Reflector.php#L151 if route action and parameter are defined as a union like this

public function edit(TwillModelContract|int $id): mixed {...}

(actual code from Twill CMS https://github.com/area17/twill/blob/3.x/src/Http/Controllers/Admin/ModuleController.php#L1131 )

This is because ReflectionUnionType and ReflectionIntersectionType don't have a getName() method, unlike ReflectionNamedType. So we need to skip checking if $parameter is not an instance of ReflectionNamedType.

…edType

Skip check isParameterBackedEnumWithStringBackingType() for ReflectionUnionType and ReflectionIntersectionType because they dosn't have getName() method
@driesvints
Copy link
Member

Thanks @vladimirmartsul. Can we also add a test for this?

@driesvints driesvints changed the title Skip isParameterBackedEnumWithStringBackingType for non ReflectionNamedType [10.x] Skip isParameterBackedEnumWithStringBackingType for non ReflectionNamedType Mar 20, 2023
@danharrin
Copy link
Contributor

Yeah this is a BC that we're also experiencing in Filament. It seems to also affect any Livewire mount() method.

@taylorotwell taylorotwell merged commit c5a5d1c into laravel:10.x Mar 20, 2023
@morloderex
Copy link
Contributor

@vladimirmartsul I am so sorry for this issue. There wasn't any tests that broke for me when i did this change.

And i didn't know that we needed to adhere for a special case regarding union types and intersection types. Thank you for the bugfix anyway.

@Faks
Copy link

Faks commented Mar 20, 2023

@vladimirmartsul I am so sorry for this issue. There wasn't any tests that broke for me when i did this change.

And i didn't know that we needed to adhere for a special case regarding union types and intersection types. Thank you for the bugfix anyway.

don't worry, at least we found them (who encountered them), we only could for future create for route registration regression tests to avoid such pitfalls?

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.

6 participants