Documentation and possibly implementation for Differ needs updated

[denzel-morris]

The documentation for Differ::diff states that it accepts both arrays and strings as parameters:

/**
     * Returns the diff between two arrays or strings as string.
     *
     * @param array|string                            $from
     * @param array|string                            $to
     * @param LongestCommonSubsequenceCalculator|null $lcs
     *
     * @return string
     */
    public function diff($from, $to, LongestCommonSubsequenceCalculator $lcs = null): string

This is clearly not the case since Differ::validateDiffInput explicitly tries to cast everything to a string except arrays:

/**
     * Casts variable to string if it is not a string or array.
     *
     * @param mixed $input
     *
     * @return string
     */
    private function validateDiffInput($input): string
    {
        if (!\is_array($input) && !\is_string($input)) {
            return (string) $input;
        }
        return $input;
    }

The first fix is obvious, the documentation for Differ::diff needs to be updated. The second issue, is maybe we want to throw an InvalidArgumentException on an array instead of the much more opaque:

TypeError: Return value of SebastianBergmann\Diff\Differ::validateDiffInput() must be of the type string, array returned

Throwing InvalidArgumentException with an appropriate description would represent a better user experience.

If we agree, let me know, and I'll submit a PR with both fixes referencing this issue.

[SpacePossum]

Thanks for reporting 👍

IMHO there are a few things that need fixing about the method validateDiffInput:

• the return type hint : string is a bug and should be removed
• the PHPDoc should be updated that as well (@return string|array)
• it is probably better to rename this method to normalizeDiffInput
• the unit tests should be updated to prove the bug in the first place and than prove that this issue is resolved

This all can be done without breaking the BC promise.

Throwing an exception when not an array nor string is passed will be a BC break.
Currently we support more cases using the string cast, for example you can diff(123, '456').
While the weak type support is not ideal, it is probably best to keep for now and drop the support of it on the next major release.

[denzel-morris]

Awesome, thanks for the speedy response @SpacePossum, I agree with your assessment. I'll take the steps to submit a PR for this.