Methods from installed packages show undefinedSymbol error even in 1.3.1

[thinkstudeo]

Describe the bug When using a method on a class from any composer package Undefined symbol error is shown.

To Reproduce I have pulled in PaypalCheckoutSdk v1.0.1 and have a class as following

<?php

namespace Billing\Paypal;

use Billing\Paypal\Client;
use PayPalCheckoutSdk\Orders\OrdersCreateRequest;

class Payment
{

    /**
     * Create a new instance of Payment
     */
    public function __construct()
    {
        $this->client = Client::instance();
    }

    /**
     * Create a new Paypal order for the given payload
     *
     * @param array $payload
     */
    public static function create(array $payload)
    {        
        $request = new OrdersCreateRequest;
        $request->body = json_encode($payload);
        try {
            $response = (new static)->client->execute($request);
            return $response->result;
        } catch(\Exception $e) {
            return response()->json(['message' => "Paypal error: ". $e->getMessage()], 409);
        }
}

And the client class is as under

<?php

namespace Billing\Paypal;

use Illuminate\Support\Arr;
use PayPalCheckoutSdk\Core\PayPalHttpClient;
use Billing\Exceptions\InvalidGateway;

class Client
{
    /**
     * Get a new Paypal Http Client instance,
     *
     * @return \PaypalCheckoutSdk\Core\PayPalHttpClient
     */
    public static function instance()
    {
        return new PayPalHttpClient(self::getEnvironment());
    }

