Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Chain calls must be put on separate lines #155

Closed
arogachev opened this issue Mar 28, 2022 · 14 comments
Closed

Chain calls must be put on separate lines #155

arogachev opened this issue Mar 28, 2022 · 14 comments
Assignees
Labels
status:ready for adoption Feel free to implement this issue. type:docs Documentation

Comments

@arogachev
Copy link
Contributor

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 arogachev self-assigned this Mar 28, 2022
@arogachev arogachev added the type:docs Documentation label Mar 28, 2022
@arogachev
Copy link
Contributor Author

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

@samdark
Copy link
Member

samdark commented Mar 29, 2022

I usually do it as:

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

@tomaszkane
Copy link

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.

@vjik
Copy link
Member

vjik commented Mar 30, 2022

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

Although, I prefer @samdark variant:

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

@samdark
Copy link
Member

samdark commented Mar 30, 2022

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

@samdark samdark added status:ready for adoption Feel free to implement this issue. and removed status:under discussion labels Mar 30, 2022
@sankaest
Copy link

sankaest commented May 12, 2022

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
Copy link

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()));

@samdark
Copy link
Member

samdark commented May 16, 2022

Yes, it is.

@sankaest
Copy link

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
    );

@xepozz
Copy link
Contributor

xepozz commented Jun 2, 2022

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.

@sankaest
Copy link

sankaest commented Jun 2, 2022

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?

@xepozz
Copy link
Contributor

xepozz commented Jun 3, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:ready for adoption Feel free to implement this issue. type:docs Documentation
Projects
None yet
Development

No branches or pull requests

6 participants