Skip to content

Add traits and don't import their methods - #89

Merged
dg merged 1 commit into
nette:v3.6from
mikepsinn:v3.6
Sep 14, 2021
Merged

dg merged 1 commit into
nette:v3.6from
mikepsinn:v3.6

Conversation

@mikepsinn

Copy link
Copy Markdown
Contributor

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

@dg

dg commented Sep 14, 2021

Copy link
Copy Markdown
Member

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
Copy Markdown
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

dg commented Sep 15, 2021

Copy link
Copy Markdown
Member

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