phpcq / coding-standard

This repository holds the C-C-A coding standard definitions for phpcs and phpmd.
BSD 3-Clause "New" or "Revised" License
1 stars 2 forks source link

Add check for use of static:: instead of self:: #4

Closed discordier closed 8 years ago

discordier commented 9 years ago

As we all should be using PHP 5.4 we might want to add a check that no self is used. As a base we could use: https://github.com/dereuromark/cakephp-codesniffer/blob/master/Vendor/PHP/CodeSniffer/Standards/MyCakePHP/Sniffs/PHP/LateStaticBindingSniff.php

tristanlins commented 9 years ago

I'm not sure about this. Late static binding is not necessary at all, e.g. for constants. And for private static functions it does not work.

$ php -r '
class A {
    private static function foo() { echo "foo".PHP_EOL; }
    public static function bar() { static::foo(); }
}
class B extends A {
    private static function foo() { echo "bar".PHP_EOL; }
}
B::bar();'

Fatal error: Call to private method B::foo() from context 'A' in Command line code on line 1

Call Stack:
    0.0003     233944   1. {main}() Command line code:0
    0.0003     233976   2. A::bar() Command line code:1

Using self::foo(); still works.

discordier commented 9 years ago

Good point but this proves a broken class design. :) So we do not have any way to test if it works but if it breaks, it is almost guaeanteed a problem in the implementation.

tristanlins commented 9 years ago

Good point but this proves a broken class design. :) ...it is almost guaeanteed a problem in the implementation.

Only if you force to use static::... instead of self::... :-P Private static methods are necessary if you provide static methods and want to reduce their complexity -> split into multiple static functions. I realy not see the point why we enforce to use static::... instead of self::...?!

discordier commented 9 years ago

Your implementation bears the problem that you are reintroducing a private method which is already called in the parent class. This is bad design as you are masking the existing method. In strict programming languages the compiler is already warning, if not forbids, you to implement such a pattern. When you want to reintroduce, use protected methods. Do not implement the methods named the same as privates in derived classes. This will almost ever cause confusion as you have to know which method is private, in which class from where called. Using protected you know which one will get called. Using static you know you are always using the derived one. But on the overall problem, I only wanted to place this at discussion as I found this sniff in the CakePHP style and wondered if it would make sense for US to also implement it.

tristanlins commented 9 years ago

Your implementation bears the problem that you are reintroducing a private method which is already called in the parent class. This is bad design as you are masking the existing method.

This is wrong. Private methods are "local" for the defining class. I must not check, if "my" local private method is "overwriting" a parent method. At least because overwriting privates is not possible. That is no masking or overwriting, even no IDE will complain you about writing the same private method in an extending class because it is valid by design.

This will almost ever cause confusion as you have to know which method is private, in which class from where called.

It cannot confuse, because privates can only called from the defining class. Never ever from another class.

But on the overall problem, I only wanted to place this at discussion ...

I must say, my example is still valid, but a rare edge case. It only affect private static methods, called with self::method(). Personally I don't see any use cases for such code design ^^