Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

phpdoc are not precise #30

Open
max-matiukhin opened this issue Dec 24, 2018 · 5 comments
Open

phpdoc are not precise #30

max-matiukhin opened this issue Dec 24, 2018 · 5 comments

Comments

@max-matiukhin
Copy link

I've caught this error using \Aerospike::get() method, but it seems, other methods might have similar issues.
The file https://github.com/aerospike/aerospike-client-php/blob/master/doc/phpdoc/aerospike.php gives us this interface for the method get():

public function get(array $key, array &$record, array $select = [], array $options = []) {}

There are 2 problems here.
First, it says that the default value for the last 2 parameters is an empty array. However this code gonna fail:

$aerospike = new \Aerospike(['hosts' => [ ['addr' => 'aerospike1.local', 'port' => 4000]]]);
$key = $aerospike->initKey('test', 'test', 'abc')
$response_status = $aerospike->get($key, $result, [], []);

with $response_status === 4 (AS_PROTO_RESULT_FAIL_PARAMETER)
It seems that the real default value for the last 2 parameters is null.
This code works without errors:

$aerospike = new \Aerospike(['hosts' => [ ['addr' => 'aerospike1.local', 'port' => 4000]]]);
$key = $aerospike->initKey('test', 'test', 'abc')
$response_status = $aerospike->get($key, $result, null, null);

The second problem is about the second parameter - array &$record. AFAIK, in PHP if you create function with such parameter, you should pass an array there, otherwise it fails due to type mismatch

function get(array &$record) { ... }
$my_record = []; 
get($my_record); // this call is ok because my_record is array and function expects an array
get($undeclared_record); // this call gonna fail because $undeclared_record is null but function expects an array

It's not critical, but these errors are visible if you:

  1. use static analyzers
  2. create a wrapper-class around the original Aerospike class and you want to mimic the original class. In this case you might want to copy-paste the interface from the phpdoc and you will get these problems.
@aerospikerobertmarks
Copy link
Contributor

Thanks for noticing this. I'll fix the those documentation issues.

In the second case, we were trying to indicate that, you were passing in a reference to a return value which would end up being turned into an array, but as you correctly point out, the current documentation does not correctly indicate that.

Thanks again for discovering these issues.

@max-matiukhin
Copy link
Author

One more issue with constants.
In the php file they are string.
So, when you run your code and got an error 4, you can't open this file and find what this code means.
Also, static analyzers check this file and see that constants are strings and in the real code we compare it with int. So we get messages from static analyzers.

Another issue.
Some constants in this file have wrong name. Here is the list:

const ERR_TLS = -9;
const ERR_FORBIDDEN = 22;
const ERR_FAIL_NOT_FOUND = 23;
const ERR_QUERY_END = 50;
const ERR_BATCH_MAX_REQUESTS_EXCEEDED = 151;
const OPT_POLICY_RETRY = 4;
const OPT_SCAN_INCLUDELDT = 11;
const COMPRESSION_THRESHOLD = 19;
const POLICY_RETRY_NONE = 0;
const POLICY_RETRY_ONCE = 1;
const POLICY_REPLICA_PREFER_RACK = 3;
const OPT_TLS_CAFILE = 33;

@ondrejmirtes
Copy link

Also there's a problem with ReflectionParameter because for some function+parameter from this extension, ReflectionParameter::getName() returns null which shouldn't be possible.

@dwelch-spike
Copy link
Contributor

Thank you for pointing this out. I'll look into a fix and post an update when done. If you know which functions are causing the issue could you please post them?

Thanks

@ramunasd
Copy link
Contributor

ramunasd commented Apr 23, 2020

I can confirm this issue. Easy way to list all empty parameter names:

$class = new ReflectionClass('Aerospike');
foreach ($class->getMethods() as $method) {
    $parameters = $method->getParameters();
    foreach ($parameters as $key => $parameter) {
        $name = $parameter->getName();
        if (empty($name)) {
            printf("Empty method %s parameter %u name\n", $method->getName(), $key);
        }
    }
}

in my env PHP7.3, Aerospike client 7.5.2 output is:

Empty method operateOrdered parameter 0 name
Empty method operateOrdered parameter 1 name
Empty method operateOrdered parameter 2 name
Empty method operateOrdered parameter 3 name
Empty method scanInfo parameter 0 name
Empty method scanInfo parameter 1 name
Empty method scanInfo parameter 2 name

Due this is impossible to use Aerospike class directly as Symfony DI lazy service.

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

No branches or pull requests

5 participants