Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alias in use statement not working in Stubs #1986

Closed
ruudboon opened this issue Oct 26, 2019 · 6 comments
Assignees
Labels
bug

Comments

@ruudboon
Copy link
Member

@ruudboon ruudboon commented Oct 26, 2019

When generating stubs for the following Zephir code.

namespace Phalcon\Mvc\Model;
use Phalcon\Events\ManagerInterface as EventsManagerInterface;

class Manager implements ManagerInterface, InjectionAwareInterface, EventsAwareInterface
{
    public function getEventsManager() -> <EventsManagerInterface>
    {
        return this->eventsManager;
    }
}

this will result in the following (incorrect) php

<?php

namespace Phalcon\Mvc\Model;

use Phalcon\Events\ManagerInterface;

class Manager implements \Phalcon\Mvc\Model\ManagerInterface, \Phalcon\Di\InjectionAwareInterface, \Phalcon\Events\EventsAwareInterface
{
    public function getEventsManager(): EventsManagerInterface
    {
    }
}
@sergeyklay sergeyklay added bug and removed bug labels Oct 26, 2019
@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Oct 26, 2019

@ruudboon Strictly speaking, this is absolutely correct PHP code :) Just redundant use statement. We're not optimize imports. There are a lot of tools (e.g. php-cs-fixer, phpcs) which can do this task better. In my opinion it would be wasteful to duplicate their functionality. On the other hand, I think redundant import isn't a problem at all. This import doesn't affect the IDE's possibility to auto-complete or/and validate the code, isn't?

@sergeyklay sergeyklay added the bug label Oct 26, 2019
@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Oct 26, 2019

Ah, I see. Expected result is:

<?php

namespace Phalcon\Mvc\Model;

use Phalcon\Events\ManagerInterface;

class Manager implements \Phalcon\Mvc\Model\ManagerInterface, \Phalcon\Di\InjectionAwareInterface, \Phalcon\Events\EventsAwareInterface
{
-    public function getEventsManager(): EventsManagerInterface
+    public function getEventsManager(): ManagerInterface
    {
    }
}
@ruudboon

This comment has been minimized.

Copy link
Member Author

@ruudboon ruudboon commented Oct 26, 2019

Yes or

<?php

namespace Phalcon\Mvc\Model;

- use Phalcon\Events\ManagerInterface;

class Manager implements \Phalcon\Mvc\Model\ManagerInterface, \Phalcon\Di\InjectionAwareInterface, \Phalcon\Events\EventsAwareInterface
{
-    public function getEventsManager(): EventsManagerInterface
+    public function getEventsManager(): \Phalcon\Events\ManagerInterface
    {
    }
}
@AlexNDRmac

This comment has been minimized.

Copy link
Member

@AlexNDRmac AlexNDRmac commented Nov 1, 2019

@ruudboon could you pls check fix for Phalcon stubs generation?

@sergeyklay sergeyklay closed this in 3b7b789 Nov 1, 2019
@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Nov 1, 2019

Fixed in the development branch. If there are bugs, we can follow up in separate issues. Thank you for the bug report.

@ruudboon

This comment has been minimized.

Copy link
Member Author

@ruudboon ruudboon commented Nov 1, 2019

@AlexNDRmac Looks good! Seeing the alias now. thnx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.