Aliasing a trait method imports the method with the original name too

[dtdesign]

Description

Traits have support for aliasing as part of the conflict resolution strategy. This does not only allow to change visibility of the imported method but also to import it using a different.

Aliasing the method works as expected but the original method will be copied too. Unless the class defines the exact same method already (which overrules the trait’s method) the alias will cause two methods to be copied from the trait.

<?php

trait Foo {
    public function hello(): string
    {
        return "Hello from Foo!";
    }
}

class Bar {
    use Foo {
        hello as private somethingElse;
    }
}

$bar = new Bar();
var_dump($bar->hello());
// Throws
var_dump($bar->somethingElse());

Resulted in this output:

string(15) "Hello from Foo!"

Fatal error: Uncaught Error: Call to private method Bar::somethingElse() from global scope in /in/uAjcv:19
Stack trace:
#0 {main}
  thrown in /in/uAjcv on line 19

Process exited with code 255.

A live version is available at https://3v4l.org/uAjcv. It should be noted that the behavior for the visibility change works just fine, importing the method as protected or private will correctly change the visibility while maintaining the original method.

The issue at hand is that both the “original” method is copied over and its visibility remains unchanged.

Psalm correctly validates this behavior, but PHPStan incorrectly (up for discussion 😉) flags the call to the original method name as an error. I will open a bug report with PHPStan too.

PHP Version

8.4.3

Operating System

No response

[nielsdos]

The behaviour seems intentional. See section on "Renaming" from https://wiki.php.net/rfc/horizontalreuse:

One of the main issues of the Traits proposal has always been the fact, that aliasing does not mean renaming.

And it contains some motivation.
So this isn't a bug.
Changing this would also be a big BC break and would need an RFC. If you strongly feel like this needs to be changed, please contact the mailing list.

[dtdesign]

Thanks you @nielsdos for your swift reaction! Considering that this behavior exists for ages already and that it is described in the RFC as expected behavior, it is at least working the way its supposed to do. I would still argue that this is a design flaw but that is merely my personal view and it doesn’t cause conflicts if the method is defined locally, so I won’t pursue this any further.

That said, I believe this at least warrants a note in the official docs.

Rationale: The aliasing is something people are familiar from other languages (although usually in a different context) so this is at least unexpected behavior. For example, aliasing exists in TypeScript for the purpose of conflict resolution (colliding names) or to rename generic names to something more meaningful. In this case the original name is not available and the alias will be the only valid reference in that scope.

[nielsdos]

You can file a PR to the docs repo if you want to: https://github.com/php/doc-en/

[dtdesign]

I was thinking about the proper wording for said notice when I started thinking about possible side effects. After tormenting 3v4l.org for a bit, I came up with a test case where this implicit copy breaks the ability to import trait methods in a child class when a method with the same method exists in a parent class.

<?php
class Base {
    public function getAnswer(): string
    {
        return "42";
    }
}

trait ExampleTrait {
    public function getAnswer(): int
    {
        return 42;
    }
}

class Child extends Base {
    use ExampleTrait {
        getAnswer as private copiedMethod;
    }
}

This yields this error:

Fatal error: Declaration of ExampleTrait::getAnswer(): int must be compatible with Base::getAnswer(): string in /in/7KMs2 on line 10

Process exited with code 255.

You can find the code at https://3v4l.org/7KMs2. Strictly going by the section about “Conflict Resolution” in the docs, it only mentions the ability to resolve these conflicts for collisions within traits. However, this is a side-effect of how the aliasing works and not really about the conflict resolution.

Question: Should I submit this as a new bug report or is this part of the behavior that I should include in the PR for the docs?

Edit: The only solution for this scenario is to redefine the method with the exact signature in the child class:

// The rest of the code is above.

class Child extends Base {
    use ExampleTrait {
        getAnswer as private copiedMethod;
    }

    public function getAnswer(): string {
        return parent::getAnswer();
    }
}

https://3v4l.org/gOX9D

[nielsdos]

Question: Should I submit this as a new bug report or is this part of the behavior that I should include in the PR for the docs?

This is IMO a logical consequence of the behaviour that is currently in place, so not a bug, albeit weird.
One argument in favour is that functions in the trait that use another function can use the original name.
I suppose you could add such an example as a warning to the manual.

[dtdesign]

One argument in favour is that functions in the trait that use another function can use the original name.

That’s an excellent point that I haven’t though about!

This tickled some part in the back of brain because I know that trait methods are “weak” in that they can be overwritten by implementing class. LSP isn’t enforced which is okay but opens a whole new can of worms because I cannot make changes to traits that rely on calling these methods from within a trait:

<?php
trait ExampleTrait {
    public function hasAnswer(): bool
    {
        return $this->calculateResult() !== null;
    }
    
    private function calculateResult(): ?int
    {
        return 42;
    }
}

class Example {
    use ExampleTrait;
    
    private function calculateResult(int $a, int $b): int
    {
        return $a + $b;
    }
}

var_dump((new Example())->hasAnswer());

Results in

Fatal error: Uncaught ArgumentCountError: Too few arguments to function Example::calculateResult(), 0 passed in /in/UVBHm on line 5 and exactly 2 expected in /in/UVBHm:17
Stack trace:
#0 /in/UVBHm(5): Example->calculateResult()
#1 /in/UVBHm(23): Example->hasAnswer()
#2 {main}
  thrown in /in/UVBHm on line 17

Process exited with code 255.

You can see the live demo at https://3v4l.org/UVBHm. This is a bit of an odd example but just consider that the trait is provided by a library and in its original version there was only hasAnswer(). calculateResult() was added later but there was already such a method in the class Example.

Keeping the original method around appears to be a half-baked solution because it doesn’t enforce the LSP for these methods meaning the trait’s methods can’t rely on the signatures. Is this the expected behavior?

Rant: We’ve been using quite a lot of traits over the years and it feels like solving something with a trait is just turning one problem into two problems …

[nielsdos]

Keeping the original method around appears to be a half-baked solution because it doesn’t enforce the LSP for these methods meaning the trait’s methods can’t rely on the signatures. Is this the expected behavior?

I don't know. Maybe people have some niche use case for this that would break though if we start enforcing LSP.

Rant: We’ve been using quite a lot of traits over the years and it feels like solving something with a trait is just turning one problem into two problems …

I have little experience with them so I don't know what particular problems you face, they worked fine for when I needed them (which again isn't much). Although they are odd in some ways as we've seen in this issue.

[dtdesign]

@nielsdos I was about to follow this up with a PR to the doc repository until I noticed that this behavior is indeed documented and I somehow managed to completely miss it.

Thanks for taking your time to address my issue although it could have been avoided with better reading skills on my end. Your swift reactions were greatly appreciated though!

[nielsdos]

Honestly, I didn't check the manual thoroughly either 😅 . Good to know it's already documented, albeit a bit mentioned "in passing".
And no problem.