Hi, can you please send your suggested changes as pull request?

Duplicate #16174? Which template are you using?

@Quy, it doesn't matter what template, it's in the Joomla core: libraries/src/HTML/HTMLHelper.php.

It appears this issue has cropped up specifically because of the way PHP 7.1 handles array parameters to functions. Here is an example, from HTMLHelper.php:

public static function stylesheet( $file, $options = array(), $attribs = array() )

PHP 7.1 throws a warning since $options may not be passed as an array, so my fix shown above tests if it is not an array, and if so defines it as an array, which is the same thing @davidcchapman did in #16174.

@zero-24, best would be for a developer to search for all cases of functions in the core with parameters declared as arrays – I don't know if this is practical because of the huge number of cases, but perhaps better than having this issue pop up again and again all over the Joomla core. I don't have the time to do this, sorry.

UPDATE: I just did some quick grepping and came up with 131 instances of static functions in the core with "array()" in the parameters list. I looked in "includes", "layouts", and "libraries". The grand total, including installed extensions and administrator folder was 180.

it doesn't matter what template, it's in the Joomla core: libraries/src/HTML/HTMLHelper.php

Actually, yes, it does matter. It's not core's responsibility to handle undocumented parameter types in every function. If a developer is not passing the right parameter types into methods, there should correctly be warnings and/or errors emitted, the code shouldn't be silently absorbing those and coercing someone's broken input into something that can generate a valid output. The second parameter of JHtml::stylesheet() was always documented to be an array, even before the signature restructure in 3.7, so to generate this error a developer must have explicitly been passing a non-array type into the method, which depending on how the param is used can and will eventually result in errors and/or warnings popping up somewhere in the system.

@mbabker ...

It's not core's responsibility to handle undocumented parameter types in every function

Well, it's a choice of the Joomla developers for functions to not check parameters and return error codes. In my opinion, it shouldn't be left to various PHP versions to emit warnings or not.

I actually wrote some test code last night to try to prove what was happening in JHtml::stylesheet() and couldn't reproduce the failure, which is strange in itself. I couldn't get PHP 7.1 to emit the warning with my test code.

JHtml::stylesheet('file', null, array());

That should be all you need, the key is in passing a non-array as the second parameter.

Well, it's a choice of the Joomla developers for functions to not check parameters and return error codes.

Littering everything with assert(is_array($foo)); is going a bit far. Can't force everyone to follow the documented contract without major B/C breaks at this point (because I'm in the camp that everything should be strictly typehinted, and without a PHP 7.2 minimum adding typehints is a huge B/C break), and a lot of people argue that adding new errors in a method not documented as throwing an error is a B/C break (so you can't throw an InvalidArgumentException if it's not the right type). So, the only "good" fix is to have people do it right the first time and stop developing things with error reporting turned off or error_reporting(0) statements in their code to force their bad code's issues to be hidden. The if (!is_array($options)) $options = array(); option, which "fixes" core, masks an error condition that a developer should be alerted to and that's pretty bad for debugging problems.

I'm not suggesting that core API functions emit errors, I'm suggesting they return error codes – but this may not be how Joomla developers decided to write all the code. If they did, then a) PHP wouldn't emit errors in core functions, and b) extension devs could check return codes and make their code better. This would place the responsibility on extension devs, rather than on Joomla devs or website developers to report problems with extension code. By the way, how many functions are considered API functions?

