doctrine / orm

Doctrine Object Relational Mapper (ORM)
https://www.doctrine-project.org/projects/orm.html
MIT License
9.94k stars 2.52k forks source link

Interfaces mis `@throw` annotations #7786

Open hansbogert opened 5 years ago

hansbogert commented 5 years ago

Bug Report

Q A
BC Break no
Version 2.6.3

Summary

Interfaces of, e.g., EntityManagerInterface do not include @throw annotations.

Current behavior

This causes IDEs to not notice that for example calls to EntityManagerInterface::flush may cause exceptions.

How to reproduce

Reference EntityManagerInterface in an IDE like PhpStorm or other, and have the following snippet

function foo(EntityManagerInterface $em) {
  $em->flush();
}

Expected behavior

I expect the IDE to be able to tell, based on docblocks of the Doctrine library, that we should catch, (or re-throw) the exceptions which can occur.

Ocramius commented 5 years ago

Documenting all @throws is not possible here, since there are multiple un-checked exception types upstream from that interface (in implementations).

You can gladly suggest those that are directly thrown in the EntityManager (implementation).

Related: https://github.com/doctrine/orm/issues/7780

hansbogert commented 5 years ago

Well if you mean by unchecked exceptions, runtime exceptions, then I totally agree.

So basically we'd want to add

    /**
     * @throws \Doctrine\ORM\OptimisticLockException If a version check on an entity that
     *         makes use of optimistic locking fails.
     * @throws ORMException
     * @throws UniqueConstraintViolationException
     */

right?

Ocramius commented 5 years ago

Something like that, but please look at the current 3.x work.

Ocramius commented 5 years ago

See https://github.com/doctrine/orm/pull/6743

derrabus commented 2 years ago

We're doing some housekeeping on the 3.0.0 milestone. Since nobody seems to be actively working on this topic I'm removing this PR from the 3.0.0 milestone.

That being said, the issue does not really look actionable at the moment. I'd be happy to discuss and merge any PR adding reasonable @throws annotations though. Also, if we take this topic seriously, we should think about some kind of tooling that tells us about correctness and completenes of such annotations.

greg0ire commented 1 year ago

@morozov mentioned https://phpstan.org/blog/bring-your-exceptions-under-control in https://github.com/doctrine/dbal/pull/5777, we might want to look into it.