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

Invalid phpdoc (list instead of string) #1423

Open
momala454 opened this issue Feb 20, 2025 · 3 comments
Open

Invalid phpdoc (list instead of string) #1423

momala454 opened this issue Feb 20, 2025 · 3 comments

Comments

@momala454
Copy link

momala454 commented Feb 20, 2025

In your phpdoc, by example vendor/elasticsearch/elasticsearch/src/Endpoints/Indices.php

/**
	 * Updates the index settings.
	 *
	 * @see https://www.elastic.co/guide/en/elasticsearch/reference/master/indices-update-settings.html
	 *
	 * @param array{
	 *     index: list, //  A comma-separated list of index names; use `_all` or empty string to perform the operation on all indices
	 *     master_timeout: time, // Specify timeout for connection to master
	 *     timeout: time, // Explicit operation timeout
	 *     preserve_existing: boolean, // Whether to update existing settings. If set to `true` existing settings on an index remain unchanged, the default is `false`
	 *     reopen: boolean, // Whether to close and reopen the index to apply non-dynamic settings. If set to `true` the indices to which the settings are being applied will be closed temporarily and then reopened in order to apply the changes. The default is `false`
	 *     ignore_unavailable: boolean, // Whether specified concrete indices should be ignored when unavailable (missing or closed)
	 *     allow_no_indices: boolean, // Whether to ignore if a wildcard indices expression resolves into no concrete indices. (This includes `_all` string or when no indices have been specified)
	 *     expand_wildcards: enum, // Whether to expand wildcard expression to concrete indices that are open, closed or both.
	 *     flat_settings: boolean, // Return settings in flat format (default: false)
	 *     pretty: boolean, // Pretty format the returned JSON response. (DEFAULT: false)
	 *     human: boolean, // Return human readable values for statistics. (DEFAULT: true)
	 *     error_trace: boolean, // Include the stack trace of returned errors. (DEFAULT: false)
	 *     source: string, // The URL-encoded request definition. Useful for libraries that do not accept a request body for non-POST requests.
	 *     filter_path: list, // A comma-separated list of filters used to reduce the response.
	 *     body: array, // (REQUIRED) The index settings to be updated
	 * } $params
	 *
	 * @throws NoNodeAvailableException if all the hosts are offline
	 * @throws ClientResponseException if the status code of response is 4xx
	 * @throws ServerResponseException if the status code of response is 5xx
	 *
	 * @return Elasticsearch|Promise
	 */
	public function putSettings(array $params = [])
	{
...
}

You use list to describe comma-separated list of index names, so a string.
However, a list means an array of numerical indexes starting at 0.

I have updated phpstan to 2.1.6, and now (probably due to TypeParser - support comments at EOL with // (https://github.com/phpstan/phpdoc-parser/pull/264), https://github.com/phpstan/phpdoc-parser/issues/184, thanks @shmax!), it requires an array (list) instead of string

Parameter #1 $params of method Elastic\\Elasticsearch\\Endpoints\\Indices::putSettings() expects array{index: list, master_timeout: Elastic\\Elasticsearch\\Endpoints\\time, timeout: Elastic\\Elasticsearch\\Endpoints\\time, preserve_existing: bool, reopen: bool, ignore_unavailable: bool, allow_no_indices: bool, expand_wildcards: Elastic\\Elasticsearch\\Endpoints\\enum, ...}, array{index: string, body: array{index: array{refresh_interval: string, number_of_replicas: string}}} given. \n💡 Offset 'index' (list<mixed>) does not accept type string: string is not a list.
@AJenbo
Copy link

AJenbo commented Feb 20, 2025

Looks like most of them are actually pulled though this:

    /**
     * Converts array to comma-separated list;
     * Converts boolean value to true', 'false' string
     * 
     * @param mixed $value
     */
    private function convertValue($value): string
    {
        // Convert a boolean value in 'true' or 'false' string
        if (is_bool($value)) {
            return $value ? 'true' : 'false';
        // Convert to comma-separated list if array
        } elseif (is_array($value) && $this->isNestedArray($value) === false) {
            return implode(',', $value);
        }
        return (string) $value;
    }

Meaning the lib internally will convert an actual list to a comma-separated string. So probably the type should be updated to list|string since it can handle both.

@momala454
Copy link
Author

momala454 commented Feb 20, 2025

Thanks.
Another problem. They are all optional, so they should be written like this (with a ?)

	 *     index?: list, //  A comma-separated list of index names; use `_all` or empty string to perform the operation on all indices

edit: actually on putSettings, body is required, all others are optionals

@AJenbo
Copy link

AJenbo commented Feb 20, 2025

There is a PR to fix the optional part: #1426

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

No branches or pull requests

2 participants