PHP Magic Methods - false positive

[stodorovic]

eg. Visual Composer reports

ERROR Visibility for magic method __sleep must be public. Found: private

Code example:

        /**
         * Cloning disabled
         */
        private function __clone() {
        }

        /**
         * Serialization disabled
         */
        private function __sleep() {
        }

        /**
         * De-serialization disabled
         */
        private function __wakeup() {
        }

They methods are private and it's common practice for blocking clone/serialize. It's strange because it affects only __sleep. Also, some other plugins are passing tests. I'll try to find more examples.

[jrfnl]

See: http://php.net/manual/en/language.oop5.magic.php#object.sleep, the method must be public. You can still have a void function to disable it.

[stodorovic]

I know, but it's common practice in many plugins/other code. Here is an example for singleton which uses some developers: http://www.phptherightway.com/pages/Design-Patterns.html

It's only issue in Visual Composer. Also, clone/wakeup are private, but they don't report error.

Second example from Slider revolution:

        /**
         * No cloning allowed
         */
        private function __clone() {}

it isn't reported as error.

I know that both plugin works on PHP7 and it's confirmed from their developers. Also, it's total strange behaviour .

Easy Social Share Buttons for WordPress

        /**
         * Cloning disabled
         */
        private function __clone() {
        }

        /**
         * Serialization disabled
         */
        private function __sleep() {
        }

        /**
         * De-serialization disabled  
         */
        private function __wakeup() {
        }

it again reports only __sleep

FILE: ..../wp-content/plugins/easy-social-share-buttons3/easy-social-share-buttons3.php
------------------------------------------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
------------------------------------------------------------------------------------------------------------------
 147 | ERROR | Visibility for magic method __sleep must be public. Found: private
------------------------------------------------------------------------------------------------------------------

[jrfnl]

I know, but it's common practice in many plugins/other code.

Common practice does not make it valid code.

Also, clone/wakeup are private, but they don't report error.

Those methods don't have a visibility requirement. See:

• http://php.net/manual/en/language.oop5.cloning.php#object.clone
• http://php.net/manual/en/language.oop5.magic.php#object.wakeup

[stodorovic]

I totally agree with you. Many developers use dirty tricks and various workarounds. I don't like them. I'm trying to report all possible issues to support, but someone accepts it, someone don't even answer. (catastrophic support for some plugins, but they are very popular)

Also, anyone which want to create clear code, can use:

public function __sleep() {
    // cannot be serialized
    trigger_error( ' may not be serialized', E_USER_ERROR );
}

eg. W3TC is buggy in new PHP versions, reports a lot of notice/warnings in the logs and it still has million installs.

[jrfnl]

I'd avoid using E_USER_ERROR in this case, throwing an Exception might be more appropriate ?

[stodorovic]

I forgot. Yes, it's better to use throw new Exception('may not be serialized'); I think that works on all PHP 5.x versions ?

[jrfnl]

It does: http://php.net/manual/en/language.exceptions.php