Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Closes issue #4876 - added support for trait generation and trait scanning #6339

Closed

Conversation

steverhoades
Copy link
Contributor

This is a first attempt at adding generation and scanning support for Traits.

For basic examples see below, for other usage please see ZendTest/Code/Generation/ClassGenerationTest.php

Example 1: Add a trait

$traitGenerator = new TraitGenerator();
$traitGenerator->setName('myTrait');
$traitGenerator->generate();

Outputs:

trait myTrait
{

}

Example 2: Add a trait to a class

$classGenerator = new ClassGenerator();
$classGenerator->setName('myClass');
$classGenerator->addTrait('myTrait');

Outputs:

class myClass
{
    use myTrait;

}

Example 2: Add multiple traits with aliases and overrides

$classGenerator = new ClassGenerator();
$classGenerator->setName('myClass');
$classGenerator->addTraits(array('myTrait', 'hisTrait'));
$classGenerator->addTraitAlias('myTrait::foo', 'newfoo');
$classGenerator->addTraitOverride('hisTrait::bar', 'myTrait');

Outputs:

class myClass
{
    use myTrait, hisTrait {
        myTrait::foo as newfoo;
        hisTrait::bar insteadof myTrait;
    }

}

/*
* @todo test if this works when trait is included or required instead of autoloaded?
*/
$fileName = str_replace("\\", "/", $traitName) .".php";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently the unit test is failing because of this. This is attempting to create a FileScanner instance of the Trait that is being used. If it's not on the include_path it isn't going to work. Not quite sure how to accomplish this at the moment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could do something like this:

$r = new ReflectionClass($traitName);
if (! $r->isTrait()) {
    throw new Exception\RuntimeException('Non-trait detected as a trait: ' . $traitName);
}
$fileName = $r->getFileName();

I'll give that a try shortly to see if it works.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worked!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... this may not be the best approach; the point of the Scanner subcomponent is to allow scanning without reflection. I'm going to leave with my changes for now; we can see if there's a better way to accomplish this later.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please be aware that changing this to not using reflection could be a BC-break depending on how it is changed. Code can depend on the fact that the traits are autoloaded by ReflectionClass.

@Ocramius
Copy link
Member

@steverhoades need to rebase this one

@steverhoades
Copy link
Contributor Author

@Ocramius sorry missed this notification somehow. Will do this as soon as possible.

@steverhoades
Copy link
Contributor Author

@Ocramius I have rebased this commit.

@RSully
Copy link

RSully commented Sep 26, 2014

Any updates on this?

@Ocramius
Copy link
Member

I'm currently reviewing this: will rewrite some bits.

@Ocramius
Copy link
Member

@steverhoades what is your take about using a ValueObject instead of different array formats? The current API is messy mainly because of all the different supported parameter types...

@steverhoades
Copy link
Contributor Author

@Ocramius a ValueObject would make much more sense.

@Ocramius
Copy link
Member

@steverhoades TraitUsageGenerator? Thoughts? :-)

@steverhoades
Copy link
Contributor Author

@Ocramius Works for me.

@Ocramius
Copy link
Member

I still didn't find the time to work on this, but I will have to before 2.4

@samsonasik
Copy link
Contributor

it need rebase

@weierophinney
Copy link
Member

@steverhoades Can you rebase, please?

Also, do you or @Ocramius have time to do the changes for:

  • renaming the generator
  • converting to using a value object

in the next 2 weeks? If not, I can see if I can carve out some time, but I need to know fairly soon so I can determine if this will be part of 2.4.

Thanks!

@steverhoades
Copy link
Contributor Author

@weierophinney My time is going to be tight over the next few weeks. I would like to see this as part of 2.4 though. I'll take another look at whats left and get back to you asap.

…for trait generation, trait scanning and method reflection
@steverhoades
Copy link
Contributor Author

@weierophinney @samsonasik I rebased the commit. Matthew @Ocramius I have not yet had a chance to look at the refactoring. I will do that asap now that the branch has been rebased.

@weierophinney
Copy link
Member

@steverhoades awesome; many thanks in advance!

@steverhoades
Copy link
Contributor Author

@weierophinney @Ocramius I've gone ahead and refactored into a TraitUsageGenerator class. @Ocramius let me know if this is what you had in mind. We can tweak as necessary.

@steverhoades
Copy link
Contributor Author

@Ocramius I think I may have mis-interpreted the direction on this. I think you were thinking more along the lines of addTrait(TraitUsageGenerator $trait). Let me know - I may need to go back to the drawing board on this one. /cc @weierophinney

* @param string|null $useAlias
* @return ClassGenerator
*/
public function addUse($use, $useAlias = null)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was in the middle of commenting that removing this is a BC break, and suggesting to have it proxy to the TraitUsageGenerator... and then saw that you just moved the method to later in the class. :)

weierophinney added a commit that referenced this pull request Mar 17, 2015
Closes issue #4876 - added support for trait generation and trait scanning
weierophinney added a commit that referenced this pull request Mar 17, 2015
@weierophinney
Copy link
Member

Merged to develop for release with 2.4.

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

Successfully merging this pull request may close these issues.

6 participants