Skip to content
This repository was archived by the owner on Nov 21, 2019. It is now read-only.

Allow multi-line method chaining #7

Merged
merged 5 commits into from
Oct 2, 2018
Merged

Allow multi-line method chaining #7

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1558984
Allow multi-line method chaining
thewilkybarkid Sep 24, 2018
a296741
Merge branch 'master' into method-chaining
thewilkybarkid Oct 1, 2018
6013d07
CS fix
thewilkybarkid Oct 1, 2018
5c2b516
Live with the weird namespace
thewilkybarkid Oct 1, 2018
15802a4
Merge branch 'master' into method-chaining
thewilkybarkid Oct 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions Libero/Sniffs/WhiteSpace/ObjectStaticOperatorSpacingSniff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace Libero\CodingStandard\Sniffs\WhiteSpace;

use PHP_CodeSniffer\Standards\Squiz\Sniffs\WhiteSpace\ObjectOperatorSpacingSniff as BaseObjectOperatorSpacingSniff;
use const T_DOUBLE_COLON;

// This is only present to allow double colons to have separate configuration to the regular object operator.

final class ObjectStaticOperatorSpacingSniff extends BaseObjectOperatorSpacingSniff
{
public function register()
{
return [T_DOUBLE_COLON];
}
}
4 changes: 3 additions & 1 deletion src/Libero/ruleset.xml → Libero/ruleset.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
<exclude name="PSR2.Namespaces.UseDeclaration.SpaceAfterLastUse"/>
</rule>

<rule ref="PEAR.WhiteSpace.ObjectOperatorIndent"/>
<rule ref="SlevomatCodingStandard.Namespaces.UseDoesNotStartWithBackslash"/>
<rule ref="SlevomatCodingStandard.Namespaces.UseSpacing"/>
<rule ref="SlevomatCodingStandard.TypeHints.ParameterTypeHintSpacing"/>
Expand All @@ -21,10 +22,11 @@
<rule ref="Squiz.Classes.ClassFileName"/>
<rule ref="Squiz.WhiteSpace.ObjectOperatorSpacing">
<properties>
<property name="ignoreNewlines" value="false"/>
<property name="ignoreNewlines" value="true"/>
</properties>
</rule>
<rule ref="Squiz.WhiteSpace.OperatorSpacing"/>
<rule ref="Squiz.WhiteSpace.SemicolonSpacing"/>
<rule ref="Squiz.WhiteSpace.SuperfluousWhitespace">
<properties>
<property name="ignoreBlankLines" value="false"/>
Expand Down
5 changes: 5 additions & 0 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@
"description": "Libero PHP coding standard",
"type": "phpcodesniffer-standard",
"license": "MIT",
"autoload": {
"psr-4": {
"Libero\\CodingStandard\\": "Libero/"
}
},
"autoload-dev": {
"psr-4": {
"tests\\Libero\\CodingStandard\\": "tests/"
Expand Down
2 changes: 1 addition & 1 deletion phpcs.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

<rule ref="Libero"/>

<file>src/</file>
<file>Libero/</file>
Copy link
Contributor Author

@thewilkybarkid thewilkybarkid Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To avoid the namespace being Libero\CodingStandard\Libero. (Is there a way to have PHP_CodeSniffer work without having the ruleset's parent directory being the name? It's already defined in the ruleset itself...)

Copy link
Contributor Author

@thewilkybarkid thewilkybarkid Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, think we'll have to revert this. The sniff name ends up being CodingStandard. WhiteSpace.ObjectStaticOperatorSpacing rather than the expected Libero. WhiteSpace.ObjectStaticOperatorSpacing.

Copy link

@giorgiosironi giorgiosironi Oct 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the decision here in the end? Happy either way

<file>tests/</file>

<rule ref="PSR1.Methods.CamelCapsMethodName.NotCamelCaps">
Expand Down
2 changes: 1 addition & 1 deletion phpunit.xml.dist
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

<filter>
<whitelist>
<directory>src</directory>
<directory>Libero</directory>
</whitelist>
</filter>

Expand Down
29 changes: 20 additions & 9 deletions tests/cases/whitespace/method-call
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,25 +3,36 @@ Method calls must not have unnecessary whitespace
---CONTENTS---
<?php

$foo -> bar ( $arg1 );
$foo -> bar ( $arg1 ) -> baz ( );

$foo
-> bar ($arg1
,$arg2)
-> baz ($arg1
,$arg2);

$fooBar()
->baz()
->qux()
;

---FIXED---
<?php

$foo->bar($arg1);
$foo->bar($arg1)->baz();
Copy link
Contributor Author

@thewilkybarkid thewilkybarkid Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather these became multiline.

Copy link

@giorgiosironi giorgiosironi Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is reasonable for me to allow an existing single-line statement to remain single-line, while if you want to go multi-line then you would have to follow a standard.


$foo
->bar(
Copy link
Contributor Author

@thewilkybarkid thewilkybarkid Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would rather not keep the newline.

$arg1,
$arg2
)
->baz(
$arg1,
$arg2
);
Copy link
Contributor Author

@thewilkybarkid thewilkybarkid Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure about whether the semi-colon should go on the next line. Produces nicer diffs, but looks ugly.

Copy link

@stephenwf stephenwf Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah so this is enforced, even if you manually put it on the next line.

Copy link

@stephenwf stephenwf Sep 24, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diff might look something like this:

$foo
    ->bar(
        $arg1,
        $arg2
+    )
+    ->baz(
+        $arg1,
+        $arg2
     );

Copy link

@giorgiosironi giorgiosironi Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought semicolon-on-next-line was a Symfony standard by now

Copy link

@giorgiosironi giorgiosironi Sep 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd favor it not because of the diffs but because keeps lines independent (for fluent interfaces, which is the use case for chained calls):

  • add a new call
  • remove a call
  • change the order of calls
  • swap two calls
    become operations that only involve full lines


$foo->bar(
$arg1,
$arg2
)->baz(
$arg1,
$arg2
);
$fooBar()
->baz()
->qux();

---