Skip to content

Commit

Permalink
Use Javac's overrides to determine if the method is overridden (#10281
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@dstepanov
dstepanov authored Dec 18, 2023
1 parent e3066c4 commit 4947687
Show file tree
Hide file tree
Showing 12 changed files with 244 additions and 76 deletions.
5 changes: 5 additions & 0 deletions core-processor/src/main/java/io/micronaut/inject/ast/ConstructorElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
69 changes: 57 additions & 12 deletions core-processor/src/main/java/io/micronaut/inject/ast/MemberElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand All @@ -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
*
* <p>This method uses {@link #isReflectionRequired()} with a checks if the reflection access is allowed.
* By checking for {@link io.micronaut.core.annotation.ReflectiveAccess} annotation.
* </p>
*
* @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.
Expand All @@ -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
Expand Down
112 changes: 56 additions & 56 deletions core-processor/src/main/java/io/micronaut/inject/ast/MethodElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}

/**
Expand Down
31 changes: 23 additions & 8 deletions inject-java/src/main/java/io/micronaut/annotation/processing/visitor/JavaMethodElement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions inject-java/src/test/groovy/io/micronaut/inject/beans/BeanDefinitionSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -668,6 +669,40 @@ interface AInterface<K, V> {
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()
Expand Down
21 changes: 21 additions & 0 deletions inject-java/src/test/groovy/io/micronaut/visitors/ClassElementSpec.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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('''
Expand Down
Loading

0 comments on commit 4947687

Please sign in to comment.