Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

handle protected methods with the attribute syntax #1325

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

Conversation

mridul89
Copy link

@mridul89 mridul89 commented Mar 4, 2022

Summary

Laravel introduced a new syntax to define attribute "accessors / mutators" in laravel/framework#40022
Support for this new syntax was added in #1289

However, laravel documentation asks to use protected methods, but #1289 only looked at the public methods. This PR will also include protected methods.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • Add a CHANGELOG.md entry
  • Code style has been fixed via composer fix-style
  • Updated the existing test and the corresponding snapshot

the test now uses protected method, which is the correct way per laravel documentation
@sforward
Copy link
Contributor

@barryvdh I've tested this in our applications and it works perfectly. Could you look into this some time?

@keatliang2005
Copy link

@barryvdh can review this PR ?

@mfn
Copy link
Collaborator

mfn commented Feb 18, 2023

FTR: this competes with #1339 which covers additional cases and is thus preferred

@barryvdh
Copy link
Owner

I merged #1339
Does this solve your issue? @keatliang2005 @mridul89 @sforward

@vpratfr
Copy link

vpratfr commented Apr 11, 2024

I merged #1339 Does this solve your issue? @keatliang2005 @mridul89 @sforward

Hi,

#1531 is still pending, and may have been introduced by #1339. I don't know however if this one addresses that issue.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants