Skip to content
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

Interfaces mis @throw annotations #7786

Open
hansbogert opened this issue Jul 31, 2019 · 6 comments
Open

Interfaces mis @throw annotations #7786

hansbogert opened this issue Jul 31, 2019 · 6 comments

Comments

@hansbogert
Copy link

hansbogert commented Jul 31, 2019

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
Copy link
Member

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: #7780

@Ocramius Ocramius added this to the 3.0 milestone Jul 31, 2019
@hansbogert
Copy link
Author

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
Copy link
Member

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

@Ocramius
Copy link
Member

See #6743

@derrabus
Copy link
Member

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.

@derrabus derrabus removed this from the 3.0.0 milestone May 11, 2022
@greg0ire
Copy link
Member

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

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

No branches or pull requests

4 participants