It is of course up to discussion, but I would suggest to allow one line closures, because some times there is a need to pass empty closure as an argument and then you always have to chop down whole call.

$this->doMagic( 'with string', function () {} );

VS

$this->doMagic( 
    'with string', 
    function () {
    } 
);

Have no idea how to approach this, but am willing it to take it on. @Legoktm Any guidance you can give me on how to tackle this?

Change 355368 had a related patch set uploaded (by MtDu; owner: MtDu):
[mediawiki/tools/codesniffer@master] Allow one line closures for functions

https://gerrit.wikimedia.org/r/355368

Upstream task is squizlabs/PHP_CodeSniffer#1580.

function () {} fails Squiz.WhiteSpace.ScopeClosingBrace.ContentBefore. function () { statement(); } fails that plus Generic.Formatting.DisallowMultipleStatements.SameLine.

Can we decline this, please, along with https://gerrit.wikimedia.org/r/355368? I mean, what is even the problem we are trying to solve here? array_map( function ( $x ) { return $this->format( $x ); }, $array ); is close to being unreadable. Enforcing it to be formatted as

array_map( function ( $x ) {
    return $this->format( $x );
}, $array );

or even

array_map(
    function ( $x ) {
        return $this->format( $x );
    },
    $array
);

is a really good thing, in my humble opinion.

The other example mentions the need to pass empty, unnamed function () {}. Please, help me to understand:

1. How often does something like this even happen? I checked and am able to find about a dozen instances in all Wikibase codebases combined, and about a dozen in MediaWiki core. Are (essentially) two dozen occurrences of {\n} really worth any effort?
2. In an example like $object->process( function () {} ); I would argue it is very valuable to give the mysterious parameter a name, e.g.:

$nullValidator = function () {
};
$object->process( $nullValidator );

Sure, the last argument is unrelated to the fact there is a newline in the empty function. But is it really a problem to have it?

In T154789#4824254, @thiemowmde wrote:

I mean, what is even the problem we are trying to solve here?

Things like https://gerrit.wikimedia.org/r/c/mediawiki/core/+/467094 where a closure can be put on a single line because it only contains one statement. Other examples are redefining services in tests with a closure returning an object, a submit handler for HTMLForm with a closure returning true. (HTMLForm requires the submit handler to be present)

To give an example for the last one:

$form->setSubmitTextMsg( 'submit' )
	->setSubmitCallback( function () { return true; } )
	->setId( 'id' )
	->setMethod( 'post' )
	->setFormIdentifier( 'identifier' );

Using this is a much neater approach: it doesn't break the flow as much as

$form->setSubmitTextMsg( 'submit' )
	->setSubmitCallback( function () { 
		return true;
 	} )
	->setId( 'id' )
	->setMethod( 'post' )
	->setFormIdentifier( 'identifier' );

does.

In T154789#4824254, @thiemowmde wrote:

Sure, the last argument is unrelated to the fact there is a newline in the empty function. But is it really a problem to have it?

I'd argue that

$nullValidator = function () {};
$object->process( $nullValidator );

is more readable than

$nullValidator = function () {
};
$object->process( $nullValidator );

is, especially as it makes it look like the curly bracket is part of a flow/control statement like if or while, instead of the closure.

I agree with @Mainframe98 here.

I agree both with Thiemo that the case of array_map, where parameter(s) in addition to the closure are involved that it is no longer a single statement on that line, which adds to the cognitive load of balancing parenthesis, comma's, etc. and gets trickier to read correctly and quickly without ambiguity.

I also agree with @Mainframe98 here that it should be allowed (and encouraged) to have empty functions or functions with a single statement, be written on a single line.

$foo = function () {};

$props = array_map(
  function ( $val ) { return $val->prop; },
  $values
};

$chain->with()
  ->setBefore( function () {} )
  ->execute();

These all seem valid and highly readable and should not be rejected.

If accepting this means we also let PHPCS accept code we'd prefer differently, I think that's okay.

For PHPCS, I believe that if (and only if) we can't enforce our style guide exactly, we should favour allowance over restriction (until we can do better). False negatives are more hurtful here, than true negatives because ultimately the impact is only readability. And that's something I expect code reviewers to naturally notice, because it would affect them while reviewing.

For Phan or security analysis on the other hand, they tend to catch things a person could easily miss even when they try to look for it, so a small amount of inconvenience there to require opt-out where needed seems tolerable.

Change 355368 abandoned by Thiemo Kreuz (WMDE):

[mediawiki/tools/codesniffer@master] Allow one line closures for functions

Reason:

5 years old and certainly not a valid approach, according to the discussion in the ticket.

https://gerrit.wikimedia.org/r/355368

To supplement the discussion, now that we require PHP 7.4+, we have access to arrow functions. This should be the preferred approach for one-liners, given how little boilerplate it contains. @thiemowmde's examples would look like this instead:

array_map( fn ( $x ) => $this->format( $x ), $array );

array_map(
    fn ( $x ) => $this->format( $x ),
    $array
);

This neatly side-steps the issue described in the task description because in those cases you'd just use an arrow function. In more complex cases, we can follow @thiemowmde's request. Best of both worlds.

(Sure, function () {}; cannot be expressed that way, but fn () => null is functionally equivalent.)

Basically, we can decline this task with the recommendation to use arrow functions for one-liners and keep the existing PHPCS rules.