guewin commented on code in PR #191: URL: https://github.com/apache/bval/pull/191#discussion_r2098237674
########## bval-jsr/src/main/java/org/apache/bval/cdi/BValExtension.java: ########## @@ -132,28 +134,31 @@ public <A> void processAnnotatedType(final @Observes ProcessAnnotatedType<A> pat final int modifiers = javaClass.getModifiers(); if (!javaClass.isInterface() && !javaClass.isAnonymousClass() && !Modifier.isFinal(modifiers) && !Modifier.isAbstract(modifiers)) { try { - Queue<Class<?>> toProcess = new LinkedList<>(); - toProcess.add(annotatedType.getJavaClass()); + Queue<AnnotatedType<?>> toProcess = new LinkedList<>(); + toProcess.add(annotatedType); while (!toProcess.isEmpty()) { - Class<?> now = toProcess.poll(); - Executable[] methods = now.getMethods(); - Executable[] constructors = now.getConstructors(); + AnnotatedType<?> now = toProcess.poll(); + Set<? extends AnnotatedMethod<?>> methods = now.getMethods(); + Set<? extends AnnotatedConstructor<?>> constructors = now.getConstructors(); if (hasValidation(now) - || hasValidation(methods) || hasValidation(constructors) - || hasParamsWithValidation(methods) || hasParamsWithValidation(constructors)) { + || methods.stream().anyMatch(this::hasValidation) + || constructors.stream().anyMatch(this::hasValidation) + || methods.stream().anyMatch(this::hasParamsWithValidation) + || constructors.stream().anyMatch(this::hasParamsWithValidation)) { pat.setAnnotatedType(new BValAnnotatedType<>(annotatedType)); - break; } - // Nothing found, collect superclass/interface and repeat (See BVAL-222) - if (now.getSuperclass() != Object.class && now.getSuperclass() != null) { - toProcess.add(now.getSuperclass()); + Class<?> superclass = now.getJavaClass().getSuperclass(); Review Comment: If I may add my 2 cents. I would also interpret the spec in the same way as @rmannibucau , in essence, Liskovs Principle has to be followed and precondition weakening should be possible. Thus, for retaining an interface method constraint, the implementation also has to declare the very same constraint, otherwise its a valid constraint weakening and the parent constraint should be ignored. The rules from ยง5.6.5 seem to be ambiguous or incomplete for this case, and should be clarified. In this regard, issues like https://issues.apache.org/jira/projects/TOMEE/issues/TOMEE-4449 seem to be not-spec-compliant. How about a configurable switch? Like already proposed in https://issues.apache.org/jira/projects/BVAL/issues/BVAL-175. It seems like BVal considered the parent method constraints till version 2.0.2, so keeping this behaviour configurable, but have the spec compliant way as default, i.e. parent constraints are not considered to support the weakening case. Since both ways also differ in performance, a switch looks like a good compromise, so that user can choose by themselves what they prefer. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@bval.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org