OrderedInterfacesFixer - Introduction

[dmvdbrugge]

Fixes #4108

[julienfalque]

Great stuff 👍

[keradus]

I let myself resolve the conflicts, readme was not regenerated after changes

[keradus]

This rule is not safe.
I tried to apply it to our own codebase:

ker@dus:~/github/PHP-CS-Fixer λ git ds src/AbstractFixer.php
diff --git a/src/AbstractFixer.php b/src/AbstractFixer.php
index 44a07d83..af0cb1da 100644
--- a/src/AbstractFixer.php
+++ b/src/AbstractFixer.php
@@ -33,7 +33,7 @@
  *
  * @internal
  */
-abstract class AbstractFixer implements FixerInterface, DefinedFixerInterface
+abstract class AbstractFixer implements DefinedFixerInterface, FixerInterface
 {

ker@dus:~/github/PHP-CS-Fixer λ vendor/bin/phpunit 
PHP Fatal error:  Class PhpCsFixer\AbstractFixer cannot implement previously implemented interface PhpCsFixer\Fixer\FixerInterface in /home/d.ruminski/github/PHP-CS-Fixer/src/AbstractFixer.php on line 36

It has to at least be marked as risky

[dmvdbrugge]

I will mark as risky. However, in the case of our own code base, isn't it bad practise anyway to have this interface-duplication? I mean, DefinedFixerInterface extends FixerInterface, why implements them both? (Also: why does php break one way but not the other? Strange...)

[SpacePossum]

isn't it bad practise anyway to have this interface-duplication?

it doesn't harm to be explicit, it could also be because it makes merging over the various branches more easy, but maybe it is (indeed) overcomplete and can be cleaned up some day in another PR

why does php break one way but not the other? Strange...

I have no idea, it really puzzles me!, sadly it does break, so please update to be risky

[keradus]

I wouldn't consider it a bad idea. If you want to implement interfaces A and B, you may not know, not control, or even not care internal detail that B also implement A under the hood. Yet, this rule can break the code in current PHP engine, so we need risky flag with good explanation to the user why is it so.

[dmvdbrugge]

@keradus all changes have been processed (like, 3 months ago 😜) could you update your review?

[SpacePossum]

Thank you @dmvdbrugge.

[keradus]

all changes have been processed (like, 3 months ago stuck_out_tongue_winking_eye) could you update your review?

post mortem, @dmvdbrugge - pushing fixes to PR doesn't mark the PR as changed for repo maintainer (or at least to me). without new comment (!= commit), it's hard to notice that sth changed