statamic / cms

The core Laravel CMS Composer package
https://statamic.com
Other
4.01k stars 527 forks source link

Laravel v8.81 introduces an exception during console commands and package discovery #5126

Closed dniccum closed 2 years ago

dniccum commented 2 years ago

Bug description

After performing a version update, I am receiving an error with the statamic/stringy/src/Stringy.php utility that is included with the Statamic CMS. This does appear to be a breaking issue as any of the php artisan and php please commands throw the error.

How to reproduce

  1. Upgrade your installation to use the most recent version of Statamic
  2. Make sure that you have the following entry in your composer.json file: "laravel/framework": "^8.81"
  3. Attempt to perform a composer dumpautoload command to leverage Laravel's auto package discovery

Logs

> Statamic\Console\Composer\Scripts::preUpdateCmd
Loading composer repositories with package information
Updating dependencies
Lock file operations: 0 installs, 1 update, 0 removals
  - Upgrading laravel/framework (v8.80.0 => v8.81.0)
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Upgrading laravel/framework (v8.80.0 => v8.81.0): Extracting archive
Package swiftmailer/swiftmailer is abandoned, you should avoid using it. Use symfony/mailer instead.
Generating optimized autoload files
> Illuminate\Foundation\ComposerScripts::postAutoloadDump
> @php artisan package:discover --ansi

   InvalidArgumentException 

  Passed value cannot be an array

  at vendor/statamic/stringy/src/Stringy.php:45
     41▕      */
     42▕     public function __construct($str = '', $encoding = null)
     43▕     {
     44▕         if (is_array($str)) {
  ➜  45▕             throw new InvalidArgumentException(
     46▕                 'Passed value cannot be an array'
     47▕             );
     48▕         } elseif (is_object($str) && !method_exists($str, '__toString')) {
     49▕             throw new InvalidArgumentException(

      +8 vendor frames 
  9   [internal]:0
      Statamic\Extend\AddonRepository::Statamic\Extend\{closure}("aryehraber/statamic-logbook")

      +14 vendor frames 
  24  artisan:37
      Illuminate\Foundation\Console\Kernel::handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))

Versions

Installation

Starter Kit using via CLI

Additional details

No response

aerni commented 2 years ago

I just ran into this as well. Everything works fine when fixing the Laravel version to "laravel/framework": "8.80" but fails with "laravel/framework": "^8.0". It seems to be an issue with Laravel 8.81.

okaufmann commented 2 years ago

Hi @jasonvarga

I ran into the same problem before I found this issue only after I had already looked in the code.

At first, it seemed to be a problem with the parameter order of Statamic\Support\Str where the signature is

replace($string, $search, $replace)

(in latest version v3.2.31 https://github.com/statamic/cms/blob/v3.2.31/src/Support/Str.php#L262) but Laravel's Str::replace uses

replace($search, $replace, $subject)

(did not change in the last 9 months https://github.com/laravel/framework/blame/8.x/src/Illuminate/Support/Str.php#L612).

When I did change the arguments of Statamic\Support\Str's replace method to the compatible signature public static function replace($search, $replace, $string) (https://github.com/statamic/cms/blob/v3.2.31/src/Support/Str.php#L262) it then goes on.

But then it calls Stringy's replace method (https://github.com/statamic/Stringy/blob/master/src/Stringy.php#L1077).

public function replace($search, $replacement)
{
    return $this->regexReplace(preg_quote($search), $replacement);
}

I think here lays the main problem, that preg_quote is just not compatible with $search be an array (what is supported by the default Laravel replace method as it just uses the built-in str_replace method and the studly method in Laravel 8.80 did).

As this is fixed now, I thought my deep dive helps identify the core problem about this "bug".

Here is the PR changing the studly method for Laravel v8.81.0 https://github.com/laravel/framework/commit/3d50d374ab9f473ef2d2daf5008f146a60e7ce07#diff-262e10b3591ac74c9157a3e732ee544209ea224da8af4844ebacbb1e8e2713d4L842

And here is the Blame for the method: https://github.com/laravel/framework/blame/8.x/src/Illuminate/Support/Str.php#L840

Maybe the old Stringy replace method could be made compatible to allow multiple search needles to prevent copy pasting code from the source of Laravel?

okaufmann commented 2 years ago

I just saw that you anyway want to remove the Stringy dependency, which will solve this problem too 👍

jasonvarga commented 2 years ago

Our Str::replace() had always used Stringy's version, which broke when Laravel added their own Str::replace(). So yes, we'll be addressing that in 4.0. Not really something we can change at the moment.