@AutoValue is not compatible with validation 2.0's @Valid

[findepi]

Same as #288 but for Validation 2.0 (javax.validation:validation-api:2.0.1.Final) annotation.

Consider class:

@AutoValue
public abstract class MyClass
{
    @Valid
    @AutoValue.CopyAnnotations(exclude = Valid.class) // this is ineffective
    public abstract Optional<@NotEmpty String> getFoo();

    public static MyClass create(Optional<String> newFoo)
    {
        return new AutoValue_MyClass(newFoo);
    }
}

@CopyAnnotations works for validation 1.0 annotation but does not work for validation 2.0 (javax.validation:validation-api:2.0.1.Final).
There:

@Target({ METHOD, FIELD, CONSTRUCTOR, PARAMETER, TYPE_USE })
@Retention(RUNTIME)
@Documented
public @interface Valid {
}

@CopyAnnotations javadoc says that TYPE_USE annotations are copied regardless:

@Generated("com.google.auto.value.processor.AutoValueProcessor")
final class AutoValue_MyClass extends MyClass {
  ...
  @Override
  public @Valid Optional<String> getFoo() {
    return foo;
  }
  ....

This breaks validation (using Apache BVal):

javax.validation.ConstraintDeclarationException: Multiple method return values marked @Valid in hierarchy [Meta.ForMethod(public abstract java.util.Optional com.example.MyClass.getFoo()), Meta.ForMethod(public java.util.Optional com.example.AutoValue_MyClass.getFoo())]

	at org.apache.bval.jsr.metadata.Liskov.noRedeclarationOfReturnValueCascading(Liskov.java:172)
	at org.apache.bval.jsr.metadata.Liskov.validateContainerHierarchy(Liskov.java:118)
	at org.apache.bval.jsr.metadata.HierarchyBuilder$ForContainer.<init>(HierarchyBuilder.java:302)
	at org.apache.bval.jsr.metadata.HierarchyBuilder.forContainer(HierarchyBuilder.java:372)
	at org.apache.bval.jsr.metadata.CompositeBuilder$ForExecutable.getReturnValue(CompositeBuilder.java:204)
	at org.apache.bval.jsr.descriptor.MetadataReader$ForExecutable.getReturnValueDescriptor(MetadataReader.java:367)
	at org.apache.bval.jsr.descriptor.ExecutableD.<init>(ExecutableD.java:43)
	at org.apache.bval.jsr.descriptor.MethodD.<init>(MethodD.java:32)
	at org.apache.bval.jsr.descriptor.MetadataReader$ForBean.lambda$getMethods$6(MetadataReader.java:190)
	at java.util.HashMap.forEach(HashMap.java:1289)
	at org.apache.bval.jsr.descriptor.MetadataReader$ForBean.getMethods(MetadataReader.java:186)
	at org.apache.bval.jsr.descriptor.BeanD.<init>(BeanD.java:56)
	at org.apache.bval.jsr.descriptor.DescriptorManager.getBeanDescriptor(DescriptorManager.java:81)
	at org.apache.bval.jsr.job.ValidationJob.getBeanDescriptor(ValidationJob.java:577)
	at org.apache.bval.jsr.job.ValidationJob.access$200(ValidationJob.java:81)
	at org.apache.bval.jsr.job.ValidationJob$BeanFrame.<init>(ValidationJob.java:246)
	at org.apache.bval.jsr.job.ValidationJob$BeanFrame.<init>(ValidationJob.java:242)
	at org.apache.bval.jsr.job.ValidateBean.computeBaseFrame(ValidateBean.java:40)
	at org.apache.bval.jsr.job.ValidationJob.getResults(ValidationJob.java:555)
	at org.apache.bval.jsr.ValidatorImpl.validate(ValidatorImpl.java:53)

Expected behavior

com.google.auto.value.AutoValue.CopyAnnotations#exclude should be honored, so that user remains in control of the annotations output in the generated class.

[eamonnmcmanus]

To my mind the check that BVal is performing is not legitimate. The return type of the abstract method is @Valid Optional<@NotEmpty String>, and AutoValue is correctly copying that type to the implementation. If @Valid were supposed to apply to a single method in the hierarchy (so to the abstract method but not its override) then it would have METHOD but not TYPE_USE as its @Target.

I do think it's a bug that AutoValue isn't copying the @NotEmpty from the type argument of Optional.

[findepi]

@eamonnmcmanus it might be that my particular problem could be solved on bval's side alone. I don't know enough yet to judge this.

However, I see no reason why I, as an @AutoValue user, should be left with no control over annotations in the generated subclass. Especially when the mechanism to control these already exists -- @CopyAnnotations(exclude = ...).
In the code, I am saying explicitly "hey, @AutoValue, do not copy this annotation over there" and @AutoValue is saying "ekhm, I just know better, I choose to disobey you".

[eamonnmcmanus]

Right now AutoValue considers that type annotations on the return value of an abstract property method are part of the property type, and it copies them everywhere that type is mentioned in the generated code: on the return type of the implementing method, as we have seen, but also on the type of the private field that holds the property value, and on the type of the corresponding constructor parameter or builder field. We could extend the meaning of @CopyAnnotations so that it allows you to prevent an annotation from being copied in these contexts, though I'm concerned that the specification of @CopyAnnotations would then be even more complicated than it already is. If we don't do that then we should at least make it an error to try to exclude a type annotation. I agree that the current behaviour of just ignoring the exclusion is not good.

[tbroyer]

Given that annotations with both TYPE_USE and METHOD apply to both the return type and the method, how about honoring @CopyAnnotations(exclude = …) for those annotations?

But then of course, I too do think BVal is wrong (and/or it was a mistake to make @Valid a type annotation in Bean Validation 2.0; they probably should have added a new annotation instead that would have only been a type annotation, I suppose they did it that way for backwards compatibility…)

[eamonnmcmanus]

OK, I suppose it does make sense for AutoValue to omit an annotation that it has been explicitly told to, even when that annotation is a type annotation. I continue to think that for this to happen there must be a problem elsewhere, but I've made the change anyway and it will be present in the next release.