[DeadCode] Add RemoveDuplicatedReturnSelfDocblockRector to drop @return docblock duplicating native self/static

[TomasVotruba]

Adds a dedicated DeadCode rule RemoveDuplicatedReturnSelfDocblockRector that removes a @return docblock when it only duplicates the native self/static return type of the current object.

This was originally bundled into ReturnTypeFromStrictFluentReturnRector; moved here so the fluent rule stays focused on adding the return type, while docblock cleanup is a DeadCode concern. The fluent rule is left untouched.

Covers @return $this, @return self, @return static, and @return CurrentClass (the gap vs the existing RemoveUselessReturnTagRector is mainly @return $this).

 final class SomeClass
 {
-    /**
-     * @return $this
-     */
     public function some(): self
     {
         return $this;
     }
 }

[escopecz]

Your productivity is crazy! 🚀

[TomasVotruba]

Thank you! So is your merge rate 💯

[calebdw]

@TomasVotruba, @samsonasik this is breaking phpstan---@return $this DOES MATTER and means something very different than self or static. PHPStan treats them all very differenty

• self: method returns NEW instance of same class
• static: method returns NEW instance of same or child class
• $this: method returns EXACT same instance

(the gap vs the existing RemoveUselessReturnTagRector is mainly @return $this).

@return $this should NOT be removed! This makes this rule useless---I really need a rule that automatically ADDs @return $this to methods when it actually returns $this

[TomasVotruba]

Our PHPStan didn't notice a thing. Could you send a failing PHPStan demo link?

self and static do not explore inner way of constructing object, like other types. It could be new or same object.

This rule can be just skipped, that's why it's a separate rule 👍

[calebdw]

The errors I started getting mainly had to do with Larastan and builder/collection generics, and also undefined model attributes errors, because the builder/collection generics were no longer correct

• Parameter #1 $items of method ProductSort::items() expects Collection<int, Item>, Collection<int, Illuminate\\Database\\Eloquent\\Model> given.
• Access to an undefined property Illuminate\\Database\\Eloquent\\Model::$url.

Here's an example on the phpstan playground:

• https://phpstan.org/r/27dfec74-25c0-4080-83c8-59930e6196bf

Removing the @return $this: causes errors:

	/**
	 * @template TNew
	 * @param TNew $value
-	 * @return $this
	 * @phpstan-this-out static<TNew>
	 */
	public function wrap(mixed $value): static

• https://phpstan.org/r/9a17bb8f-4e3f-4e9c-8f6e-3fd5c9766f33

This rule can be just skipped, that's why it's a separate rule 👍

I (or anybody else, and this will absolutely affect Laravel users) shouldn't have to skip this in the first place---@return $this is NOT dead code and should NOT be removed! It provides MORE specific information to phpstan than self and static are not able to rely:

Important

A narrower @return $this instead of @return static can also be used, and PHPStan will check if you’re really returning the same object instance and not just an object of the child class.

Here's another example where PHPStan requires $this to be returned instead of a new instance:

• https://phpstan.org/r/1792234f-b8cb-4ff3-8ffe-e6c4949ab6fd

[TomasVotruba]

Just curious, is this GPT output or do you type all this? It's rather weird to see AI sloop/different style than I used to read from you. I prefer that :)

It seems related to @template logic, see:
https://phpstan.org/r/adb9a2f8-d0e1-47d7-a06e-19503641c33e

[calebdw]

Just curious, is this GPT output or do you type all this?

This is all me

It seems related to https://github.com/template logic, see:

No, there's other cases for @return $this than just template tags, I don't have the time to generate bespoke examples for every single use case---the bottom line is that it's not dead code and shouldn't be automatically removed.

If you're adamant about keeping this rule, then it should be removed from the default set and be made completely optional

[TomasVotruba]

Sorry, I just never so many CAPS used by human latelly.

This one reports error regardless docblock: https://phpstan.org/r/1792234f-b8cb-4ff3-8ffe-e6c4949ab6fd
I don't see any example that is related to bare removal of @return $this causing an issue. We'll need a simple reproducer before addressing this issue.

[calebdw]

https://phpstan.org/r/7abe5a21-8d30-4113-aaea-a976b962f60a

[TomasVotruba]

@calebdw No error with/without docblock.

https://phpstan.org/r/7abe5a21-8d30-4113-aaea-a976b962f60a

[calebdw]

@TomasVotruba,

The point is that there SHOULD be an error---the method indicates that the SAME instance should be returned, not a new instance:

• https://phpstan.org/r/508a92c1-7554-441d-add0-a57a9aa85223

Removing @return $this silences an important error which is dangerous:

CC: @ondrejmirtes, do you have any thoughts/opinions here? You designed it---should @return $this be considered dead code and automatically be removed?

[TomasVotruba]

This rule only removes docblock, it's not related to self/static choice.

[TomasVotruba]

Ideally send a failing test case fiture PR, I'm getting lost in this.