8.3 - as final trait-used method does not correctly report visibility in Reflection

[ondrejmirtes]

Description

The following code:

<?php

trait SimpleTrait
{
    private function foo() {}
}


class TraitFixtureWithFinal
{
    use SimpleTrait {
        foo as final;
    }
}

$rm = new ReflectionMethod(TraitFixtureWithFinal::class, 'foo');
var_dump($rm->isFinal());
var_dump($rm->isPrivate());
var_dump($rm->isPublic());
var_dump($rm->isProtected());

Resulted in this output:

bool(true)
bool(false)
bool(false)
bool(false)

But I expected this output instead:

bool(true)
bool(true)
bool(false)
bool(false)

Alternatively, a syntax resulting in "final private" class method should be rejected because normally they're not allowed.

But the same bug applies to "protected" method - isProtected in ReflectionMethod is also false: https://3v4l.org/0PZvm#v8.3.0

PHP Version

PHP 8.3

Operating System

No response

[iluuu1994]

This should error, as combining final and private are a usually illegal combination. It seems the visibility flags aren't correctly initialized at all.

https://3v4l.org/UIrg1#v8.3.0

Method [ <user> final <visibility error> method foo ] {
  @@ /in/UIrg1 5 - 5
}

[ondrejmirtes]

Yeah, as I'm saying in the original post:

Alternatively, a syntax resulting in "final private" class method should be rejected because normally they're not allowed.

But reflection lies for protected too: https://3v4l.org/0PZvm#v8.3.0

[nielsdos]

The problem, and similar for the other issue, is that the following code makes an assumption that if any modifier is set it is always the PPP mask:
fn_copy.common.fn_flags = alias->modifiers | (fn->common.fn_flags & ~ZEND_ACC_PPP_MASK);

I think instead something like this is desirable:

if (alias->modifiers & ZEND_ACC_PPP_MASK) {
    fn_copy.common.fn_flags = alias->modifiers | (fn->common.fn_flags & ~ZEND_ACC_PPP_MASK);
} else {
    fn_copy.common.fn_flags = alias->modifiers | fn->common.fn_flags;
}

In this case if a visibility modifier is set we always take those (and other flags) from the modifier.
If it is not set we don't remove the visibility from fn_flags but instead just add the modifiers on top.

To make it error out we additionally need a check like if ((fn->common.fn_flags & ZEND_ACC_PRIVATE) && (fn->common.fn_flags & ZEND_ACC_FINAL)) somewhere.