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

Improvements to Eloquent docblocks #258

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

Conversation

freerkminnema
Copy link

@freerkminnema freerkminnema commented Feb 13, 2025

Hi Joe (and/or other maintainers)!

Love the hard work y'all are doing with this extension.

Personally, I use the Eloquent attribute autocompletion feature a lot, and I've been hacking on the extension to make it match the database schema better.

I like the type mapping refactor done in #233, but I hope I can offer a few suggestions to make it even more robust:

1. Separate cast mapping from raw type mapping

Currently, the extension maps the attribute.cast (from the Eloquent model) in the same way as the raw attribute.type (from the database). But this often produces docblock types that do not match the actual PHP type.

For instance, an attribute that is cast to "datetime" has the \\Illuminate\\Support\\Carbon PHP type, but a MySQL table column with the raw type "datetime" is still just a string in PHP.

So given the following model:show output:

image

The extension in its current form, generates the following docblock:

image

But if you dump the record, it shows the actual PHP types, which are:

image

To solve this, I have split the list into mappings for casts and mappings for raw database types. If we fail to find a mapping for the cast, we try to map the type. This generates the following docblocks:

image

2. Increased the amount of supported database types

I'm sure we're not there yet, but I've gone through I think all the PostgreSQL column types and I've started on the MySQL ones.

I've used a lot more regular expressions, to group similar types, but I'm happy to break that up into a few more simple regex's if you want.

3. Return "mixed" if we don't find a mapping

Originally, when the extension failed to find a mapping, it would return the original type. This worked fine when dealing with custom casts, but not if there was no cast to begin with. The extension would then return the original raw database type as a final type. This would result in an invalid PHP type in the docblock.

Returning "mixed" instead means we should never generate invalid docblocks anymore.

4. Don't return "mixed|null"

Since mixed is a standalone type, I prevent the final type from becoming mixed|null, because that is not a valid type.

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.

1 participant