laminas / laminas-stdlib

SPL extensions, array utilities, error handlers, and more
https://docs.laminas.dev/laminas-stdlib/
BSD 3-Clause "New" or "Revised" License
190 stars 40 forks source link

StringUtils::registerWrapper(): in_array() expects parameter 2 to be array, null given #51

Closed arokettu closed 2 years ago

arokettu commented 2 years ago

Bug Report

Q A
Version(s) 2.1.0-3.7.0

Summary

\Laminas\Stdlib\StringUtils::registerWrapper() generates a warning in PHP < 8.0, TypeError on PHP >= 8.0 if the current wrapper list is empty.

The scenario is broken since the times immemorial introduction of StringUtils in Zend 2.1

Current behavior

in_array() expects parameter 2 to be array, null given

How to reproduce

<?php

use Laminas\Stdlib\StringUtils;
use Laminas\Stdlib\StringWrapper\MbString;

require 'vendor/autoload.php';

StringUtils::resetRegisteredWrappers();
StringUtils::registerWrapper(MbString::class);
var_dump(StringUtils::getRegisteredWrappers());

Expected behavior

A single required wrapper is registered without warnings or exceptions

Ocramius commented 2 years ago

@arokettu would you be able to send a patch for this? 🤔

arokettu commented 2 years ago

I can do it but I would like to discuss what is the real expected behavior here then. What's clear for me is that it's definitely not the current one.

My expectations came from reading the code that the wrappers that I register immediately after reset will replace the default list but now I think that the new wrapper should be added to the default ones. (in that case I will also have a feature request)

Ocramius commented 2 years ago

I wish I knew the original intent here: best we can do is look at any existing tests and docs for the feature

arokettu commented 2 years ago

I haven't found any except for PHPDoc in the file that claims that resetRegisteredWrappers() should reset to the default list

then replacing

in_array($wrapper, static::$wrapperRegistry, true)

with

in_array($wrapper, static::getRegisteredWrappers(), true)

makes most sense to me

Ocramius commented 2 years ago

@marc-mabe I see that the component has been mostly designed by you: are you able to help out with some guidance?

marc-mabe commented 2 years ago

getRegisteredWrappers should be used and the wrapperRegistry should not be used directly. The NULL value is a marker that the static class hasn't been initialized

arokettu commented 2 years ago

Ok. #52

Ocramius commented 2 years ago

Thanks!