Skip to content

Conversation

@neellii
Copy link

@neellii neellii commented Oct 16, 2025

refs: #203

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds model relations property annotations to Laravel models by generating @property annotations for relationships. The implementation adds support for both single and collection-based relationships with appropriate type hints.

  • Adds @property annotations for model relationships in generated models
  • Imports Illuminate\Database\Eloquent\Collection when collection-type relations are present
  • Generates proper type hints for singular (Model|null) and plural (Collection|Model) relationships

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

File Description
stubs/model.blade.php Adds conditional import for Collection class when collection-type relations exist
src/Generators/ModelGenerator.php Implements logic to generate relation annotations and determine Collection import necessity
tests/fixtures/ModelGeneratorTest/*.php Test fixtures showing expected output with relation property annotations

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

foreach ($relations as $relation) {
$relation = class_basename($relation);

$result[$this->getRelationName($relation, $type)] = $this->getRelationType($relation, $type);
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The method getRelationName() is being called but is not defined in this class. This will cause a fatal error when the code is executed.

Copilot uses AI. Check for mistakes.
@neellii neellii assigned neellii and unassigned DenTray Oct 16, 2025
protected function getRelationType(string $relation, string $type): string
{
if (in_array($type, self::PLURAL_NUMBER_REQUIRED)) {
return "Collection|$relation";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return "Collection|$relation";
return "Collection<{$relation}>";

Comment on lines 211 to 213
$relation = class_basename($relation);

$result[$this->getRelationName($relation, $type)] = $this->getRelationType($relation, $type);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's try to refactor it to prevent code duplication (the same logic already exists in the prepareRelations)

@neellii neellii assigned DenTray and unassigned neellii Oct 17, 2025
@neellii neellii assigned neellii and DenTray and unassigned DenTray and neellii Oct 20, 2025
@DenTray DenTray requested a review from Copilot October 20, 2025 06:38
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

'User.php' => file_get_contents(getcwd() . '/tests/Support/Models/WelcomeBonus.php'),
'Forum/Author.php' => file_get_contents(getcwd() . '/tests/Support/Models/WelcomeBonus.php'),
];
if (!empty ($models)) {
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the space between !empty and the opening parenthesis. The correct syntax is !empty($models).

Suggested change
if (!empty ($models)) {
if (!empty($models)) {

Copilot uses AI. Check for mistakes.
'entity' => $this->model,
]);

// TODO: use ronasit/larabuilder instead
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra spaces before the TODO comment. Use consistent indentation with // followed by a single space.

Suggested change
// TODO: use ronasit/larabuilder instead
// TODO: use ronasit/larabuilder instead

Copilot uses AI. Check for mistakes.
$import = "use {$import};";

if (!Str::contains($classContent, $import)) {
// TODO: use ronasit/larabuilder instead
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra spaces before the TODO comment. Use consistent indentation with // followed by a single space.

Suggested change
// TODO: use ronasit/larabuilder instead
// TODO: use ronasit/larabuilder instead

Copilot uses AI. Check for mistakes.
{
$annotation = "* @property {$propertyDataType} \${$propertyName}";

// TODO: use ronasit/larabuilder instead
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove the extra spaces before the TODO comment. Use consistent indentation with // followed by a single space.

Suggested change
// TODO: use ronasit/larabuilder instead
// TODO: use ronasit/larabuilder instead

Copilot uses AI. Check for mistakes.
@neellii neellii assigned neellii and unassigned DenTray Oct 23, 2025
@neellii neellii assigned DenTray and unassigned neellii Oct 23, 2025
protected function getRelationType(string $model, string $relation): string
{
if (in_array($relation, self::PLURAL_NUMBER_REQUIRED)) {
return "Collection<int, $model>";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return "Collection<int, $model>";
return "Collection<{$model}>";

* @property Carbon $published_at
* @property array $meta
* @property Comment|null $comment
* @property Collection<int, User> $users
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @property Collection<int, User> $users
* @property Collection<User> $users


//TODO: add @property annotation for each model's field
/**
* @property Collection<int, User> $users
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @property Collection<int, User> $users
* @property Collection<User> $users


/**
* @property string $title
* @property Collection<int, Author> $authors
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @property Collection<int, Author> $authors
* @property Collection<Author> $authors

* @property Carbon $published_at
* @property array $meta
* @property Comment|null $comment
* @property Collection<int, User> $users
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @property Collection<int, User> $users
* @property Collection<User> $users


//TODO: add @property annotation for each model's field
/**
* @property Collection<int, Category> $categories
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @property Collection<int, Category> $categories
* @property Collection<Category> $categories

public function testAddPropertyAnnotationToRelatedModel()
{
$this->mockFilesystem([
'Post.php' => $this->getFixture('new_model_without_fields.php'),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may we just add the Post model to the mockFilesystem method?

}

public function mockFilesystem(): void
public function mockFilesystem(array $models = []): void
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's implement it as separate methods

  1. public function mockFilesystem(array $models = []): void
  2. public function mockDefaultFilesystem(): void

Copy link
Contributor

@AZabolotnikov AZabolotnikov Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neellii @DenTray

public function mockFilesystem(array $models = []): void

Are you sure about this?

MB: public function mockFilesystem(array $models): void

@DenTray DenTray assigned neellii and unassigned DenTray Oct 23, 2025
@neellii neellii assigned DenTray, neellii and AZabolotnikov and unassigned neellii and DenTray Oct 24, 2025
public function mockFilesystem(): void
public function mockFilesystem(array $models = []): void
{
$fileSystemMock = new FileSystemMock;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$fileSystemMock = new FileSystemMock;
$fileSystemMock = new FileSystemMock();

ref: https://www.php-fig.org/psr/psr-12/#4-classes-properties-and-methods

public function mockDefaultFilesystem(): void
{
$fileSystemMock = new FileSystemMock();
$fileSystemMock = new FileSystemMock;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
$fileSystemMock = new FileSystemMock;
$fileSystemMock = new FileSystemMock();

ref: https://www.php-fig.org/psr/psr-12/#4-classes-properties-and-methods

@neellii neellii assigned AZabolotnikov and unassigned neellii Oct 28, 2025
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.

4 participants