MethodSpec.overriding shouldn't copy annotations

[gk5885]

For a few reasons:

• It's not necessarily semantically correct. @Nullable, for example, might apply to the method being overridden, but not necessarily to the implementation of the override.
• It doesn't necessarily compile. A private annotation on the method being overridden nested in the same type that declares the method is not accessible from a different type that might override the method.

[sormuras]

Two valid points, Gregory.

What do you guys prefer:

• replace annotation copy code by just adding an @Override tag
• guard current implementation by boolean switches like:

Builder overriding(ExecutableElement method, boolean copyMethodAnnotations, boolean copyParameterAnnotations) {

• or fix by exposing Builder state fields and let the caller decide which annotation(s) to kick out? See access to TypeSpec.Builder state (modifiable TypeSpec) #478 and Methods to access the state of Builders, for lazy members creation #341 Add fields() and methods() accessors to TypeSpec.Builder #341 #342 and others.

[gk5885]

My vote is for the first one. It's the simple, safe, predictable behavior.

The second one, is unlikely to be all that useful. You have to know something about the annotations on the method you're overriding in order to be able to predict if copying them is safe/correct. If you've come that far you might as well just put them on the overriding method yourself. There might be a version that took a Predicate or something that might be useful, but that seems like overkill to me.

The third assumes that a blacklist (rather than a whitelist) is what you're going to want. I think more often than not, it might be the other way around, but I think a Predicate or something similar might work better for that too.

[HenryLoenwind]

Is it only me who finds it really annoying that copying a parameter WITH annotations now is a major effort that involves research into how to do that? VariableElements are not the easiest thing to work with...

Aside from that, I find the behaviour of ParamSpec.get() very unexpected. I would have expected it to give me an object that contains the same information as its parameter, not some stripped down version. It's more ParamSpec.getSomethingThatLooksABitLike() now...

PS: I'm a bit annoyed because I had to find out why my code wasn't working from reading comments in yours. Not even in a javadoc, but hidden away inside the method.