PSR-19 Prevent PHPDoc inheritance

[Magnar Ovedal Myrtveit]

According to the current version of PSR-19, PHPDoc is implicitly inherited, and even when PHPDoc is present in both the subclass and the superclass, tags that are present in the superclass' PHPDoc but not in the subclass' PHPDoc are inherited.

I think this is a very good idea, but I see a potential issue. Consider the following code.

interface FooInterface {

/**

* @throws RuntimeException If the operation fails.

*/

public function foo();

}

class FooImplementation implements FooInterface {

public function foo() {

// Do something that will never fail.

}

}

FooImplementation::foo() will never throw an exception, but FooInterface allows other implementations of FooInterface::foo() to throw exceptions. For someone depending on FooInterface, they will have to handle any thrown exceptions. For someone depending on FooImplementation, however, they do not have to care about FooImplementation::foo() throwing exceptions. But there seems to be no way to prevent FooImplementation::foo() from inheriting the @throws tag from FooInterface::foo().

Do we need a way to specify tags that should not be inherited?

[Nicholas Ruunu]

A lot of people, me included, try not to handle exceptions at all.

But if you need to handle them, it's probably a good idea to do so even if the specific implementation doesn't throw.

Since otherwise you'd have a problem if you switch to an implementation that do throw in the future.

[Robbie Averill]

Good afternoon,

It seems to me that your interface says that it throws an exception "if an operation fails." If your implementation has no way of failing, then the fact that it never throws an exception is not of consequence. In future your implementation may change and may fail, in which case you have an exception you can throw without breaking the public API definition.

TL;DR: I don't see a problem with your example code.

Regards

Robbie

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/b1619215-f3e3-4865-9465-c5f2346a9536%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Robbie Averill | Senior Developer

04 978 7330

http://silverstripe.com/

[Alexandru Pătrănescu]

Hi,

I think it's a valid point of view.

Simple example I can think of is that develop might want to use reuse the simple implementation that never fails instead of the interface in a private method/class.

And he knows that it will not fail so it should not be hinted by editor to handle the exception.

Alex

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CANv6TC0PVd0enH6Euey11pd0%2BhGo7m4AkueN5CTVM_gJaoXvCg%40mail.gmail.com.

[Marcos Passos]

I'm with Magna on this.

There are some classes that to do not thrown exceptions by design. Think about a loader interface, that can throw a LoadingException. A StringLoader will never throw an exception, and any class tightly coupled to it should not care about LoadingException. It's an explicit design decision.

- Marcos

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAAwdEzDjkq6_YgTVSGGQnNVi0a%2BsmLWVp9NC1OYPaejjW%2BKhZQ%40mail.gmail.com.

[Woody Gilk]

On Mon, Nov 26, 2018 at 6:56 AM Marcos Passos <marcospa...@gmail.com> wrote:

Think about a loader interface, that can throw a LoadingException. A StringLoader will never throw an exception, and any class tightly coupled to it should not care about LoadingException.

In what situation would you have code tightly coupled to the implementation instead of the interface?

--
Woody Gilk
https://shadowhand.me

[Alexandru Pătrănescu]

A RemoteStringLoader implementation that will fetch the string from a remote location and then use StringLoader by composition.

Alex

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAGOJM6JABnTv0%3DO4OCGNUMs_2YPR4pxe%3DBXg1j5%2B2ayNWHgBZg%40mail.gmail.com.

[Nicholas Ruunu]

Yeah, that's a valid point.

In PHPStorm you can just overwrite `@throws` with nothing, or with anything that's not an exception like `-` and it won't squawk.

What about just doing something like that?

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/CAAwdEzDXSt7DxNJYDzmA8bTv5nnxxL2jmdOHR2pdXUj2qSbiow%40mail.gmail.com.

[Daniel Hunsaker]

Reading through this, I couldn't help but think the example was getting in the way of the suggestion, here. There are any number of tags, besides `@throws`, which could benefit from being removed from child implementations without outright replacing them, given the right use cases. Removal of optional parameters. Dropping attributions without providing one's own. Eliminating `@todo` entries which aren't applicable to the implementation. Any of these for an `extends` rather than an `implements`. And so on.

I mean, maybe I misunderstood the original suggestion itself, but it seemed like it covered myriad scenarios, rather than just the specific example.

- Dan

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/etPan.5bfc0739.14709aeb.259%40ruu.nu.

[Miguel Rosales]

Personally, I think that if a class is an implementation of an interface, their methods' docblocks should stick to the interface methods' docblocks as they are, because that fits better with the purpose behind using interfaces in 1st place, and that's what I always do in my code...

That being said, I'm not sure how you'd implement "eliminating @todo entries which aren't applicable to the implementation", because note that you can have multiple @todo entries in the interface! how do you identify which one is the one you want to eliminate?
For something like @throws, it's the same thing, although at least you can identify each tag by the exception they throw - anyway, you'd need something like a new "negation tag" like @not-throws (?), so you can override each specific exception that might be defined in the implementation... or am I missing something?

As I said, *personally* I don't consider this an important feature to focus on...

