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

Add support for new types (like CurlHandle) in PHP8 #3824

Closed
xPaw opened this issue Jul 16, 2020 · 11 comments · Fixed by #9227
Closed

Add support for new types (like CurlHandle) in PHP8 #3824

xPaw opened this issue Jul 16, 2020 · 11 comments · Fixed by #9227

Comments

@xPaw
Copy link

xPaw commented Jul 16, 2020

https://github.com/php/php-src/blob/master/UPGRADING

  1. curl_init() will now return a CurlHandle object rather than a resource.
  2. curl_multi_init() will now return a CurlMultiHandle object rather than a resource.
  3. curl_share_init() will now return a CurlShareHandle object rather than a resource.
  4. shmop_open() will now return a Shmop object rather than a resource.
  5. enchant_broker_init() will now return an EnchantBroker object rather than a resource.
  6. The GD extension now uses objects as the underlying data structure for images, rather than resources.
  7. msg_get_queue() will now return an SysvMessageQueue object rather than a resource.
  8. sem_get() will now return an SysvSemaphore object rather than a resource.
  9. shm_attach() will now return an SysvSharedMemory object rather than a resource.
  10. xml_parser_create(_ns) will now return an XmlParser object rather than a resource.
  11. inflate_init() will now return an InflateContext object rather than a resource.
  12. deflate_init() will now return a DeflateContext object rather than a resource.
  13. openssl_x509_read() and openssl_csr_sign() will now return an OpenSSLCertificate object rather than a resource.
  14. openssl_csr_new() will now return an OpenSSLCertificateSigningRequest object rather than a resource.
  15. openssl_pkey_new() will now return an OpenSSLAsymmetricKey object rather than a resource.
  16. socket_create(), socket_create_listen(), socket_accept(), socket_import_stream(), socket_addrinfo_connect(), socket_addrinfo_bind(), and socket_wsaprotocol_info_import() will now return a Socket object rather than a resource.
  17. socket_addrinfo_lookup() will now return an array of AddressInfo objects rather than resources.

All the related functions that previous took resource now take the object. is_resource does not work on them.

If I understand correctly, a CallMap delta for 8.0 needs to be created?

@psalm-github-bot
Copy link

Hey @xPaw, can you reproduce the issue on https://psalm.dev ?

@weirdan
Copy link
Collaborator

weirdan commented Jul 16, 2020

If I understand correctly, a CallMap delta for 8.0 needs to be created?

Yes.

@xPaw
Copy link
Author

xPaw commented Nov 14, 2020

Would also be great to introduce a warning for functions like curl_close and imagedestroy to replace them with unset calls.

@TysonAndre
Copy link
Contributor

One potential solution I thought of for analyzing forward compatibility would be to add a type such as resource<'CurlHandle'> or resource{CurlHandle} in the function signature maps and deltas for the older signatures. (e.g. curl_init would return that type, and curl_exec would accept that type as a param type, and so on. A generic resource would be allowed to cast to that type.)

  • When analyzing a function such as curl_exec, the presence of resource{CurlHandle} would indicate that type is converted to an object in the future, and the error message can mention CurlHandle when warning about is_resource or is_object being potentially wrong in older php versions. (general advice would be to check truthiness instead and delete all references instead of calling *_close(), or to move checks to a reusable helper method)
  • This also allows warning about passing a handle for curl_init to curl_multi_init (CurlHandle != CurlMultiHandle) even when targeting older php versions.
  • Treating template params differently from regular classes in templates or as a non-template would potentially help avoid false positive warnings about undeclared classes, but the phpdoc parser could probably do that conversion internally.

@derrabus
Copy link

can you reproduce the issue on https://psalm.dev ?

https://psalm.dev/r/d9103abb4b

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/d9103abb4b
<?php

class Secret
{
    private \OpenSSLAsymmetricKey $key;
    
    public function __construct(string $pk, string $passphrase)
    {
        if (!$key = openssl_pkey_get_private($pk, $passphrase)) {
            throw new InvalidArgumentException('Unable to load DKIM private key: '.openssl_error_string());
        }
        
        $this->key = $key;
    }
    
    public function getKey(): \OpenSSLAsymmetricKey
    {
        return $this->key;
    }
}
Psalm output (using commit f496cca):

ERROR: UndefinedClass - 5:13 - Class, interface or enum named OpenSSLAsymmetricKey does not exist

ERROR: InvalidPropertyAssignmentValue - 13:22 - $this->key with declared type 'OpenSSLAsymmetricKey' cannot be assigned type 'resource'

ERROR: UndefinedClass - 16:31 - Class, interface or enum named OpenSSLAsymmetricKey does not exist

@kkmuffme
Copy link
Contributor

@orklah afaik this is implemented already now? (ticket can be closed?)

@AndrolGenhald
Copy link
Collaborator

Looks like we still need:

4. shmop_open() will now return a Shmop object rather than a resource.
5. enchant_broker_init() will now return an EnchantBroker object rather than a resource.
7. msg_get_queue() will now return an SysvMessageQueue object rather than a resource.
8. sem_get() will now return an SysvSemaphore object rather than a resource.
9. shm_attach() will now return an SysvSharedMemory object rather than a resource.
11. inflate_init() will now return an InflateContext object rather than a resource.
12. deflate_init() will now return a DeflateContext object rather than a resource.

@TAS-EW-02
Copy link

In the current version (Psalm 4.26.0) I still get false-positives for PHP 8.0:

Class, interface or enum named CurlHandle does not exist (see https://psalm.dev/019)

It is the same message for all CURL related classes and constants (e.g. CURLOPT_URL).

kelunik added a commit to amphp/byte-stream that referenced this issue Sep 11, 2022
@weirdan
Copy link
Collaborator

weirdan commented Nov 24, 2022

@derrabus OpenSSL snippet now does not produce errors: https://psalm.dev/r/d9103abb4b

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/d9103abb4b
<?php

class Secret
{
    private \OpenSSLAsymmetricKey $key;
    
    public function __construct(string $pk, string $passphrase)
    {
        if (!$key = openssl_pkey_get_private($pk, $passphrase)) {
            throw new InvalidArgumentException('Unable to load DKIM private key: '.openssl_error_string());
        }
        
        $this->key = $key;
    }
    
    public function getKey(): \OpenSSLAsymmetricKey
    {
        return $this->key;
    }
}
Psalm output (using commit 8f39de9):

No issues!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants