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

Woven methods lost their protected scope #50

Open
Cyrille37 opened this issue Sep 11, 2023 · 3 comments
Open

Woven methods lost their protected scope #50

Cyrille37 opened this issue Sep 11, 2023 · 3 comments
Assignees
Labels
bug 🐞 Something isn't working

Comments

@Cyrille37
Copy link
Contributor

Cyrille37 commented Sep 11, 2023

Hi

A woven protected method lost it's "protected" scope.

class TargetClass {
    public function helloWorld()
    {
    }
    protected function helloHere()
    {
    }
}
$obj = new TargetClass();
$obj->helloHere();

Without weaving it throws "Error: Call to protected method ..."

But after weaving the method became "public" so we can call it without error.

@WalterWoshid WalterWoshid added the bug 🐞 Something isn't working label Sep 11, 2023
@WalterWoshid WalterWoshid self-assigned this Sep 11, 2023
@WalterWoshid WalterWoshid changed the title [bug] Woven methods lost their protected scope Woven methods lost their protected scope Sep 12, 2023
@WalterWoshid
Copy link
Contributor

This, as a fact, was a choice I have made, I will add an option in the kernel to either deactive this behaviour or keep it active.

@Cyrille37
Copy link
Contributor Author

Cyrille37 commented Sep 17, 2023

Hello @WalterWoshid

Perhaps it's a good choice, what was your motivation for this behavior?

I don't remember if it conflicts with option "onlyPublicMethods" for "Implicit advice options: bypass trait methods, weaving only public methods" #51 ...

I just seen that when writing unit test ; it has no error while I was calling a protected method ;-)

@WalterWoshid
Copy link
Contributor

Hi @Cyrille37,

good question! In my old job we used Magento 2, which is a module-based e-commerce framework for an online shop. Here we mostly bought other modules for good money and sometimes we had to adjust the logic of those modules with plugins. Those worked like AOP, but they were ugly and the biggest problem we had was that sometimes we couldn't intercept private and protected classes and I think maybe even final classes and methods. For this reason, and because sometimes a private method just NEEDS to be intercepted, I have allowed my library to do just that.

Encapsulation is a good thing, don't get me wrong, but only if it is implemented correctly, which I don't often see. For that reason I have implemented this "feature", which I will add as an option to the kernel, so that people can disable it if they want :)

Also I later want to add intercepting code structure (see issue #46) which will take it to a more extrem approach of intercepting code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐞 Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants