Deprecate calling QueryBuilder methods with an array argument

[BenMorel]

Fixes #3837.

[morozov]

Could we please annotate the corresponding arguments as variadic so that it's clear what should be used instead?

[morozov]

Otherwise, looks good.

[BenMorel]

This makes phpstan choke:

$groupBy = is_array($groupBy) ? $groupBy : func_get_args();

Else branch is unreachable because ternary operator condition is always true.

Any idea how to fix this?

[morozov]

Any idea how to fix this?

Could you try adding treatPhpDocTypesAsCertain: false to the PHPStan configuration? See phpstan/phpstan#2894 (comment). Alternatively, maybe annotate a second variadic argument instead of making the first one variadic:

/**
 * @param mixed    $value
 * @param ...mixed $values
 */
function f($value/*, ...$values */)

[BenMorel]

Could you try adding treatPhpDocTypesAsCertain: false

Didn't work. Same errors.

Alternatively, maybe annotate a second variadic argument instead of making the first one variadic

Doesn't work if the argument is commented out:

PHPDoc tag @param references unknown parameter: $groupBys

And if it's not commented out, back to square one:

Call to function is_array() with string will always evaluate to false.

[morozov]

Then we’ll need to change the deprecation message:

USING AN ARRAY ARGUMENT IS DEPRECATED. Pass each value as an individual argument.

[BenMorel]

What works is keep documenting the array, and the variadic as string. Which may make sense, after all:

    /**
     * Adds a grouping expression to the query.
     *
     * USING AN ARRAY ARGUMENT IS DEPRECATED. This feature will be removed in the next major version.
     *
     * (...)
     *
     * @param string|array $groupBy     The grouping expression. USING AN ARRAY IS DEPRECATED.
     * @param string       ...$groupBys
     *
     * @return $this This QueryBuilder instance.
     */
    public function addGroupBy($groupBy, ...$groupBys)

[morozov]

It will introduce a BC break for extending classes (https://3v4l.org/CSa1P). Let's avoid code changes.

[BenMorel]

It will introduce a BC break

Damn. What about this: 0af7922?

[morozov]

What about this: 0af7922?

Good with me.

[BenMorel]

Removed "Pass at least one value." and squashed.

[morozov]

Thanks, @BenMorel!

[BenMorel]

@morozov Have you been able to merge to master?

[morozov]

Just filed #3857. There was a ton of conflicts between 3.0.x and master.

[BenMorel]

I knew this would be fun. Are you doing it as a PR to get reviews?

[morozov]

It's not that I want to get a review but I want the merge commit to be built in the same environment where it would be built after the merge. Also, we have all release branches protected.

I knew this would be fun.

Actually, merging 2.11.x into 3.0.x was pretty much straightforward. Merging 3.0.x into master was a nightmare because it currently contains a collection of commits from different times previously backported from master. I could have it done earlier. Then this merge would have been a walk in the park.

[morozov]

@BenMorel, #3857 is merged.

[BenMorel]

Good job. Thank you!