Allow to define if the type is an alias or not.

[vonglasow]

When a use statements is declared we should be able to use alias as type
for parameters. This patch keep the current behavior and add a way to
define if the parameter type is an alias or not.

[postalservice14]

Instead of mixed, you could say array|string. That way we know exactly what the function is designed to consume

[Ocramius]

I wouldn't allow array. Pass in a TypeGenerator if needed: arrays are squishy. Let's avoid stringly-typed and array-typed code ;-)

[weierophinney]

Seconding @Ocramius ; looking through the method body, this would be far simpler if it accepts a type instance.

[weierophinney]

Maybe an "AliasedType", with getOriginalType() and getAlias() methods.

[vonglasow]

@Ocramius Hello, I have made some modification regarding your feedback. Feel free for any more feedback.

[vonglasow]

@Ocramius Any news ?

[basz]

Feedback regarding the argument of the ParameterGenerator::setType method hasn't been addressed.

array|string vs TypeGenerator|string 

Further complicated by weierophinney's remarks

I think I would go for a string or TypeGenerator argument

$parameterGenerator->setType(TypeGenerator::fromTypeString('Foo', true))

Additionally a test for the happy path and exception path should be added to 'test/Generator/ParameterGeneratorTest.php' .

[vonglasow]

@basz thanks for your feedback! Is it really necessary to keep the string as argument of the ParameterGenerator::setType method. As @weierophinney said it will be more easy to pass only TypeGenerator object. I have refactored the code to use TypeGenerator class and string.

[basz]

I would say that would be better but is not possible without Breaking Compatibility.

[vonglasow]

Hello, is there anyone who can tell me what is missing for this PR ? Thank you.

[Ocramius]

I just went through this again, and I can't find a reason for the patch. What is the reasoning behind it? At least in generators, aliasing is actively harmful, and only causes confusion.

@vonglasow where is this needed, exactly?

[vonglasow]

@Ocramius hello,

The goal was defined in this issue #37

to be short currently it's always a FQCN as type parameter.

use Foo\Bar\SomeClass;

class GeneratedClass
{
    public function generatedMethod(\Foo\Bar\SomeClass $instance) {
    }
}

and this PR allow to create code with using the implicit alias from use.

use Foo\Bar\SomeClass;

class GeneratedClass
{
    public function generatedMethod(SomeClass $instance) {
    }
}

[Ocramius]

Yes, that's what I don't understand: never had a need for aliasing in code
generators, and it always caused issues (when allowed), since the reduced
strictness produced invalid code on multiple occasions.
This class specifically denies usage of non-absolute FQCN by design.

If the final aim is to produce a "code beautifier, I would do so by
replacing AST nodes on an already dumped piece of generated code...

On 20 Sep 2016 18:52, "vonglasow" notifications@github.com wrote:

@Ocramius https://github.com/Ocramius hello,

The goal was defined in this issue #37
#37

to be short currently it's always a FQDN as type parameter.

use Foo\Bar\SomeClass;class GeneratedClass{ public function generatedMethod(\Foo\Bar\SomeClass $instance) { }}

and this PR allow to create code with using the implicit alias from use.

use Foo\Bar\SomeClass;class GeneratedClass{ public function generatedMethod(SomeClass $instance) { }}

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#40 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAJakBGrEeGRX6f-ldehGWHtRjyyD3iLks5qsA8_gaJpZM4HQ3Gj
.

[vonglasow]

Hello, sorry for the delay a lot of work.

So, I don't understand your point. You mean you can arrived to the same results without allowing to use alias? The final aim for me was to produce a beautifier code but I don't know if it was the same goal for @postalservice14.

If you can do it without using alias you can replace this PR by your own. But if the goal of @postalservice14 is to use alias as he wants, I think we need provide a way to do it but as you explain make it explicitly as not recommended.

[Ocramius]

The final aim for me was to produce a beautifier code

That's what this component is NOT for though. We actually moved to FQCNs-only because it was really messy to deal with aliases, since no contextual information is given to a type otherwise.

make it explicitly as not recommended.

That's a good suggestion, but I'd still write a separate AST-based tool for transforming equivalent code then...

[vonglasow]

Hello, what about this PR ? What do you need more ?

[Ocramius]

To clarify, I'll put this in pseudo-code.

Let's say we generate a class like following:

namespace Foo;

use Bar\Baz;

class Taz
{
    public function tab(Baz $baz) {
    }
}

Now, Foo\Taz#tab() has a parameter that is imported.

From a logical perspective, the generators used to create such a file are (pseudo-code - I don't remember the actual classes involved, just the rough structure):

 * FileGenerator
    * TypeGenerator (for the namespace)
 * ClassGenerator
    * MethodGenerator
        * ParameterGenerator
            * TypeGenerator (for the parameter type)

Now, the state of the TypeGenerator passed to the ParameterGenerator is strictly coupled with the state of passed to the `FileGenerator.

This is a mess to deal with, especially if your code generation logic is local to a single method, and has no clue about the outer scope.

This is the TL;DR why imported symbols in a code generator are messy as heck, and IMO shouldn't land in zend-code anymore (they were actually removed)

[Ocramius]

I'm leaving this open to allow you to counter my argument, but this will likely not be merged, sorry :-(

[vonglasow]

@Ocramius Ok, got your point. Don't be sorry, I understand ;) Thanks to take time to explain.