Skip to content

#202 Add rule to prevent final constructors in doctrine entities #395

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

Merged
merged 3 commits into from
Apr 5, 2023
Merged

Conversation

LeoVie
Copy link
Contributor

@LeoVie LeoVie commented Nov 25, 2022

This fixes #202.

  • Prevent Doctrine entities from containing final constructors
  • At the same time, don't forbid non-constructor final methods in Doctrine entities

What do you think about that?

@LeoVie
Copy link
Contributor Author

LeoVie commented Nov 25, 2022

The failing checks don't seem to be related to the changes of this PR.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

Also, the rule needs to be registered in rules.neon, but only for bleedingEdge.

Thanks.

throw new ShouldNotHappenException();
}

return [sprintf(
Copy link
Member

Choose a reason for hiding this comment

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

Please use RuleErrorBuilder here.

return [];
}

$classReflection = $scope->getClassReflection();
Copy link
Member

Choose a reason for hiding this comment

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

You're reporting in all classes, not just entities.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm correct, this should work now only for entities. I've taken this code from EntityNotFinalRule for the detection, if the class is an entity

if ($this->objectMetadataResolver->isTransient($classReflection->getName())) {
	return [];
}

$metadata = $this->objectMetadataResolver->getClassMetadata($classReflection->getName());
if ($metadata !== null && $metadata->isEmbeddedClass === true) {
	return [];
}

@LeoVie
Copy link
Contributor Author

LeoVie commented Jan 27, 2023

Hi, sorry for the delay :)
I've updated the PR according to your suggestions. I hope, everything is correct now.

@LeoVie LeoVie requested a review from ondrejmirtes January 27, 2023 15:11
rules.neon Outdated
@@ -23,10 +23,13 @@ rules:
- PHPStan\Rules\Doctrine\ORM\DqlRule
- PHPStan\Rules\Doctrine\ORM\RepositoryMethodCallRule
- PHPStan\Rules\Doctrine\ORM\EntityNotFinalRule
- PHPStan\Rules\Doctrine\ORM\EntityConstructorNotFinalRule
Copy link
Member

Choose a reason for hiding this comment

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

You need to remove it from the rules section.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, okay. I removed it there. :)

@LeoVie LeoVie requested a review from ondrejmirtes February 24, 2023 10:32
@ondrejmirtes ondrejmirtes merged commit a1ba454 into phpstan:1.3.x Apr 5, 2023
@ondrejmirtes
Copy link
Member

Thank you!

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.

Implement a rule that prevents final constructors in Doctrine Entities
2 participants