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

[BUG]: PHPStan static analysis ends up with internal errors #16023

Closed
kristytoman opened this issue Jul 14, 2022 · 8 comments · Fixed by #16066
Closed

[BUG]: PHPStan static analysis ends up with internal errors #16023

kristytoman opened this issue Jul 14, 2022 · 8 comments · Fixed by #16066
Assignees
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report external dependency This issue depends on external issue to be resolved.

Comments

@kristytoman
Copy link

Describe the bug
We've upgraded to phalcon 5.0.0-RC3 and PHP8.1 on our projects and we have troubles with running static analysis on it. Currently found problems:

  • Files using Phalcon\Events\Event fails with
Uncaught PHPStan\BetterReflection\SourceLocator\Ast\Exception\ParseToAstFailure: AST failed to parse in located source (first 20 characters: <?php

namespace Pha)`
  • Files using Phalcon\Mvc\Model ends up with Child process error (exit code 139): Segmentation fault (core dumped)

Unfortunately I couldn't get anything more specific than this message. I wrote an issue about the first one to PHPStan and I'm waiting for a response.

To Reproduce
We have not found a problem with Phalcon other than the static analysis file

  1. We use dockerfile with
    • PHP version = 8.1
    • Zephir parser version = 1.5.0
    • Zephir version = 0.16.0
    • Phalcon version = 5.0.0RC3
    • Operating system = Debian Bullseye
    • server = Apache2
  2. In composer.json we have
    • phalcon/ide-stubs version = 5.0.0-RC1
    • phpstan/phpstan = 1.8.1

all you need is just mention the class in the analysed class and then when you run vendor/bin/phpstan analyse ./sourcepath it fails

Expected behavior
PHPStan analysis ends up with zero internal errors caused by using Phalcon

@kristytoman kristytoman added bug A bug report status: unverified Unverified labels Jul 14, 2022
@Jeckerson Jeckerson self-assigned this Jul 14, 2022
@Jeckerson Jeckerson added status: high High 5.0 The issues we want to solve in the 5.0 release and removed status: unverified Unverified labels Jul 14, 2022
@Jeckerson
Copy link
Member

After several tests with different versions of PHP, Phalcon and PHPStan. It seems that error is not in Phalcon, but in PHPStan, since version 1.7.0 (https://github.com/phpstan/phpstan/releases/tag/1.7.0).

image

According to this blog post - https://phpstan.org/blog/zero-config-analysis-with-static-reflection, since version 1.7.0, since there it uses AST tree reflection, which might have some internal problems while reading external package (Phalcon).

@Jeckerson
Copy link
Member

Related phpstan/phpstan#7590

@ondrejmirtes
Copy link

My personal guess is that Phalcon uses some method name like throw - something that wouldn't be possible to parse and write in userland, but is possible to declare in an extension.

I've added a bit more debugging capability to PHPStan so it should tell us what's the problem.

@ondrejmirtes
Copy link

The root issue here is that Phalcon\Events\Event has some weird corrupted info in reflection.

The getSource() method reports return type ((void*)0) (ReflectionNamedType::getName()) which isn't valid. Please fix that in the extension. Thanks.

@Jeckerson
Copy link
Member

Please fix that in the extension.

Thank you! I'll check it.

@Jeckerson Jeckerson added external dependency This issue depends on external issue to be resolved. and removed external dependency This issue depends on external issue to be resolved. labels Jul 17, 2022
Jeckerson added a commit that referenced this issue Aug 22, 2022
Jeckerson added a commit that referenced this issue Aug 22, 2022
Jeckerson added a commit that referenced this issue Aug 22, 2022
Jeckerson added a commit that referenced this issue Aug 22, 2022
@Jeckerson Jeckerson linked a pull request Aug 22, 2022 that will close this issue
5 tasks
Jeckerson added a commit that referenced this issue Aug 22, 2022
Jeckerson added a commit that referenced this issue Aug 22, 2022
Jeckerson added a commit that referenced this issue Aug 22, 2022
@Jeckerson Jeckerson added this to Implemented in Phalcon Roadmap Aug 23, 2022
@Jeckerson
Copy link
Member

$stubber = new \Roave\BetterReflection\SourceLocator\SourceStubber\ReflectionSourceStubber();
$data = $stubber->generateClassStub(\Phalcon\Events\Event::class);
echo $data->getStub();

Now it generates correct stubs:

<?php

namespace Phalcon\Events;

class Event implements \Phalcon\Events\EventInterface
{
    protected $cancelable = null;
    protected $data = null;
    protected $source = null;
    protected $stopped = false;
    protected $type = null;
    public function getData()
    {
    }
    public function getType() : string
    {
    }
    public function __construct(string $type, $source, $data, bool $cancelable)
    {
    }
    public function isCancelable() : bool
    {
    }
    public function isStopped() : bool
    {
    }
    public function getSource() : ?object
    {
    }
    public function setData($data) : \Phalcon\Events\EventInterface
    {
    }
    public function setType(string $type) : \Phalcon\Events\EventInterface
    {
    }
    public function stop() : \Phalcon\Events\EventInterface
    {
    }
}

@niden
Copy link
Sponsor Member

niden commented Aug 23, 2022

Resolved in #16066

@niden niden closed this as completed Aug 23, 2022
@Jeckerson
Copy link
Member

Released in v5.0.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
5.0 The issues we want to solve in the 5.0 release bug A bug report external dependency This issue depends on external issue to be resolved.
Projects
Archived in project
Phalcon Roadmap
  
Implemented
Development

Successfully merging a pull request may close this issue.

4 participants