Overzealous multi-line parameter rule

[johnbillion]

Ref: #1453

The following code does not violate the multi-line parameter rule, but gets flagged as such:

$message = esc_html( sprintf(
	/* Translators: 1: user display name; 2: username; */
	__( 'Switched to %1$s (%2$s).', 'user-switching' ),
	$user->display_name,
	$user->user_login
) );

There is only one parameter being passed to esc_html(). That parameter itself contains a function call with multiple lines, but the call to esc_html() is a valid, single parameter call.

[jrfnl]

@johnbillion How does this not violate the multi-line parameter rule ? The one parameter being passed is a multi-line parameter.

[pento]

The same thing was mentioned in this comment, I find myself inclined to allow a single multi-line parameter.

With @johnbillion's example, I don't find this to be any more readable than the original code.

$message = sprintf(
	/* Translators: 1: user display name; 2: username; */
	__( 'Switched to %1$s (%2$s).', 'user-switching' ),
	$user->display_name,
	$user->user_login
);

$message = esc_html( $message );

[jrfnl]

Let's continue this discussion in issue #1330 to keep all the considerations for what may or may not need adjusting and for the secondary sniff which still needs to be created, in one place.

Re: @johnbillion's code sample - until that secondary sniff is written, formatting that code as follows should pass without issues:

$message = esc_html(
	sprintf(
		/* Translators: 1: user display name; 2: username; */
		__( 'Switched to %1$s (%2$s).', 'user-switching' ),
		$user->display_name,
		$user->user_login
	)
);

[lkraav]

On a hopefully related note, I'm not sure what I'm supposed to do with closures now:

ZEALOUSLY FLAGGED

*       add_filter( 'wc_memberships_for_teams_new_team_data', function( $data, $args ) {
            ...
            return $data;
*       }, 10, 2 );

UNFLAGGED, BUT VERTICALLY BLOATED TOWARDS HELL

        add_filter(
            'wc_memberships_for_teams_new_team_data',
            function( $data, $args ) {
                ...
                return $data;
            },
            10,
            2
        );

This can't be the intended result...?

EDIT cleverly hiding a reply to @azaozz here :) #1485 (comment)

Frankly I thing we should "ban" lambda functions (closures) in add_filter() and add_action().

In public facing code, maybe.

In internal apps, where thinking of callback function names is profit reducing overhead, because nothing is supposed to be unhooked, and everything is and can be properly documented, lambda functions still look fine to me.

[jrfnl]

@lkraav This is being discussed in #1330 from #1330 (comment) down.

[azaozz]

With @johnbillion's example, I don't find this to be any more readable than the original code.

True. It adds another level of indentation which is not great, but doesn't make that particular example too much harder to read.

However why is the sprintf() call on multi-lines in the first place, and has a nested call to __()? It would be much better if it was split and each call to a function is on a separate line, instead of "cleverly" combining it (where "cleverly" means reducing readability and opportunity for inline documentation).

IMHO the most readable way to write the example bit of code would be:

/* Translators: 1: user display name; 2: username; */
$message = __( 'Switched to %1$s (%2$s).', 'user-switching' );
// (Possible inline comment)
$message = sprintf( $message, $user->display_name, $user->user_login );
// (Possible inline comment)
$message = esc_html( $message );

The above "layout" has the following advantages:

1. No indentation.
2. Very easy to read, understand, and follow the order of execution.
3. Plenty of opportunities for inline docs at every function call.
4. It is actually more concise than any of the above examples.

[azaozz]

I'm not sure what I'm supposed to do with closures now

Frankly I thing we should "ban" lambda functions (closures) in add_filter() and add_action(). They bring the same disadvantage as in JS: once set, there is no way to remove them! This is a big drawback and we shouldn't let it "just happen" when (finally) we move to a more modern minimum PHP version :)

If you're using any lambda functions now I'd suggest gradually removing them. They make your code harder to read in 99% of the cases, usually are not documented, and almost always add unneeded/redundant levels of indentation. I know there are couple of well established coding "patterns" that use lambda functions in JS, but thinking we should strongly discourage them in all other cases.

        add_filter(
            'wc_memberships_for_teams_new_team_data',
            function( $data, $args ) {
                ...
                return $data;
            },
            10,
            2
        );

This can't be the intended result...?

Right. It is not. Try defining that function somewhere else and making the call to add_filter() on one line. I promise it will look so much better! :)

[johnbillion]

Frankly I thing we should "ban" lambda functions (closures) in add_filter() and add_action().

Let's keep this thread to one topic please.

[azaozz]

Right, sorry, opened #1486 for the lambda callbacks :)

[jrfnl]

Closing as duplicate of #1330. Let's keep the conversation in one place please.