This repository has been archived by the owner on Jan 29, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 79
[ZF3] FQCN in parameter type hints #76
Comments
keradus
added a commit
to PHP-CS-Fixer/PHP-CS-Fixer
that referenced
this issue
Mar 20, 2018
…o short version (veewee, keradus) This PR was squashed before being merged into the 2.11-dev branch (closes #3276). Discussion ---------- Transform Fully Qualified parameters and return types to short version This PR removes fully qualified parameters and return types to the short versions when they are being imported / in de same namespace: Result: ```php <?php use Foo\Bar; use Foo\Bar\Baz; class SomeClass { public function doSomething(Bar $foo): Baz { } } ``` Source: ```php <?php use Foo\Bar; use Foo\Bar\Baz; class SomeClass { public function doSomething(\Foo\Bar $foo): \Foo\Bar\Baz { } } ``` In older codebases (early namespaces), this was frequently used. Besides that, it can be used to fix autogenerated code from zend-code (zendframework/zend-code#76) and crappy docblocks in the #3110 PR I am working on: Commits ------- d01075c Transform Fully Qualified parameters and return types to short version
allansun
added a commit
to allansun/kubernetes-php-client
that referenced
this issue
Aug 2, 2018
Fixed: method parameter type incorrectly shown due to zend-code bug zendframework/zend-code#76
This repository has been closed and moved to laminas/laminas-code; a new issue has been opened at laminas/laminas-code#23. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
This issue has been moved from the
zendframework
repository as part of the bug migration program as outlined here - http://framework.zend.com/blog/2016-04-11-issue-closures.htmlOriginal Issue: https://api.github.com/repos/zendframework/zendframework/issues/4149
User: @Ocramius
Created On: 2013-03-30T11:59:09Z
Updated At: 2014-09-28T00:34:50Z
Body
This PR introduces FQCNs for type-hints in generated method signatures. This is a BC break, but it is necessary to use the code generator when the generated code is not placed on a dedicated file.
Before:
After
This PR also takes into account the new PHP 5.4
callable
data type when generating code.Comment
User: @Ocramius
Created On: 2013-04-08T14:28:13Z
Updated At: 2013-04-08T14:28:13Z
Body
@prolic ping? You used Zend\Code quite a bit - is this a huge BC break for you?
Comment
User: @prolic
Created On: 2013-04-08T23:02:49Z
Updated At: 2013-04-08T23:02:49Z
Body
I am okay with these changes. This helps in more situations as I would expect troubles with it.
Comment
User: @weierophinney
Created On: 2013-04-12T16:36:04Z
Updated At: 2013-04-12T16:36:04Z
Body
Would it be possible to make this optional via a flag?
I ask, because our own CS indicates that parameter type hints should not be fully qualified; they should always resolve to current imports. The use case you're specifying here is highly specific -- it's for when classes are not in dedicated files, which, again, was not the original purpose, and goes against our own CS and recommendations.
That said, I can see the rationale for having this, but doing it as an optional behavior makes more sense to me.
Comment
User: @Ocramius
Created On: 2013-04-12T16:38:03Z
Updated At: 2013-04-12T16:38:03Z
Body
I'll think of a way of making this optional. The fact is that this currently makes it impossible to have classes generated via
generate()
without a filegenerator too (for eval'd code)Comment
User: @icywolfy
Created On: 2013-04-16T16:57:20Z
Updated At: 2013-04-16T16:57:20Z
Body
Why would one need to use FQCN when multiple classes are in the same file/eval'd code?
So long as each section is prefixed with a non-global namespace; there's no issue with conflicting uses/namespaces.
And if you need to use the global namespace, then bracketed notation is better.
so
or if you need global support: (this isn't supported in the ClassGenerator)
But, I would personally see the above generated/used rather than giving users the option of using FQCN, since it's still much better to be using Use statements if only to promote better coding habits.
But that said, I have had no issues generating multiple classes into a single cached-file, or dynamic unclusion via eval() at work (hate using eval, but since the temporary classes don't live in the filesystem, can't really auto-load it)
Comment
User: @Ocramius
Created On: 2013-04-16T21:26:17Z
Updated At: 2013-04-16T21:26:17Z
Body
@icywolfy this causes a number of problems atm.
Just as a simple example, take multiple imported classes with the same name.
Using the FQCN is not better from a CS perspective, but honestly, nobody should ever care about generated code CS.
Comment
User: @icywolfy
Created On: 2013-04-16T23:05:33Z
Updated At: 2013-04-16T23:05:33Z
Body
@Ocramius Perhaps, but just don't see the issues with
Or if you need to use a FQCN, explicitly prepend add the '' during generation;
(we just "use" our global and vendor prefixes namespaces)
Though I would love to have it propagate use-aliases set against methods; so that you can define the aliases you use in the method-body and have it resolve up to the class-level and be added to the generator (and throw exception if conflicting use-aliases are used)
But, we here heavily use the generated code for both sub-project generation, and for use of user and type management; User's are generated upon major changes, types are rebuilt/committed to code-base based on the config files.
We do manual changes for people to meet their desired behaviours and mark their classes as manually edited, and then treat them specially until such time that it's properly incorporated into the build/generation process. But since we are actually using\editing the generated code, it's nice to have it be readily readable.
Though fundamentally I'm against BC breaks now that ZF2 is relatively stable in the 2.1.x branch.
(we are operating without access to a database for several of the East-coast data-centres, and this was the solution that was ended up on, it's actually quite nice, if not a bit unconventional. Damned lawyers dictating how things work.)
Comment
User: @Ocramius
Created On: 2013-04-17T07:42:23Z
Updated At: 2013-04-17T07:42:23Z
Body
That's way too complex for no real reason. Having FQCN simply removes all these problems at once. I'll see about the break - for now I had to subclass ALL generators to get rid of the problem, since the current API is unusable.
Comment
User: @ralphschindler
Created On: 2013-06-06T22:48:45Z
Updated At: 2013-06-06T22:48:45Z
Body
@Ocramius after a cursory review, I'm ok with this, do you still feel like this should be merged?
Comment
User: @Ocramius
Created On: 2013-06-06T23:13:21Z
Updated At: 2013-06-06T23:13:46Z
Body
@ralphschindler not sure since it's a bc break. Gimme some more time, otherwise feel free to close this and I'll redo it somehow.
Comment
User: @EvanDotPro
Created On: 2013-10-15T03:05:44Z
Updated At: 2013-10-15T03:05:44Z
Body
@Ocramius ping? Let's either get this merged or refactored. 😄
Comment
User: @Ocramius
Created On: 2014-02-04T12:05:50Z
Updated At: 2014-02-04T12:05:50Z
Body
This cannot land in 2.x - I'll keep my adaptation in my library and the issue open since it's a quite relevant "bug"
Comment
User: @weierophinney
Created On: 2014-03-03T16:31:55Z
Updated At: 2014-03-03T16:31:55Z
Body
Marking for 3.0.0
The text was updated successfully, but these errors were encountered: