diff --git a/core-processor/src/main/java/io/micronaut/inject/ast/ConstructorElement.java b/core-processor/src/main/java/io/micronaut/inject/ast/ConstructorElement.java index 9fa039961e4..4ac2176019d 100644 --- a/core-processor/src/main/java/io/micronaut/inject/ast/ConstructorElement.java +++ b/core-processor/src/main/java/io/micronaut/inject/ast/ConstructorElement.java @@ -41,6 +41,11 @@ default boolean hides(MemberElement memberElement) { return false; } + @Override + default boolean hides(MethodElement hiddenMethod) { + return false; + } + @Override default boolean overrides(MethodElement overridden) { return false; diff --git a/core-processor/src/main/java/io/micronaut/inject/ast/MemberElement.java b/core-processor/src/main/java/io/micronaut/inject/ast/MemberElement.java index 026b75dfc34..8c85366b24a 100644 --- a/core-processor/src/main/java/io/micronaut/inject/ast/MemberElement.java +++ b/core-processor/src/main/java/io/micronaut/inject/ast/MemberElement.java @@ -76,17 +76,7 @@ default boolean isReflectionRequired() { * @since 3.4.0 */ default boolean isReflectionRequired(@NonNull ClassElement callingType) { - if (isPublic()) { - return false; - } - if (isPackagePrivate() || isProtected()) { - // the declaring type might be a super class in which - // case if the super class is in a different package then - // the method or field is not visible and hence reflection is required - final ClassElement declaringType = getDeclaringType(); - return !declaringType.getPackageName().equals(callingType.getPackageName()); - } - return isPrivate(); + return !isAccessible(callingType, false); } /** @@ -104,6 +94,61 @@ default boolean isAccessible() { return isAccessible(getOwningType()); } + /** + * Returns whether this member element can be invoked or retrieved at runtime. + * It can be accessible by a simple invocation + * + *

This method uses {@link #isReflectionRequired()} with a checks if the reflection access is allowed. + * By checking for {@link io.micronaut.core.annotation.ReflectiveAccess} annotation. + *

+ * + * @param callingType The calling type + * @param allowReflection If reflection invocation can be used + * @return Will return {@code true} if is accessible. + * @since 4.3.0 + */ + default boolean isAccessible(@NonNull ClassElement callingType, boolean allowReflection) { + if (isPublic()) { + return true; + } + if (isProtected()) { + // the declaring type might be a super class in which + // case if the super class is in a different package then + // the method or field is not visible and hence reflection is required + final ClassElement declaringType = getDeclaringType(); + String packageName = declaringType.getPackageName(); + if (!packageName.equals(callingType.getPackageName())) { + return allowReflection && hasAnnotation(ReflectiveAccess.class); + } + return true; + } + if (isPackagePrivate()) { + // the declaring type might be a super class in which + // case if the super class is in a different package then + // the method or field is not visible and hence reflection is required + final ClassElement declaringType = getDeclaringType(); + String packageName = declaringType.getPackageName(); + if (!packageName.equals(callingType.getPackageName())) { + return allowReflection && hasAnnotation(ReflectiveAccess.class); + } + if (isPackagePrivate()) { + // Check if there is a subtype that breaks the package friendship + ClassElement superClass = getOwningType(); + while (superClass != null && !superClass.equals(declaringType)) { + if (!packageName.equals(superClass.getPackageName())) { + return allowReflection && hasAnnotation(ReflectiveAccess.class); + } + superClass = superClass.getSuperType().orElse(null); + } + } + return true; + } + if (isPrivate()) { + return allowReflection && hasAnnotation(ReflectiveAccess.class); + } + return true; + } + /** * Returns whether this member element can be invoked or retrieved at runtime. * It can be accessible by a simple invocation or a reflection invocation. @@ -117,7 +162,7 @@ default boolean isAccessible() { * @since 3.7.0 */ default boolean isAccessible(@NonNull ClassElement callingType) { - return !isReflectionRequired(callingType) || hasAnnotation(ReflectiveAccess.class); + return isAccessible(callingType, true); } @Override diff --git a/core-processor/src/main/java/io/micronaut/inject/ast/MethodElement.java b/core-processor/src/main/java/io/micronaut/inject/ast/MethodElement.java index c4764c8bad3..7498392ea0f 100644 --- a/core-processor/src/main/java/io/micronaut/inject/ast/MethodElement.java +++ b/core-processor/src/main/java/io/micronaut/inject/ast/MethodElement.java @@ -20,6 +20,7 @@ import io.micronaut.core.annotation.AnnotationValue; import io.micronaut.core.annotation.AnnotationValueBuilder; import io.micronaut.core.annotation.Experimental; +import io.micronaut.core.annotation.NextMajorVersion; import io.micronaut.core.annotation.NonNull; import io.micronaut.core.annotation.Nullable; import io.micronaut.core.util.ArgumentUtils; @@ -266,84 +267,83 @@ default boolean isVarArgs() { * @return true if this overrides passed method element * @since 3.1 */ + @NextMajorVersion("Review the package-private methods with broken access, those might need to be excluded completely") default boolean overrides(@NonNull MethodElement overridden) { - if (this.equals(overridden) || isStatic() || overridden.isStatic()) { + if (equals(overridden) || isStatic() || overridden.isStatic() || isPrivate() || overridden.isPrivate()) { return false; } - ClassElement thisType = getDeclaringType(); - ClassElement thatType = overridden.getDeclaringType(); - if (thisType.getName().equals(thatType.getName())) { - return false; - } - if (!thisType.isAssignable(thatType)) { - // not a parent class + if (!isSubSignature(overridden)) { return false; } MethodElement newMethod = this; if (newMethod.isAbstract() && !newMethod.isDefault() && (!overridden.isAbstract() || overridden.isDefault())) { return false; } - if (!newMethod.getName().equals(overridden.getName()) || overridden.getParameters().length != newMethod.getParameters().length) { - return false; - } - for (int i = 0; i < overridden.getParameters().length; i++) { - ParameterElement existingParameter = overridden.getParameters()[i]; - ParameterElement newParameter = newMethod.getParameters()[i]; - ClassElement existingType = existingParameter.getGenericType(); - ClassElement newType = newParameter.getGenericType(); - if (!newType.isAssignable(existingType)) { + if (isPackagePrivate() && overridden.isPackagePrivate()) { + // This is a special case for identical package-private methods from the same package + // if the package is changed in the subclass in between; by Java rules those methods aren't overridden + // But this kind of non-overridden method in the subclass CANNOT be called by the reflection, + // it will always call the subclass method! + // In Micronaut 4 we mark this method as overridden, in the future we might want to exclude them completely + if (!getDeclaringType().getPackageName().equals(overridden.getDeclaringType().getPackageName())) { return false; } } - ClassElement existingReturnType = overridden.getReturnType().getGenericType(); - ClassElement newTypeReturn = newMethod.getReturnType().getGenericType(); - if (!newTypeReturn.isAssignable(existingReturnType)) { + // Check if a parent class + return getDeclaringType().isAssignable(overridden.getDeclaringType()); + } + + @Override + default boolean hides(@NonNull MemberElement memberElement) { + if (memberElement instanceof MethodElement hidden) { + return hides(hidden); + } + return false; + } + + /** + * Checks if this member element hides another. + * + * @param hiddenMethod The possibly hidden method + * @return true if this member element hides passed field element + * @since 4.3.0 + */ + default boolean hides(@NonNull MethodElement hiddenMethod) { + if (equals(hiddenMethod) || isStatic() || hiddenMethod.isStatic() || hiddenMethod.isPrivate()) { return false; } - // Cannot override existing private/package private methods even if the signature is the same - if (overridden.isPrivate()) { + if (!isSubSignature(hiddenMethod)) { return false; } - if (overridden.isPackagePrivate()) { - return newMethod.getDeclaringType().getPackageName().equals(overridden.getDeclaringType().getPackageName()); + if (!getDeclaringType().isAssignable(hiddenMethod.getDeclaringType())) { + // not a parent class + return false; } - return true; + if (hiddenMethod.isPackagePrivate()) { + return getDeclaringType().getPackageName().equals(hiddenMethod.getDeclaringType().getPackageName()); + } + return hiddenMethod.isAccessible(getDeclaringType(), false); } - @Override - default boolean hides(@NonNull MemberElement memberElement) { - if (memberElement instanceof MethodElement hidden) { - if (equals(hidden) || isStatic() || hidden.isStatic() || hidden.isPrivate()) { - return false; - } - MethodElement newMethod = this; - if (!newMethod.getName().equals(hidden.getName()) || hidden.getParameters().length != newMethod.getParameters().length) { - return false; - } - for (int i = 0; i < hidden.getParameters().length; i++) { - ParameterElement existingParameter = hidden.getParameters()[i]; - ParameterElement newParameter = newMethod.getParameters()[i]; - ClassElement existingType = existingParameter.getGenericType(); - ClassElement newType = newParameter.getGenericType(); - if (!newType.isAssignable(existingType)) { - return false; - } - } - if (!getDeclaringType().isAssignable(memberElement.getDeclaringType())) { - // not a parent class - return false; - } - ClassElement existingReturnType = hidden.getReturnType().getGenericType(); - ClassElement newTypeReturn = newMethod.getReturnType().getGenericType(); - if (!newTypeReturn.isAssignable(existingReturnType)) { + /** + * Is this method a sub signature of another. + * @param element The other method + * @return true if a sub signature + * @since 4.3.0 + */ + default boolean isSubSignature(MethodElement element) { + if (!getName().equals(element.getName()) || element.getParameters().length != this.getParameters().length) { + return false; + } + for (int i = 0; i < element.getParameters().length; i++) { + ParameterElement existingParameter = element.getParameters()[i]; + ParameterElement newParameter = getParameters()[i]; + ClassElement existingType = existingParameter.getGenericType(); + if (!newParameter.getGenericType().isAssignable(existingType)) { return false; } - if (hidden.isPackagePrivate()) { - return newMethod.getDeclaringType().getPackageName().equals(hidden.getDeclaringType().getPackageName()); - } - return memberElement.isAccessible(getDeclaringType()); } - return false; + return getReturnType().getGenericType().isAssignable(element.getReturnType().getGenericType()); } /** diff --git a/inject-java/src/main/java/io/micronaut/annotation/processing/visitor/JavaMethodElement.java b/inject-java/src/main/java/io/micronaut/annotation/processing/visitor/JavaMethodElement.java index 98ca59b2309..9aef0d2deb9 100644 --- a/inject-java/src/main/java/io/micronaut/annotation/processing/visitor/JavaMethodElement.java +++ b/inject-java/src/main/java/io/micronaut/annotation/processing/visitor/JavaMethodElement.java @@ -22,7 +22,6 @@ import io.micronaut.core.util.CollectionUtils; import io.micronaut.inject.ast.ClassElement; import io.micronaut.inject.ast.GenericPlaceholderElement; -import io.micronaut.inject.ast.MemberElement; import io.micronaut.inject.ast.MethodElement; import io.micronaut.inject.ast.ParameterElement; import io.micronaut.inject.ast.PrimitiveElement; @@ -36,6 +35,7 @@ import javax.lang.model.element.TypeElement; import javax.lang.model.element.VariableElement; import javax.lang.model.type.DeclaredType; +import javax.lang.model.type.ExecutableType; import javax.lang.model.type.TypeKind; import javax.lang.model.type.TypeMirror; import javax.lang.model.type.WildcardType; @@ -162,28 +162,43 @@ public boolean isVarArgs() { @Override public boolean overrides(MethodElement overridden) { - if (this.equals(overridden) || isStatic() || overridden.isStatic()) { + if (equals(overridden) || isStatic() || overridden.isStatic() || isPrivate() || overridden.isPrivate()) { return false; } if (overridden instanceof JavaMethodElement javaMethodElement) { - boolean overrides = visitorContext.getElements().overrides( + if (isPackagePrivate() && overridden.isPackagePrivate()) { + // Test special case of the default methods + return MethodElement.super.overrides(overridden); + } + return visitorContext.getElements().overrides( executableElement, javaMethodElement.executableElement, owningType.classElement ); - if (overrides) { - return true; - } } return MethodElement.super.overrides(overridden); } @Override - public boolean hides(MemberElement hidden) { + public boolean isSubSignature(MethodElement element) { + if (element instanceof JavaMethodElement javaMethodElement) { + return visitorContext.getTypes().isSubsignature( + (ExecutableType) executableElement.asType(), + (ExecutableType) javaMethodElement.executableElement.asType() + ); + } + return MethodElement.super.isSubSignature(element); + } + + @Override + public boolean hides(MethodElement hiddenMethod) { if (isStatic() && getDeclaringType().isInterface()) { return false; } - return visitorContext.getElements().hides(getNativeType().element(), ((JavaNativeElement.Method) hidden.getNativeType()).element()); + if (hiddenMethod instanceof JavaMethodElement javaMethodElement) { + return visitorContext.getElements().hides(getNativeType().element(), javaMethodElement.getNativeType().element()); + } + return MethodElement.super.hides(hiddenMethod); } @NonNull diff --git a/inject-java/src/test/groovy/io/micronaut/inject/beans/BeanDefinitionSpec.groovy b/inject-java/src/test/groovy/io/micronaut/inject/beans/BeanDefinitionSpec.groovy index f2973776b1b..b43447f1a3e 100644 --- a/inject-java/src/test/groovy/io/micronaut/inject/beans/BeanDefinitionSpec.groovy +++ b/inject-java/src/test/groovy/io/micronaut/inject/beans/BeanDefinitionSpec.groovy @@ -10,6 +10,7 @@ import io.micronaut.core.type.GenericPlaceholder import io.micronaut.inject.BeanDefinition import io.micronaut.inject.qualifiers.Qualifiers import spock.lang.Issue +import test.another.BeanWithPackagePrivate class BeanDefinitionSpec extends AbstractTypeElementSpec { @@ -668,6 +669,40 @@ interface AInterface { arguments[1].type == Long } + void "test package-private methods with different package are marked as overridden"() { + when: + def ctx = ApplicationContext.builder().build().start() + BeanDefinition definition = buildBeanDefinition('test.another.Test', ''' +package test.another; + +import test.Middle; +import jakarta.inject.Singleton; + +@Singleton +class Test extends Middle { + public boolean root; + void injectPackagePrivateMethod() { + root = true; + } +} + +''') + + def bean1 = ctx.getBean(BeanWithPackagePrivate) + def bean2 = ctx.getBean(definition) + then: """By Java rules the base method is not overriden and should have been injected too, but it's not possible to invoked using the reflection, +so we mark it as overridden +""" + !bean1.@root + bean1.@middle + !bean1.@base + !bean2.@root + bean2.@middle + !bean2.@base + cleanup: + ctx.close() + } + void "test repeatable inner type annotation 1"() { when: def ctx = ApplicationContext.builder().properties(["repeatabletest": "true"]).build().start() diff --git a/inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy b/inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy index cf3f4b790b6..d0fe0ec5a9f 100644 --- a/inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy +++ b/inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy @@ -48,6 +48,27 @@ import java.util.function.Supplier class ClassElementSpec extends AbstractTypeElementSpec { + void "test package-private methods with broken different package"() { + when: + ClassElement classElement = buildClassElement(''' +package test.another; + +import test.Middle; + +class Test extends Middle { + private boolean testInjected; + void injectPackagePrivateMethod() { + testInjected = true; + } +} + +''') + + def elements = classElement.getEnclosedElements(ElementQuery.ALL_METHODS) + then: "A special case for the indentical methods with package-private access and broken package access in between" + elements.size() == 2 + } + void "test class element generics"() { given: ClassElement classElement = buildClassElement(''' diff --git a/inject-java/src/test/java/test/Middle.java b/inject-java/src/test/java/test/Middle.java new file mode 100644 index 00000000000..91fd32b55aa --- /dev/null +++ b/inject-java/src/test/java/test/Middle.java @@ -0,0 +1,12 @@ +package test; + +import jakarta.inject.Inject; +import test.another.Base; + +public class Middle extends Base { + public boolean middle; + @Inject + void injectPackagePrivateMethod() { + middle = true; + } +} diff --git a/inject-java/src/test/java/test/another/Base.java b/inject-java/src/test/java/test/another/Base.java new file mode 100644 index 00000000000..04fa09e7e0f --- /dev/null +++ b/inject-java/src/test/java/test/another/Base.java @@ -0,0 +1,11 @@ +package test.another; + +import jakarta.inject.Inject; + +public class Base { + public boolean base; + @Inject + void injectPackagePrivateMethod() { + base = true; + } +} diff --git a/inject-java/src/test/java/test/another/BeanWithPackagePrivate.java b/inject-java/src/test/java/test/another/BeanWithPackagePrivate.java new file mode 100644 index 00000000000..f0fe56aa842 --- /dev/null +++ b/inject-java/src/test/java/test/another/BeanWithPackagePrivate.java @@ -0,0 +1,12 @@ +package test.another; + +import jakarta.inject.Singleton; +import test.Middle; + +@Singleton +public class BeanWithPackagePrivate extends Middle { + public boolean root; + void injectPackagePrivateMethod() { + root = true; + } +} diff --git a/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/AbstractKotlinMethodElement.kt b/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/AbstractKotlinMethodElement.kt index ab4b3889e29..896a0601766 100644 --- a/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/AbstractKotlinMethodElement.kt +++ b/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/AbstractKotlinMethodElement.kt @@ -120,6 +120,10 @@ internal abstract class AbstractKotlinMethodElement( // not sure how to implement this correctly for Kotlin false + override fun hides(hiddenMethod: MethodElement?) = + // not sure how to implement this correctly for Kotlin + false + override fun getName() = name override fun getOwningType() = owningType diff --git a/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/AbstractKotlinPropertyAccessorMethodElement.kt b/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/AbstractKotlinPropertyAccessorMethodElement.kt index b0ddc9c6d0c..225442fb831 100644 --- a/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/AbstractKotlinPropertyAccessorMethodElement.kt +++ b/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/AbstractKotlinPropertyAccessorMethodElement.kt @@ -62,4 +62,8 @@ internal abstract class AbstractKotlinPropertyAccessorMethodElement