    /**
     * Get the Paypal environment based on the configuration values.
     *
     * @return \PaypalCheckoutSdk\Core\SandboxEnvironment|\PaypalCheckoutSdk\Core\ProductionEnvironment
     */
    protected static function getEnvironment()
    {
        $config = config('billing.gateway.paypal');
        if (empty($config)) {
            throw InvalidGateway::nonRegistered('Paypal');
        }

        if (empty(Arr::get($config, 'account')) || empty(Arr::get($config, 'account.key') || empty(Arr::get($config, 'account.secret')))) {
            throw InvalidGateway::credentialsNotConfigured('Paypal');
        }

        $environmentClass = config('cashier.gateway.paypal.account.mode', 'sandbox') === 'sandbox'
            ? \PayPalCheckoutSdk\Core\SandboxEnvironment::class
            : \PayPalCheckoutSdk\Core\ProductionEnvironment::class;

        return new $environmentClass(Arr::get($config, 'account.key'), Arr::get($config, 'account.secret'));
    }
}

Same kind of errors also show up for Laravel eloquent relations like hasMany(), belongsTo() when they are defined in a trait which is then consumed by an eloquent model class - as pointed in https://github.com/bmewburn/vscode-intelephense/issues/780#issuecomment-561545145

Expected behavior Don't expect to get error regarding undefined symbol for PayPalHttpClient\Client::execute() in the line (new static)->client->execute($request) in the Payment.php class.

When I revert back to Intelephense v 1.2 it works as expected - no Undefined symbols error. But then I can't get support for PHP7.4

Platform and version OS and Intelephense version. macOS: 10.15.1 Intelephense version: 1.3.1

[andrewmclagan]

Also seeing this in 1.3.1

Closing the file and reopening it resolves the issue.

[bmewburn]

Thanks @andrewmclagan , does the problem return?

[andrewmclagan]

Not that i have noticed thus far.

[thinkstudeo]

No luck reopening the files, the issue still persists as described in the question - when the methods are called on classes which are from a package in vendor directory as well as Traits which use laravel eloquent relations methods.

[bmewburn]

I'm unable to reproduce this. I don't get any error for $response = (new static)->client->execute($request); . Laravel methods belongsTo etc also are working fine for me. Can you try reindexing the workspace, ctrl + shift + p, Index workspace? Are you excluding any files in the intelephense.files.exclude setting?

[thinkstudeo]

@bmewburn thanks for your attention. I tried reindexing the workspace, some of the errors went away but the errors for eloquent relations methods in traits still exist.

Also the errors of (new static)->client->execute() still occurs. But if I use Client::execute() the errors does not occur.

I have a workspace in vscode and the errors are occurring in a package which is not in the root folder/vendor folder of the laravel project. It's in a different location and I have the package folder added to the workspace containing the laravel app.

With same setup I wasn't getting any errors in v1.2.3. The version was automatically updated to 1.3.2 today.

And no I am not excluding any files in intelephense.files.exclude settings. The relationship methods like belongsTo etc are working fine in model classes they only show errors when used in trait.

[KorzunKarl]

The same problem, the user class uses trait, for example, phones, and if you specify relationships in the trait phones, then we get "Undefined symbol 'morphMany'". Re-indexing did not help. The problem appeared with 1.3

[bmewburn]

@KorzunKarl I don't consider your example a bug. What happens if a consumer of the trait does not implement morphMany ? Shouldn't you make sure they do by including an abstract method declaration in HasPhones ? https://www.php.net/manual/en/language.oop5.traits.php#language.oop5.traits.abstract

[KorzunKarl]

@bmewburn In my case, this is not what you said. Under equal conditions, other applications or IDE, in this section of code do not see problems and it works, this annoying underlining appeared only after updating to 1.3 Anyway Thanks for the clarification. Or make the phones method in the user's class - and specify the phones method abstractly in the HasPhones trait. Yes, the syntax error goes away - but if the model does not have this method, nothing will work and even the error will not be traced. But then in each model it will be necessary to do this, I'm not sure that this is necessary. An unsuccessful update before that everything was beautiful and it worked ... Maybe I was in a hurry with the purchase of the paid version :(, I'm joking, I believe this point will be fixed, I just can not explain otherwise unfortunately. Here is an example trait from the author of the laravel framework - in it your software also sees exactly the same error

[KapitanOczywisty]

Lalavel is abusing every bad practice known in php, I wouldn't take it as an example.

If you want to do it right, abstraction in trait is way to go. php.net :link: I'm not sure why you are reluctant to use it. In the current state Intelephense will not warn if abstraction is not implemented, but I hope will be soon.

What should be noted, one trait can fulfill abstraction from any other: 3v4l.org :link:

trait Hello {
    public function getHello(){
        return 'Hello';
    }
    public function sayHelloWorld1() {
        echo "{$this->getWorld()} {$this->getHello()}\n";
    }
    abstract public function getWorld();
}
trait World {
    public function getWorld(){
        return 'World';
    }
    public function sayHelloWorld2() {
        echo "{$this->getWorld()} {$this->getHello()}\n";
    }
    abstract public function getHello();
}
class MyHelloWorld {
    use Hello, World;
}
(new MyHelloWorld)->sayHelloWorld();

You can go even further and use trait to enforce other traits to implement methods used in trait 3v4l.org

trait AbstractWorld {
    abstract public function getWorld();
}
trait Hello {
    use AbstractWorld;
    public function getHello() {
        return 'Hello';
    }
    public function sayHelloWorld() {
        echo "{$this->getWorld()} {$this->getHello()}\n";
    }
}
trait World {
    public function getWorld() {
        return 'World';
    }
}
class MyHelloWorld {
    use Hello, World;
}
(new MyHelloWorld)->sayHelloWorld();

[bmewburn]

My argument still stands even if it's Laravel. Though, considering this might be widespread and this style is used in a popular framework I've relaxed the checks here in 1.3.3. Stricter check will be available in #830 . I'm unsure if this resolves the original issue here. I'll leave open for now.

[thinkstudeo]

@bmewburn Upgraded to 1.3.3 and the error of (new static)->client->execute() still remains. The errors for traits having eloquent relations method is now gone.

I read the comment by @KapitanOczywisty as well as your comment.

At first go it seems logical to abstract the methods used within trait so that the consumer is forced to have an implementation. But I am a bit confused if it is the best way to go with Laravel.

Cause say in my case or as in case of trait in @KorzunKarl 's use case, we will then need to copy the signature of the Illuminate\Database\Eloquent\Relations\HanMany or whatever other relation method is included in the trait from laravel.

Is that what you recommend? And also are there any other benefits to using this approach other than forcing the consumer to have an implementation?

BTW thanks for including flexible strict check

[KorzunKarl]

I agree with @thinkstudeo , updaiting to 1.3.3 The errors is now gone. Thanks...

[KapitanOczywisty]

Is that what you recommend? And also are there any other benefits to using this approach other than forcing the consumer to have an implementation?

In languages with strict rules like C# you're mostly dealing with logical errors and optimization. However PHP, to not scare new users, allows for nasty things. These things may work just fine for a while and show code errors only after few years, when correct planet alignment causes one variable to have unexpected value. Even when code is perfectly fine today, someone may change function signature in the future, accidentally breaking code without noticing. It's for sure more overhead to add all these abstractions, but this prevents many unexpected things from happening.

Using Laravel is personal preference, but It's weird that people want to enforce bad code support in editors rather than push Laravel to start following oop rules.

[thinkstudeo]

Even when code is perfectly fine today, someone may change function signature in the future, accidentally breaking code without noticing. It's for sure more overhead to add all these abstractions, but this prevents many unexpected things from happening.

While this is true for the code written specific to an app, that change in function signature in future may break code without noticing easily.

But with a framework like Laravel (any framework for that matter), when using the functions exposed by the framework in trait doesn't pose similar vulnerability/risk. Because we are relying on the function exposed by the framework and any framework will have release notes cautioning about breaking changes.

And the risk from breaking changes (change in function signature) only comes in question when we decide to upgrade the framework version which has breaking changes.

Help me correct if my understanding is wrong

Using Laravel is personal preference, but It's weird that people want to enforce bad code support in editors rather than push Laravel to start following oop rules.

I guess this will remain a differing/conflicting opinion between php land and other strict languages like C# or Java

[KapitanOczywisty]

I guess this will remain a differing/conflicting opinion between php land and other strict languages like C# or Java

There is that joke "PHP programmer" :) I know that I'm in minority, in general you are probably right, which is sad. To be clear didn't came from C#, I was just fortunate to work with C#for a while, and I highly encouraging to give it a try.

[thinkstudeo]

@KapitanOczywisty I get the point about declaring abstract methods in trait to enforce consumer has an implementation - will help avoid nasty errors due to accidental/intentional function signature change or consumer missing the implementation - appreciate your detailed explanation and assertion by @bmewburn

Am only trying to understand that when using a function exposed by some framework - whose signature won't change without notice (release notes for breaking changes) what benefits do I get by copying the function signature from the framework code for the abstract declaration.

[thinkstudeo]

@bmewburn Regarding the (new static)->client->execute() error, let me draw your attention once again to my setup. The code is in an independent local package which is in a folder different from the laravel app root. I have added the local package folder to the vscode workspace along with the laravel app.

The local package is pulling Paypal sdk as composer dependency. And the code generating the error is in a class in the local independent package.

With the same setup, I wasn't getting undefined symbol error in v1.2.3. Started getting the error only after update to v1.3.x.

Can you suggest something that I can try ( already tried reindexing workspace) to remove the error?

If I new up the Client within the method and then use $client->execute($request) instead of (new static)->client->execute($request) then I don't get any error