Daniel Hunsaker wrote on 28/11/2018 17:04:

Personally, I think that if a class is an implementation of an interface, their methods' docblocks should stick to the interface methods' docblocks as they are, because that fits better with the purpose behind using interfaces in 1st place, and that's what I always do in my code...

That being said, I'm not sure how you'd implement "eliminating @todo entries which aren't applicable to the implementation", because note that you can have multiple @todo entries in the interface! how do you identify which one is the one you want to eliminate?
For something like @throws, it's the same thing, although at least you can identify each tag by the exception they throw - anyway, you'd need something like a new "negation tag" like @not-throws (?), so you can override each specific exception that might be defined in the implementation... or am I missing something?

As I said, *personally* I don't consider this an important feature to focus on...

[Chuck Burgess]

I'm -1 on this... it seems too edge casey to me to come up with a new complex way to handle it.

CRB
about.me/ashnazg

On Wed, Nov 28, 2018 at 11:44 AM Miguel Rosales <m.rosale...@gmail.com> wrote:

(I think I've sent this message to 3 wrong places now - trying again... 🤦)

Personally, I think that if a class is an implementation of an interface, their methods' docblocks should stick to the interface methods' docblocks as they are, because that fits better with the purpose behind using interfaces in 1st place, and that's what I always do in my code...

That being said, I'm not sure how you'd implement "eliminating @todo entries which aren't applicable to the implementation", because note that you can have multiple @todo entries in the interface. How do you identify which one is the one you want to eliminate?
For something like @throws, it's the same thing, although at least you can identify each tag by the exception they throw - anyway, you'd need something like a new "negation tag" like @not-throws (?), so you can override each specific exception that might be defined in the implementation... no?

As I said, *personally* I don't consider this an important feature to focus on...    

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/851aff0f-3e55-4ed8-94da-c8aa0c20dce2%40googlegroups.com.

[Magnar Ovedal Myrtveit]

@Daniel Hunsaker Yes, the idea was a generic solution that would cover all tags, not just @throws.

@Miguel Rosales Possibility to prevent inheritance of individual entries of a tag seems overkill to me, and it would lead to a major change in the PSR. Currently either all entries of a tag are inherited, or no entries of the tag are inherited. At least this is the case when you interpret "part" as "all entries of a tag":

If a PHPDoc does not feature a part, such as Summary or Description, that is present in the PHPDoc of a super-element, then that part is always implicitly inherited.

interface FooInterface {

/**

* Any part that is not present in a subclass is inherited.

*
* @todo Bla bla bla.
* @todo Alb alb alb.

* @throws OutOfBoundsException ...

* @throws OverflowException ...

*/

public function foo();

}

class FooImplementation implements FooInterface {

/**

* Since this PHPDoc contains a @todo part, the @todo part is not inherited from the superclass.

* Since this PHPDoc contains no @throws part, the @throws part is inherited from the superclass.

*

* @todo Test test test.

* @todo Bla bla bla. Must be specified again, even though it is the same as in the superclass.

*/

public function foo() {

// Do something.

}

}

So if a subclass should contain anything other than exactly the same entries of a tag as the superclass, all the entries of the tag must be specified. There are only two options:

• Specify all entries of the tag explicitly
• Inherit all entries of the tag implicitly

I think the suggestion from @Nicholas Ruunu sounds reasonable, as it does not introduce any new tags and it follows the current definition: "@someTag -" prevents @someTag from being implicitly inherited since the part now is present. The "-" must be treated as a special thing, as it means that the tag does not apply. Since there may be tags that only consist of the tag name "-" is needed, as in that case "@someTag" cannot be used to prevent inheritance of the tag without also saying that @someTag applies in the current context. This solution might not be entirely transparent, unfortunately.

It might be more transparent if we introduce something like "@!someTag", or "@not-someTag" to prevent inheritance.

Another option that leads to a bigger change, would be to no longer do implicit inheritance, and instead allow inheritance of individual tags:

• @inheritDoc: Inherit everything.
• @inheritThrows Inherit the @throws part.
• @inheritTodo: Inherit the @todo part.

- Magnar

[Magnar Myrtveit]

phpstan-exception-rules has implemented support `@throws void` to disable inheritance of the `@throws` annotation: https://github.com/pepakriz/phpstan-exception-rules/commit/702688c127a8b86e4f9c19feae25dc86f6135ec6

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/fcbe0f73-dc7e-420c-9c7e-649d89730db4%40googlegroups.com.

[Ben Mewburn]

Hello,

One solution here is to remove @throws from the list of tags that must be inherited.

I guess the aim of having @throws inherited is like Java checked exceptions?

Though, haven't other languages like C# avoided this because it creates just as many problems as it solves?

[Nils Rückmann]

That's an issue, indeed. I've already thought about it and the only solution I can think of is to mark types explicitly (see jsdoc: `{int}`). This would also allow renderers and IDEs to link types even if they are part of the description. But this is another topic.

--

You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/f4624339-bfe7-4ad8-a7f0-3bc3f067bb32%40googlegroups.com.

[Nils Rückmann]

Sorry, wrong thread.