docs icon indicating copy to clipboard operation
docs copied to clipboard

Chain calls must be put on separate lines

Open arogachev opened this issue 3 years ago • 14 comments

The reason - readability. Suggested by @xepozz.

$object->method1()
    ->method2();

Even if there are only 2 methods and it fits on 1line. Or are there any exceptions? Not sure about $object->formatter->format() though.

If everyone is OK with that, I'll add it to the code style.

arogachev avatar Mar 28 '22 11:03 arogachev

Already checked and seems it can't be automated with StyleCI.

arogachev avatar Mar 28 '22 11:03 arogachev

I usually do it as:

$object
    ->method1()
    ->method2();

samdark avatar Mar 29 '22 20:03 samdark

I usually do it as:

$object
    ->method1()
    ->method2();

Me too, and also when chain is long sometime writes it as:

$object
    ->methot1()
    ->methot2()
    ->methot3()
    ->methot4()
    ->methot5()
;

to easy move/remove any method.

tomaszkane avatar Mar 29 '22 21:03 tomaszkane

This rule is overhead for me. Don't want to significantly expand the PSR-12.

Although, I prefer @samdark variant:

$object
    ->method1()
    ->method2();

vjik avatar Mar 30 '22 14:03 vjik

We may have it as a soft-rule with should instead of must.

samdark avatar Mar 30 '22 19:03 samdark

What about such inliners?

in array (from: yii3-app/config/routes.php):

return [
    Route::get('/')->action([SiteController::class, 'index'])->name('home'),
];

as variable (yii3-app/tests/Unit/HelloCest.php): $params = $this->getConfig()->get('params');

as return (yii3-app/tests/Support/_generated/UnitTesterActions.php): return $this->getScenario()->runStep(new \Codeception\Step\Action('assertEmpty', func_get_args()));

It will be more clear if it would be stated here as examples of different usage cases.

sankaest avatar May 12 '22 11:05 sankaest

Is this correct?

return [
    Route::get('/')
        ->action([SiteController::class, 'index'])
        ->name('home'),
];
$params = $this
    ->getConfig()
    ->get('params');
 return $this
    ->getScenario()
    ->runStep(new \Codeception\Step\Action('assertEmpty', func_get_args()));

sankaest avatar May 12 '22 18:05 sankaest

Yes, it is.

samdark avatar May 16 '22 19:05 samdark

More examples (I had during code review), are they correct?

one argument (single method) with concatenation:

    $this->assertGreaterThan($customerQuery->count() . " " . print_r($orders, true));

one argument (chain calls):

    $this->assertCount(
        $query
            ->createCommand()
            ->getRawSql() 
    );

multiple arguments (single/chained), with concatenation:

    $this->assertCount(
        1,
        $orders,
        $query
            ->createCommand()
            ->getRawSql() . 
        print_r($orders, true)
    );

chaining in if() with multiple conditions:

    if (
        $db
            ->getSchema()
            ->getTableSchema($tablename, true) === null
    ) {
        // code
    }

property after method:

    $this->assertFalse(
        $boolARQuery
            ->where(['bool_col' => false])
            ->one()->bool_col
    );

sankaest avatar May 17 '22 05:05 sankaest

In case somebody is searching for those issues and try to fix by themself, below is regex, what worked about 95% of situations (eg, will not match closure function in method).

OneLiners concatenated together by OR (|): ([a-zA-Z0-9$]|[ ])+->\w*\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(|\((new .*)\)\)->\w*\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(|([a-zA-Z0-9])+\:\:\w*\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(|[a-zA-Z0-9$]+->\w*\((([^\(\)]+|))\)\n[ ]*->[a-zA-Z0-9$]+\(|\((new .*)\)\)->\w*\((([^\(\)]+|))\)\n[ ]*->[a-zA-Z0-9$]+\(|[a-zA-Z0-9$]+\n[ ]*->[a-zA-Z0-9$]+\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(|\((new .*)\)\)\n[ ]*->[a-zA-Z0-9$]+\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(

Explanation

1.1 Search for single line multiple chain, no rounded brackets [^()] in selection. Pattern: $object->method1($attributes)->method2( Regex: ([a-zA-Z0-9$]|[ ])+->\w*\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(

1.2 The same as above, but for new objects. Pattern: (new Object())->method1($attributes)->method2( Regex: \((new .*)\)\)->\w*\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(

1.3 The same as 1.1, but for static methods Pattern: Object::method1($attributes)->method2( Regex: ([a-zA-Z0-9])+\:\:\w*\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(

2.1 Search for multiple line multiple chain (first method on the same line as object - chaining). Pattern:

$object->method1($attributes)
    ->method2(

Regex: [a-zA-Z0-9$]+->\w*\((([^\(\)]+|))\)\n[ ]*->[a-zA-Z0-9$]+\(

2.2 The same as above, but for new objects. Pattern:

(new Object())->method1($attributes)
    ->method2(

Regex: \((new .*)\)\)->\w*\((([^\(\)]+|))\)\n[ ]*->[a-zA-Z0-9$]+\(

3.1 Search for multiple line multiple chain (first method on the next line second method on the same line as first method - chaining). Pattern:

$object
    ->method1($attributes)->method2(

Regex: [a-zA-Z0-9$]+\n[ ]*->[a-zA-Z0-9$]+\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(

3.2 The same as above, but for new objects. Pattern:

(new Object())
    ->method1($attributes)->method2(

Regex: \((new .*)\)\)\n[ ]*->[a-zA-Z0-9$]+\((([^\(\)]+|))\)->[a-zA-Z0-9$]+\(

Use with care :)

sankaest avatar May 18 '22 13:05 sankaest

@sankaest merged all PRs opened so far. Thanks!

samdark avatar May 22 '22 09:05 samdark

Use with care! and save your time!

No one will use it. It can be implemented as a plugin to such tools as php-cs-fixer, rector, prettier or others.

xepozz avatar Jun 02 '22 19:06 xepozz

Well, if somebody will make a plugin I will be happy. At least until now, no changes were made and as @arogachev was pointed out: "Already checked and seems it can't be automated with StyleCI." And within two month, nobody made any changes according to this issue. But if somebody will make a plugin, let me know, so I could be happy. :) @xepozz can you make it?

sankaest avatar Jun 02 '22 20:06 sankaest

Well, if somebody will make a plugin I will be happy. At least until now, no changes were made and as @arogachev was pointed out: "Already checked and seems it can't be automated with StyleCI." And within two month, nobody made any changes according to this issue. But if somebody will make a plugin, let me know, so I could be happy. :) @xepozz can you make it?

I don't think so :)

StyleCI should be omitted as for me.

So with php-cs-fixer you can create your own rule. It's simple. https://github.com/FriendsOfPHP/PHP-CS-Fixer/blob/master/doc/custom_rules.rst

xepozz avatar Jun 03 '22 15:06 xepozz