unwanted errors magical methods in trait

[rino1]

PREMISE

To simplify things, only the __invoke() method will be considered.

Here's my scenario

+ tree
├── MyTrait.php
├── use.php
└── use-as-public.php



+ cat MyTrait.php
<?php
trait MyTrait {
    private function __invoke(){}
    
    //protected function __invoke(){} // present the same problems

    /*
     * other magical methods
     * (as __toString(), __clone(), __debugInfo(), etc.)
     * present the same problems
     */
}



+ cat use.php
<?php require "MyTrait.php";
new class {
    use MyTrait;
};



+ cat use-as-public.php
<?php require "MyTrait.php";
new class {
    use MyTrait {
        __invoke as public;
    }
};

Let's do some tests

CASE 1

+ php MyTrait.php
PHP Warning:  The magic method MyTrait::__invoke() must have public visibility in MyTrait.php on line 3

But we're in a trait, and in my opinion, certain things should be evaluated differently than a class.
The error we saw shouldn't appear!!!
I would limit the checking to syntax errors only: I would only do the more refined checks within a class.

CASE 2

+ php use.php
PHP Warning:  The magic method MyTrait::__invoke() must have public visibility in MyTrait.php on line 3

If anything (in accordance with what was said above)

PHP Warning: The magic method class@anonymous::__invoke() must have public visibility in use.php on line 3

CASE 3

+ php use-as-public.php
PHP Warning:  The magic method MyTrait::__invoke() must have public visibility in MyTrait.php on line 3

If anything (in accordance with what was said above)

PHP Warning: The magic method class@anonymous::__invoke() must have public visibility in use-as-public.php on line 3

but, in reality, I also consider the error just seen to be unwanted, in fact the appropriate adaptation has been made

+ grep 'as public' use-as-public.php
        __invoke as public;

and is no need to add anything else: the code speaks for itself.

CONCLUSION

In a future version of PHP, I think certain aspects should be revised.
What do you think?

Would anyone be willing to create a dedicated official RFC?
https://wiki.php.net/rfc/howto

[Girgias]

I am struggling to see the advantage of emitting warnings or compilation errors when useing the trait rather than when defining the trait itself.

As these warnings and compilation errors would be triggered for each use rather than once. Nor do I think we should complicate the "copy pasting" code of traits into classes more, as it is already far from simple.

[iluuu1994]

It's would be sound to skip the visibility check for traits, but I'd also question: Why? If you have to upgrade the visibility when using the trait, what's the benefit of allowing lower visibility in traits? (Gina beat me to it. 🙂)

[rino1]

It's would be sound to skip the visibility check for traits, but I'd also question: Why? If you have to upgrade the visibility when using the trait, what's the benefit of allowing lower visibility in traits? (Gina beat me to it. 🙂)

<?php // Filter.php
trait Filter {
    /**
      It's repetitive

      $ grep -i filter Filter.php | cat -n
        1	trait Filter {
        2	    private function filter(){}

      there are 2 occurrences: class name and method name
     */
    private function filter(){}
    
    /**
      It's not repetitive, but I absolutely have to make it public.
      If it's just a helper method, it's not recommended.
     */
    private function __invoke(){}
}

You could write a song

Filter... filter... filter... :)

[iluuu1994]

Not sure what this means. 🙂 You're technically correct that relaxing this warning would be sound, but there doesn't seem to be a convincing reason for this change. If you have one, please start a discussion on the internals mailing list. https://www.php.net/mailing-lists.php

[rino1]

You're technically correct that relaxing this warning would be sound, but there doesn't seem to be a convincing reason for this change.

Evidently the essence of my contribution did not arrive 🤷‍♂️

[iluuu1994]

Feel free to reiterate. If I misunderstood I can reopen.

[rino1]

$ tree
├── a.php
└── b.php



$ cat a.php 
<?php

trait Filter {
    private function filter() {
        //do something
    }
}

(new class {
    use Filter {
        filter as private filterPrivate;
    }

    public function filter() {
        //do something
        $this->filterPrivate();
        //do something
    }
})->filter();



$ cat b.php 
<?php

trait Filter {
    private function __invoke() {
        //do something
    }
}

(new class {
    use Filter {
        __invoke as private __invokePrivate;
    }

    public function __invoke() {
        //do something
        $this->__invokePrivate();
        //do something
    }
})();

Summarizing

$ grep -in filter a.php 
3:trait Filter {
4:    private function filter() {
10:    use Filter {
11:        filter as private filterPrivate;
14:    public function filter() {
16:        $this->filterPrivate();
19:})->filter();

Too many redundancies.
Too many repetitiveness.
In short, too many F/filter

$ grep -in filter b.php 
3:trait Filter {
10:    use Filter {

This is better, but

$ php -l b.php 
PHP Warning:  The magic method Filter::__invoke() must have public visibility in b.php on line 4

:(