    /**
     * Id for the object
     *
     * @var string
     */
    protected $id;

    /**
     * Paypal api response result.
     *
     * @var mixed
     */
    public $response;

    /**
     * Paypal HttpClient
     *
     * @var \Thinkstudeo\Cashier\Paypal\Client
     */
    protected $client;

    /**
     * Create a new PaymentIntent instance.
     *
     * @param string $id
     */
    public function __construct($id = null)
    {
        $this->id       = $id;
        $this->client   = Client::instance();
        $this->response = new \stdClass;
    }

    /**
     * Make a createOrder request to Paypal api.
     *
     * @param array $payload
     * @return $this
     */
    public static function create($payload)
    {
        $request = new OrdersCreateRequest;
        // static::addHeader($request, 'Paypal-Request-Id', $payload['uid']);
        $request->body = json_encode($payload);
        try {
            //\PaypalHttp\HttpResponse $response
            $response = (new static)->client->execute($request);    //Shows undefined method execute

            $client  = Client::instance();
            $response = $client->execute($request); //No errors

            return static::refreshFrom($response->result)->serialize();
        } catch (\Exception $e) {
            throw InvalidPaypalOrder::failedCreation($e);
        }
    }

Any idea what am I missing or doing wrong?

[KapitanOczywisty]

Am only trying to understand that when using a function exposed by some framework - whose signature won't change without notice (release notes for breaking changes) what benefits do I get by copying the function signature from the framework code for the abstract declaration.

In that case signature is not important. However for example morphMany comes in trait HasRelationships, which have to be added by user, maybe not in that exact case, but I can imagine situation when user forgot to add HasRelationships e.g. when copying code. If you can add trait like RequireHasRelationship with abstractions it's rather good deal to sleep better.

AFIK static is broken in intelephense - related #794

[thinkstudeo]

In that case signature is not important. However for example morphMany comes in trait HasRelationships, which have to be added by user, maybe not in that exact case, but I can imagine situation when user forgot to add HasRelationships e.g. when copying code. If you can add trait like RequireHasRelationship with abstractions it's rather good deal to sleep better.

Okay thanks. I see the point becomes more relevant especially when working in a team. And would be kind of difficult to get errors due to missing implementation.

AFIK static is broken in intelephense - related #794

Thanks. I was going through the code line by line trying to figure out why it shows error when using (new static)->client->execute() and no errors when $client = Client::instance(); $client->execute()

BTW the error changed from Undefined symbol to Undefined method in 1.3.3

Any idea by when static might get fixed?

[KapitanOczywisty]

BTW the error changed from Undefined symbol to Undefined method in 1.3.3

Because of that #824

static might be tricky to fix. I think this code may work when you assign it to variable first and then use.

[thinkstudeo]

static might be tricky to fix. I think this code may work when you assign it to variable first and then use.

Yes it works when it is assigned to a variable. $client = Client::instance() and then $client->execute() doesn't show any error. But I have 4 functions in the class where I require the client so will have to repeat assigning to variable in all 4 functions :(

Thanks

[KapitanOczywisty]

I've made simple test code, but I'm unable to reproduce error, what I'm missing?

class Foo {
    public function sayFoo(){
        echo "Foo";
    }
}
class Bar {
    /** @var Foo */
    protected $foo;
    public function __construct()
    {
        $this->foo = new Foo;
    }
    public static function create(){
        (new static)->foo->sayFoo();
    }
}

Bar::create();

In your code \Thinkstudeo\Cashier\Paypal\Client extends \PaypalCheckoutSdk\Core\PayPalHttpClient, right?

Any of these works? I'm not sure what part looses type.

$client = (new static)->client;
$client->execute();
// or
$me = new static;
$me->client->execute();

[thinkstudeo]

Sorry for the delay in response.

Nope none of them work - both shows Undefined method error

$client = (new static)->client;
$response = $client->execute($request)     //Undefined method error

$instance = new static;
$instance->client->execute($request)    //Also gives Undefined method error

No my code for Client is

<?php

namespace Thinkstudeo\Cashier\Paypal;

use Illuminate\Support\Arr;
use PayPalCheckoutSdk\Core\PayPalHttpClient;
use Thinkstudeo\Cashier\Exceptions\InvalidGateway;

class Client
{
    /**
     * Get a new Paypal Http Client instance,
     *
     * @return \PaypalCheckoutSdk\Core\PayPalHttpClient
     */
    public static function instance()
    {
        return new PayPalHttpClient(self::getEnvironment());
    }

