Issue #4669 - Class generator should return uses from file generator #5152

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

robertboloc commented Sep 21, 2013

Propagate file generator uses to class generator. Fixes issue #4669.

Member

Maks3w commented Oct 3, 2013

Seems correct but I've a question. You are adding all uses to all classes but exists the possibility that not all uses are used by all classes.

Could you add only the necessary uses for each class extracted?

Contributor

robertboloc commented Oct 3, 2013

I thought of that but I don't see how to separate the uses by class as I can not see where the instances are created. But from what I understand if someoane defines two classes in the same file and sets up a uses definition is because both classes require it, if not better use two different files right?

It's tricky, but if you have any suggestion I am open to ideas, thanks!

@Maks3w Maks3w commented on an outdated diff Oct 3, 2013

library/Zend/Code/Generator/FileGenerator.php
foreach ($fileReflection->getClasses() as $class) {
$phpClass = ClassGenerator::fromReflection($class);
$phpClass->setContainingFileGenerator($file);
+ if ($uses) {
@Maks3w

Maks3w Oct 3, 2013

Member

Due getUses definition and implementation $uses will be always an array so there is no need of this if

Member

Maks3w commented Oct 20, 2013

@robertboloc The same algos used for detect unused use statements by CS fixers can be used here.

https://github.com/fabpot/PHP-CS-Fixer/blob/master/Symfony/CS/Fixer/UnusedUseStatementsFixer.php

Owner

weierophinney commented Oct 23, 2013

@Maks3w that algorithm will not work here, as it's operating on the entire file, and not the individual classes in the file.

Considering the most common use case is one class per file, I think the solution as proposed here is fine.

@weierophinney weierophinney added a commit that referenced this pull request Oct 23, 2013

@weierophinney weierophinney Merge pull request #5152 from robertboloc/#4669
Issue #4669 - Class generator should return uses from file generator
ec07e12

@weierophinney weierophinney added a commit that referenced this pull request Oct 23, 2013

@weierophinney weierophinney Merge branch 'hotfix/5152' into develop
Forward port #5152
ad2e1b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment