-
-
Notifications
You must be signed in to change notification settings - Fork 472
Fix detectMappingType if the entity contain a package attribute #2115
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
base: 2.18.x
Are you sure you want to change the base?
Changes from 3 commits
1e2cb74
440ac19
6eb59c2
0109efc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -418,8 +418,8 @@ private function detectMappingType(string $directory, ContainerBuilder $containe | |||||
} | ||||||
|
||||||
if ( | ||||||
preg_match('/^(?: \*|\/\*\*) @.*' . $quotedMappingObjectName . '\b/m', $content) | ||||||
|| preg_match('/^(?: \*|\/\*\*) @.*Embeddable\b/m', $content) | ||||||
self::textContainsAnnotation($quotedMappingObjectName, $content) | ||||||
|| self::textContainsAnnotation('Embeddable', $content) | ||||||
) { | ||||||
$type = 'annotation'; | ||||||
break; | ||||||
|
@@ -429,6 +429,21 @@ private function detectMappingType(string $directory, ContainerBuilder $containe | |||||
return $type; | ||||||
} | ||||||
|
||||||
/** | ||||||
* Check if the file content contains a class-like annotation | ||||||
* | ||||||
* @internal | ||||||
*/ | ||||||
public static function textContainsAnnotation(string $quotedMappingObjectName, string $content): bool | ||||||
{ | ||||||
return preg_match('/^(?:[ ]\*|\/\*\*)[ ]@ # Match phpdoc start or line with an at | ||||||
\\\\? # Can start with antislash | ||||||
([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*\\\\?)* # Match 0-n namespace, the antislash is optionnal as it may be a prefix if an alias is used | ||||||
|
([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*\\\\?)* # Match 0-n namespace, the antislash is optionnal as it may be a prefix if an alias is used | |
([a-zA-Z_\x80-\xff][a-zA-Z0-9_\x80-\xff]*\\\\?)* # Match 0-n namespace, the antislash is optional as it may be a prefix if an alias is used |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, what do you mean by "0-n namespace"? It seems you're saying that in Doctrine\DoctrineBundle
, DoctruneBundle
is a namespace, which it isn't, only Doctrine
and Doctrine\DoctrineBundle
are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that it, to say that it can be of namespace component like Doctrine
or DoctrineBundle
.
I can replace by Match namespace, the antislash is optional as it may be a prefix if an alias is used
if you prefer ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or "Match namespace components", just like you wrote above. That's fine.
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 for the part about the prefix thing. This leads to matching anyway annotation that has Entity
somewhere in its name).
We should keep checking for the Entity
word
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this does not make sense. Aliasing a class does not restrict it to apply a suffix anyway, and it would match unrelated things. This part should be reverted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea was to match also if you have :
use Doctrine\ORM\Mapping\Entity as ORMEntity;
use Doctrine\ORM\Mapping\Entity as EntityORM;
/** @EntityORM */
/** @ORMEntity */
class Something { ... }
But yes, if you alias to DatabaseObject
it would not match like the previous version.
Do you still want me to revert this part ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we want to matching these, we would have to match it only when there is no namespace part (we cannot have a namespace reference before an aliased name). And making it reliable would require inspecting use statements to detect such aliases, which is overkill IMO (if the detection fails, the dev can always provide a type explicitly)
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Fixtures\Bundles\AnnotationsBundle; | ||
|
||
use Symfony\Component\HttpKernel\Bundle\Bundle; | ||
|
||
class AnnotationsBundle extends Bundle | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Fixtures\Bundles\AnnotationsBundle\Entity; | ||
|
||
/** @ORM\Entity() */ | ||
class TestAnnotationEntity | ||
{ | ||
/** | ||
* @ORM\Id | ||
* @ORM\GeneratedValue(strategy="AUTO") | ||
* @ORM\Column(type="integer") | ||
*/ | ||
public int|null $id = null; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Fixtures\Bundles\AttributesWithPackageBundle; | ||
|
||
use Symfony\Component\HttpKernel\Bundle\Bundle; | ||
|
||
class AttributesWithPackageBundle extends Bundle | ||
{ | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Fixtures\Bundles\AttributesWithPackageBundle\Entity; | ||
|
||
/** @testUnrelatedAnnotation Fixtures\Bundles\AttributesWithPackageBundle\Entity */ | ||
interface TestInterface | ||
{ | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex comments are misaligned