    /**
     * Get the Paypal environment based on the configuration values.
     *
     * @return \PaypalCheckoutSdk\Core\SandboxEnvironment|\PaypalCheckoutSdk\Core\ProductionEnvironment
     */
    protected static function getEnvironment()
    {
        $config = config('cashier.gateway.paypal');
        if (empty($config)) {
            throw InvalidGateway::nonRegistered('Paypal');
        }

        if (empty(Arr::get($config, 'account')) || empty(Arr::get($config, 'account.key') || empty(Arr::get($config, 'account.secret')))) {
            throw InvalidGateway::credentialsNotConfigured('Paypal');
        }

        $environmentClass = config('cashier.gateway.paypal.account.mode', 'sandbox') === 'sandbox'
            ? \PayPalCheckoutSdk\Core\SandboxEnvironment::class
            : \PayPalCheckoutSdk\Core\ProductionEnvironment::class;

        return new $environmentClass(Arr::get($config, 'account.key'), Arr::get($config, 'account.secret'));
    }
}

And then in other class

<?php

namespace Thinkstudeo\Cashier\Paypal;

use Thinkstudeo\Support\Serializable;
use PayPalCheckoutSdk\Orders\OrdersGetRequest;
use Thinkstudeo\Cashier\Paypal\Payload\Payload;
use PayPalCheckoutSdk\Orders\OrdersCreateRequest;
use PayPalCheckoutSdk\Orders\OrdersCaptureRequest;
use Thinkstudeo\Cashier\Exceptions\InvalidPaypalOrder;

class PaymentIntent
{
    use Serializable;

    /**
     * These constants are possible representations of the status field.
     */
    const STATUS_CREATED   = 'CREATED';
    const STATUS_SAVED     = 'SAVED';
    const STATUS_APPROVED  = 'APPROVED';
    const STATUS_VOIDED    = 'VOIDED';
    const STATUS_COMPLETED = 'COMPLETED';

    /**
     * Id for the object
     *
     * @var string
     */
    protected $id;

    /**
     * Paypal api response result.
     *
     * @var mixed
     */
    public $response;

    /**
     * Paypal HttpClient
     *
     * @var \Thinkstudeo\Cashier\Paypal\Client
     */
    protected $client;

    /**
     * Create a new PaymentIntent instance.
     *
     * @param string $id
     */
    public function __construct($id = null)
    {
        $this->id       = $id;
        $this->client   = Client::instance();
        $this->response = new \stdClass;
    }