You can't return error codes, PHP doesn't work that way (well, you can return Exception objects, but who's really writing error handling code to deal with that; core didn't for the longest time and still doesn't do error handling in a lot of places). You either throw an Exception or you emit an error (either the PHP engine can do it or you can do it yourself with trigger_error()). There shouldn't be a if (!is_array($options)) { return Joomla\CMS\Error::INVALID_ARGUMENT; } type of thing (and it's worse in this case because you lose the stack trace without creating some kind of object, i.e. Exception classes). Doing anything in that !is_array($options) check except letting an error shoot up means core is absorbing the error and that's not a good way forward.

PHP can't return error codes?! The typical way to handle this is for the function to return false or null if there was an error. If there's more than one kind of possible error, then the error code (number or string) can be passed in a pass-by-reference parameter; example:

function getFromArray($index, $list, &$error=null) {
    if ($index >= 0 && $index < count($list)) {
        return $list[$index];
    }

    $error = "out of index";
    return null;
}

In my opinion, all Joomla API functions should be coded this way, in order to be robust for interfacing to the wild world of extensions, including templates. Internal functions probably don't need this, but it is a good practice nonetheless.

I think the API functions should have strict parameter checking, and then return error codes where necessary.

That is crap for debugging. All you have is an error message, how do you determine where it comes from? error_get_last() is useless, you didn't trigger an error. A try/catch block is useless, you didn't throw an Exception. A lot of developers do not do any form of error handling, including a cursory review of their log files (assuming they've even enabled logging to them), so a core API with falsy returns or $error variables is going to be an ignored API. You're stuck trying to figure out where this "error code" came from, then having to analyze every call to that method (and hopefully you've got a unique message in every method otherwise you're in debugging hell, and hopefully it's a direct call to that method because if you get to a method dispatcher like JHtml::_() then you're really in for a fun time). You either want to detect the error yourself and throw an Exception or you let PHP emit its own error/warning. Anything else and you're re-inventing PHP's error reporting mechanisms or you're shooting yourself in the foot by writing code that is too complex to debug or just flat out non-debuggable.

No, no. The API is well documented and the extension developer needs to follow exactly the parameters that are documented for each function. If they don't, their code will be poorly written. It's that simple. This is the best way to insure that the developer writes good code. Joomla core functions causing PHP warnings or errors is a poor design in my opinion.

It just does not work that way. If you don't let a PHP error or warning be emitted, or you do not throw an Exception, you have a non-debuggable error condition. Having if (!is_array($options)) { return Joomla\CMS\Error::INVALID_ARGUMENT; } results in the actual code error being absorbed inside this "error code" and does absolutely nothing to signal that there is broken code. That makes this an error condition that cannot be debugged, a developer is unaware there is an error and the error that is given is a generic error without any context at all.

To signal there has been an error, the accepted practices are for PHP to emit an error (userland call to trigger_error() or the core C engine of PHP calling the similar function) or for a developer to throw an Exception.

Using your "error code" concept, I cannot debug an error because I have no information about it. What method emitted the error, what the actual error was, the call stack to reach that error, etc. If you are writing code that basically sucks up every possible error condition in a way that it can't be debugged, you are making your own life miserable, as well as the lives of every API consumer, because you or someone WILL have to eventually debug something because of that error absorption code and you WILL end up wanting to time travel and smack yourself for writing such code.

Make it mandatory that the code calling API functions must throw an exception if the function returns an error. It would be relatively easy to locate code in an extension that is not following this rule, so the first debugging step is to verify that the developer followed the rules. Now Joomla's API functions will not produce PHP errors or warnings in any situation, because parameter checking has been done.

Imagine this scenario:

const INVALID_ARGUMENT = 17;

function stylesheet_without_error_absorbing_code_return($file, $options, $attribs)
{
    $reference = isset($options['reference']) ? $options['reference'] : null;
}

function stylesheet_with_error_absorbing_code_return($file, $options, $attribs)
{
    if (!is_array($options)) {
        return INVALID_ARGUMENT;
    }

    $reference = isset($options['reference']) ? $options['reference'] : null;
}

// This is going to generate a PHP error inside the function being called that tells you pretty much exactly what the problem is
stylesheet_without_error_absorbing_code_return('file', null, array());

// The actual error here, assuming the developer actually implements error checking and handling procedures, has been masked by an "error code" return
if (stylesheet('file', null, array()) === INVALID_ARGUMENT) {
    throw new InvalidArgumentException('I am a bad developer');
}

It has two basic problems:

1. It relies on the code which generated the error in the first place to raise an error
2. It still obscures the real error and the error context you do get from this scenario is not the actual error context

If you want to fix this issue, then either put error_reporting(0); into your code and ignore it or disable error reporting in your PHP configuration. It is not appropriate AT ALL for Joomla to be rewritten in a way that makes it impossible to debug a code error.

the first debugging step is to verify that the developer followed the rules

That's laughable at best.

Yes, the code that generated the error is responsible for the error, not Joomla core. In my opinion, Joomla core throwing PHP errors is not acceptable and doesn't look good for Joomla when someone views a website and sees a block of PHP errors coming from Joomla code.

The extension developer needs to follow some basic rules in order to write good code for use with Joomla, just like they need to follow rules for security. It's not laughable, but I don't want to argue with you anymore.

The extension code should report an error something like this:

if ( ! stylesheet('file', null, array() ) {
    throw new InvalidArgumentException('ModuleX, FunctionX:  internal error 1');
}

Thankyou for reporting this but like Michael says we’re not going to be fixing this one. If there’s an issue in 3rd party code that’s for them to solve. The doc blocks make it clear what types are and aren’t allowed in the style sheet function