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

Add traits and don't import their methods #89

Merged
merged 1 commit into from
Sep 14, 2021
Merged

Add traits and don't import their methods #89

merged 1 commit into from
Sep 14, 2021

Conversation

mikepsinn
Copy link
Contributor

Previously the factory wasn't importing used traits from existing classes

@dg
Copy link
Member

dg commented Sep 14, 2021

Actually, this is a much more complicated task. PHP doesn't have sufficient API to detect if a method is defined in the trait (getMethodDeclaringMethod() just tries to guess) and how is resolved, like:

class Aliased_Talker {
    use A, B {
        B::smallTalk insteadof A;
        A::bigTalk insteadof B;
        B::bigTalk as talk;
    }
}

I'll add switch whether to import traits or not, and leave it up to the user, because it's hard to do it perfectly.

@mikepsinn
Copy link
Contributor Author

@dg Yeah, it's a tough one. The only flaw I observed in my code is that it still imports static properties from the traits. I couldn't really figure out a way around that, yet. But the static properties are easy to delete and at least it doesn't import a million trait functions.

Thanks for adding that option for me and for your amazing work and generosity on this project! :D

@mikepsinn mikepsinn deleted the v3.6 branch September 14, 2021 16:29
dg pushed a commit that referenced this pull request Sep 15, 2021
dg pushed a commit that referenced this pull request Sep 15, 2021
@dg
Copy link
Member

dg commented Sep 15, 2021

That's odd, I tried adding a test to it and the static variables seem to work correctly:

https://github.com/nette/php-generator/blob/master/tests/PhpGenerator/expected/ClassType.from.trait-use.expect#L6

dg pushed a commit that referenced this pull request Sep 15, 2021
dg pushed a commit that referenced this pull request Sep 15, 2021
dg pushed a commit that referenced this pull request Sep 15, 2021
dg pushed a commit that referenced this pull request Sep 15, 2021
dg pushed a commit that referenced this pull request Sep 15, 2021
dg pushed a commit that referenced this pull request Sep 16, 2021
dg pushed a commit that referenced this pull request Sep 16, 2021
dg added a commit that referenced this pull request Sep 17, 2021
dg added a commit that referenced this pull request Sep 17, 2021
dg added a commit that referenced this pull request Sep 18, 2021
dg added a commit that referenced this pull request Sep 18, 2021
dg added a commit that referenced this pull request Sep 18, 2021
dg added a commit that referenced this pull request Sep 18, 2021
dg added a commit that referenced this pull request Sep 18, 2021
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.

2 participants