    /**
     * Make a createOrder request to Paypal api.
     *
     * @param array $payload
     * @return $this
     */
    public static function create($payload)
    {
        $request = new OrdersCreateRequest;
        // static::addHeader($request, 'Paypal-Request-Id', $payload['uid']);
        $request->body = json_encode($payload);
        try {
            //\PaypalHttp\HttpResponse $response
            $response = (new static)->client->execute($request);    //Shows undefined method execute

            $instance = new static;
            $response = $instance->client->execute($response);    //Shows undefined method execute

            $client   = (new static)->client;
            $response = $client->execute($request);    //Shows undefined method execute

            $client   = Client::instance();
            $response = $client->execute($request);    //No errors

            return static::refreshFrom($response->result)->serialize();
        } catch (\Exception $e) {
            throw InvalidPaypalOrder::failedCreation($e);
        }
    }

    /**
     * Make a captureOrder request to Paypal api.
     *
     * @param string $orderId
     * @return $this
     */
    public static function capture($orderId)
    {
        $request = new OrdersCaptureRequest($orderId);

        $request->prefer('return=representation');

        try {
            $response = (new static)->client->execute($request);

            return static::refreshFrom($response->result);
        } catch (\Exception $e) {
            throw InvalidPaypalOrder::failedCapture($e);
        }
    }

    /**
     * Make a getOrder request to the Paypal api for the given orderId.
     *
     * @param string $orderId
     * @return $this
     */
    public static function retrieve($orderId)
    {
        $request = new OrdersGetRequest($orderId);
        static::addHeader($request, 'prefer', 'return=representation');

        try {
            $response = (new static)->client->execute($request);
            if ($response->result->status === 'COMPLETED') {
                //Payment has been authorized and captured

                return static::refreshFrom($response->result);
            }
        } catch (\Exception $e) {
            throw InvalidPaypalOrder::failedGet($e);
        }
    }

    /**
     * Populate the instance properties with the api response.
     *
     * @param \PaypalHttp\HttpResponse $result
     * @return $this
     */
    public static function refreshFrom($result)
    {
        // die(var_dump('refreshFrom PaypalPaymentIntent', $result));
        $instance = new static;

        $instance->id = $result->id;

        foreach ($result as $key => $value) {
            if ($key !== 'id') {
                $instance->response->{$key} = $value;
            }
        }
        // die(var_dump('refreshFrom PaypalPaymentIntent serialize', $instance->serialize()));

        return $instance;
    }

    /**
     * Add header to the Paypal api request object.
     *
     * @param \PaypalHttp\HttpRequest $request
     * @param string $key
     * @param string $value
     * @return \PaypalHttp\HttpRequest
     */
    protected static function addHeader($request, $key, $value)
    {
        return $request->headers[$key] = $value;
    }

    public function __get($key)
    {
        return $this->response->{$key};
    }
}

[KapitanOczywisty]

Nope none of them work - both shows Undefined method error

$client = (new static)->client;
$response = $client->execute($request)     //Undefined method error

$instance = new static;
$instance->client->execute($request)    //Also gives Undefined method error

What do you have on hover with $client and $instance?

[thinkstudeo]

Hover on $instance is static $instance and

hover on $client is \Thinkstudeo\Cashier\Paypal\Client $cashier while it should be \PaypalCheckoutSdk\Core\PayPalHttpClient

When I hover on $this->client in the constructor it shows the correct reference \PaypalCheckoutSdk\Core\PayPalHttpClient

[KapitanOczywisty]

Since \Thinkstudeo\Cashier\Paypal\Client is not extension of \PaypalCheckoutSdk\Core\PayPalHttpClient this part seems to have wrong @var. You're trying to assign completely different class to this property.

    /**
     * Paypal HttpClient
     *
     * @var \Thinkstudeo\Cashier\Paypal\Client
     */
    protected $client;

If you are on php 7.4 you can test if protected \Thinkstudeo\Cashier\Paypal\Client $client; will still work, but it should throw an error from what I can see.

[thinkstudeo]

Not yet, I'm still on PHP 7.3

But I see that it should have been as

  /**
     * Paypal HttpClient
     *
     * @var \PaypalCheckoutSdk\Core\PayPalHttpClient
     */
    protected $client;

I have made the above change and am reindexing to check if it works.

Update Thanks for pointing this out. Changing to @var \PayPalCheckoutSdk\Core\PayPalHttpClient and reindexing the workspace removes the error.

Silly me. Thanks for pointing this out.