laravel-ide-helper
laravel-ide-helper copied to clipboard
barryvdh
Add support for non type-hinted attribute accessors with no backed property
Summary
Even though the documentation doesn't state this explicitly at the time of writing, it is possible to use the new Attribute accessor to create a calculated property where there is no backing property.
This solves #1315.
I added a check for if the Attribute accessor function has no specified type, and then add it without type.
Type of change
- [x] New feature (non-breaking change which adds functionality)
Checklist
- [x] Existing tests have been adapted and/or new tests have been added
- [x] Add a CHANGELOG.md entry
- [x] Code style has been fixed via
composer fix-style
A little question, the PR works with "untyped get attributes", but what happen with "untyped set attributes"?
A little question, the PR works with "untyped get attributes", but what happen with "untyped set attributes"?
Because of this I added a lot more tests including a few edge cases.
I made it so that if there's a getter, that's the type, but otherwise it takes the parameter type of the setter (since the setter return type doesn't really matter).
It also seems to seems that the PR is still approved even though I made new changes afterwards. @mfn could you please look at it again?
@mfn could you look at this PR? Its changes are well described and there's tests for both the happy path as well as a few edge cases.
I would like to have this PR merged so that I can update #1339, which is required for the proper usage of the Attribute accessors that came with Laravel 9.
Is there any update on this? This feature would be highly appreciated.
No, not yet. Though I'm back from vacation 😅 it's been a super busy year so far.
@mfn also looking forward to this change being out. Hopefully it's not a major one and can be merged before christmas. If not, no worries. Enjoy your christmas break and hope the following year isn't too busy
I updated the PR to be in line with the most recent changes.
I don't know what I did to close the PR. Please re-open it, review it and merge it if there are no issues.
The PR contains tests that cover the changes made, so it shouldn't be that much work.
It seems that you force pushed your branch to the HEAD commit of the upstream, so your changes have been overwritten (that's why the PR doesn't have any changes and is probably closed automatically)
It seems that you force pushed your branch to the HEAD commit of the upstream, so your changes have been overwritten (that's why the PR doesn't have any changes and is probably closed automatically)
The branch was closed half an hour before I force pushed my changes. A force push won't close a PR. I don't remember clicking on any 'Close issue' buttons, either. Weird.
It seems that you force pushed your branch to the HEAD commit of the upstream, so your changes have been overwritten (that's why the PR doesn't have any changes and is probably closed automatically)
The branch was closed half an hour before I force pushed my changes. A force push won't close a PR. I don't remember clicking on any 'Close issue' buttons, either. Weird.
Not sure if that gap is correct.
Also, Github does close the PR automatically:
https://stackoverflow.com/a/46053849
Then it seems I have my facts mixed up. Thanks for pointing it out! I'll be sure to remember that for next time.
You can force push your branch back to https://github.com/barryvdh/laravel-ide-helper/commit/3e671ab9182b294392517696160604a94e9ce278 if you'd like to restore everything. (You might need to verify if that's the most recent changeset though)
The branches were a bit of a mess. I rebased both this and #1339 on the most recent develop and force pushed to both make sure they're up-to-date and untangle the branch mess.
With that in mind, do you think we're good?
Edit: By the way. I force pushed #1339 as well and that didn't close.
@mfn could you please re-open this PR and give it a look? I closed it by mistake and can't reopen it myself.
