Skip to content

add failing testcase for PropertyTypeDeclarationRector#1795

Closed
bendavies wants to merge 1 commit intorectorphp:masterfrom
bendavies:PropertyTypeDeclarationRector-doctrine-importer-failing-testcase
Closed

add failing testcase for PropertyTypeDeclarationRector#1795
bendavies wants to merge 1 commit intorectorphp:masterfrom
bendavies:PropertyTypeDeclarationRector-doctrine-importer-failing-testcase

Conversation

@bendavies
Copy link
Copy Markdown
Contributor

I tried out https://www.tomasvotruba.cz/blog/2019/07/29/how-we-completed-thousands-of-missing-var-annotations-in-a-day/
Thanks, this is great!

Here's a failing testcase for PropertyTypeDeclarationRector with doctrine relations.

Doctrine does not require you to set fully quantified class names for relation classes - it will look at the class use statements.

@bendavies bendavies force-pushed the PropertyTypeDeclarationRector-doctrine-importer-failing-testcase branch from 5b65fbf to cbdd92e Compare August 3, 2019 20:43
{
/**
* @ORM\OneToMany(targetEntity="Product")
* @var Product[]|\Doctrine\Common\Collections\Collection
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These should be FQN, so the annotation output is consistent

class Company
{
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These should be in /Source.

/Fixture directory is only for tested code (a code that will be changed)

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Aug 3, 2019

I tried out tomasvotruba.cz/blog/2019/07/29/how-we-completed-thousands-of-missing-var-annotations-in-a-day
Thanks, this is great!

Glad you like it and thanks for fast testing 👍

Doctrine does not require you to set fully quantified class names for relation classes - it will look at the class use statements.

I didn't know that, thanks for the test!

@snapshotpl
Copy link
Copy Markdown
Contributor

Looks like I was little faster #1793 :)

@TomasVotruba
Copy link
Copy Markdown
Member

Oh, I didn't realize it's the same issue :) allright then

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Aug 3, 2019

@bendavies Partially covered by #1793, but this PR better covers case like aliases and one to many, so I'd like to merge this one as well.

Just use FQN in fixed code and it's ready to merge

private $company;

/**
* @ORM\OneToOne(targetEntity="Entity\Company")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Accoding to Doctrine docs this can't work, as it is in a different namespace.

See: https://www.doctrine-project.org/projects/doctrine-orm/en/2.6/reference/annotations-reference.html#manytoone

image

@TomasVotruba
Copy link
Copy Markdown
Member

TomasVotruba commented Aug 4, 2019

Closing as all the working features are already included in #1793

Thank you for pointing out the weak spot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants