Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: add failing test case for bug #11857 #3599

Merged
merged 8 commits into from
Nov 6, 2024

Conversation

calebdw
Copy link
Contributor

@calebdw calebdw commented Nov 3, 2024

Adds a failing test for phpstan/phpstan#11857

@calebdw calebdw changed the base branch from 2.0.x to 1.12.x November 3, 2024 01:50
@calebdw calebdw marked this pull request as draft November 3, 2024 01:51
@ondrejmirtes ondrejmirtes force-pushed the bug-11857 branch 2 times, most recently from a450662 to bb5ef62 Compare November 6, 2024 10:25
@ondrejmirtes
Copy link
Member

Please see my commits. Can you grab the PHAR from https://github.com/phpstan/phpstan-src/actions/runs/11701663721, copy it into your vendor/phpstan/phpstan/ and test it thoroughly on Larastan and real-world projects to see if it fixes the issue? Thanks.

@ondrejmirtes
Copy link
Member

Also I modified the test expectation and added one ignore, please tell me if it's still okay.

@calebdw
Copy link
Contributor Author

calebdw commented Nov 6, 2024

@ondrejmirtes,

This fixed the relation errors, but it caused errors when using a trait with methods that return $this in a final class. I added another test file to demonstrate this failure

@ondrejmirtes ondrejmirtes marked this pull request as ready for review November 6, 2024 18:47
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

@ondrejmirtes
Copy link
Member

I'm pretty confident with the changes now.

@ondrejmirtes ondrejmirtes merged commit 6dcda72 into phpstan:1.12.x Nov 6, 2024
453 checks passed
@ondrejmirtes
Copy link
Member

Thank you.

@calebdw calebdw deleted the bug-11857 branch November 6, 2024 18:55
@calebdw
Copy link
Contributor Author

calebdw commented Nov 6, 2024

Thank you!!

@kayw-geek
Copy link

Hi @calebdw, Thank improve the generic relation to Relations! While enhancing generic relations for Relations, I encountered an issue: when a final parent class overrides method uses a PHPDoc return type with $this in a generic parameter, PHPStan a return type incompatibility.

Reproduction Code:

<?php

namespace Bug11857;

use function PHPStan\Testing\assertType;

abstract class Model {
    /** @return BelongsTo<*, *> */
    public function belongsTo(string $related): BelongsTo {
        return new BelongsTo();
    }
}

/**
 * @template TRelatedModel of Model
 * @template TDeclaringModel of Model
 */
class BelongsTo {}

class Post extends Model {
    /** @return BelongsTo<User, $this> */
    public function user(): BelongsTo {
        return $this->belongsTo(User::class);
    }
}

final class ChildPost extends Post {
    public function user(): BelongsTo {
        return $this->belongsTo(User::class);
    }
}

function test(ChildPost $child): void {
    assertType('Bug11857\BelongsTo<Bug11857\User, Bug11857\ChildPost>', $child->user());
}

Error Output:

------ ---------------------------------------------------------------
Line   test.php                                                       
------ ---------------------------------------------------------------
42     Return type (Bug11857\BelongsTo<Bug11857\User, $this(Bug11857\ChildPost)>) 
       of method Bug11857\ChildPost::user() should be compatible with 
       return type (Bug11857\BelongsTo<Bug11857\User, $this(Bug11857\Post)>) 
       of method Bug11857\Post::user()      
       🪪 method.childReturnType                                      
------ ---------------------------------------------------------------

PHPStan version is 2.1.6

@calebdw
Copy link
Contributor Author

calebdw commented Feb 26, 2025

@kayw-geek,

There's no reason for the child to override the parent user method:

class Post extends Model {
    /** @return BelongsTo<User, $this> */
    public function user(): BelongsTo {
        return $this->belongsTo(User::class);
    }
}
final class ChildPost extends Post {
}

However, please open new bug reports so we can better track them

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants