Avoid using ElementQuery#named (#11055)

micronaut-projects · Aug 12, 2024 · 7f37eca · 7f37eca
1 parent 6f99905
commit 7f37eca
Show file tree

Hide file tree

Showing 10 changed files with 53 additions and 55 deletions.
diff --git a/core-processor/src/main/java/io/micronaut/aop/writer/AopProxyWriter.java b/core-processor/src/main/java/io/micronaut/aop/writer/AopProxyWriter.java
@@ -552,8 +552,7 @@ public void visitAroundMethod(TypedElement beanType,
         final Optional<MethodElement> overridden = methodElement.getOwningType()
                 .getEnclosedElement(ElementQuery.ALL_METHODS
                         .onlyInstance()
-                        .named(name -> name.equals(methodElement.getName()))
-                        .filter(el -> el.overrides(methodElement)));
+                        .filter(el -> el.getName().equals(methodElement.getName()) && el.overrides(methodElement)));
 
         if (overridden.isPresent()) {
             MethodElement overriddenBy = overridden.get();

diff --git a/core-processor/src/main/java/io/micronaut/inject/ast/ElementQuery.java b/core-processor/src/main/java/io/micronaut/inject/ast/ElementQuery.java
@@ -162,13 +162,19 @@ public interface ElementQuery<T extends Element> {
 
     /**
      * Allows filtering elements by name.
+     * This method will only process native elements passing the name predicate.
+     * NOTE: Use this method only if the other elements shouldn't be touched (missing compilation information etc.). This method will skip all elements reusable cache.
+     *
      * @param predicate The predicate to use. Should return true to include the element.
      * @return This query
      */
     @NonNull ElementQuery<T> named(@NonNull Predicate<String> predicate);
 
     /**
      * Allows filtering elements by name.
+     * This method will only process native elements passing the name predicate.
+     * NOTE: Use this method only if the other elements shouldn't be touched (missing compilation information etc.). This method will skip all elements reusable cache.
+     *
      * @param name The name to filter by
      * @return This query
      * @since 3.5.2

diff --git a/core-processor/src/main/java/io/micronaut/inject/ast/utils/EnclosedElementsQuery.java b/core-processor/src/main/java/io/micronaut/inject/ast/utils/EnclosedElementsQuery.java
@@ -179,19 +179,23 @@ public <T extends io.micronaut.inject.ast.Element> List<T> getEnclosedElements(C
         if (result.isOnlyDeclared() || classElement.getSuperType().isEmpty() && classElement.getInterfaces().isEmpty()) {
             elements = getElements(nativeClassType, result, filter);
         } else {
-            // Let's try to load the unfiltered result and apply the filter
-            QueryResultKey queryWithoutPredicatesResultKey = new QueryResultKey(result.withoutPredicates(), classElement.getNativeType());
-            List<T> valuesWithoutPredicates = (List<T>) resultsCache.get(queryWithoutPredicatesResultKey);
-            if (valuesWithoutPredicates != null) {
-                return valuesWithoutPredicates.stream().filter(filter).toList();
-            }
-
-            elements = getAllElements(nativeClassType, (t1, t2) -> reduceElements(t1, t2, result), result);
-            if (!queryWithoutPredicatesResultKey.equals(queryResultKey)) {
+            ElementQuery.Result<T> resultWithoutPredicates = result.withoutPredicates();
+            QueryResultKey queryWithoutPredicatesResultKey = new QueryResultKey(resultWithoutPredicates, classElement.getNativeType());
+            if (queryWithoutPredicatesResultKey.equals(queryResultKey)) {
+                // No predicates query
+                elements = getAllElements(nativeClassType, (t1, t2) -> reduceElements(t1, t2, result), result);
+            } else {
+                // Let's try to load the result without predicates and cache it, then apply predicates
+                List<T> valuesWithoutPredicates = (List<T>) resultsCache.get(queryWithoutPredicatesResultKey);
+                if (valuesWithoutPredicates != null) {
+                    return valuesWithoutPredicates.stream().filter(filter).toList();
+                }
+                elements = getAllElements(nativeClassType, (t1, t2) -> reduceElements(t1, t2, resultWithoutPredicates), resultWithoutPredicates);
                 // This collection is before predicates are applied, we can store it and reuse
                 resultsCache.put(queryWithoutPredicatesResultKey, new ArrayList<>(elements));
+                // The second filtered result
+                elements.removeIf(element -> !filter.test(element));
             }
-            elements.removeIf(element -> !filter.test(element));
         }
         resultsCache.put(queryResultKey, elements);
         adjustMapCapacity(resultsCache, MAX_RESULTS);

diff --git a/core-processor/src/main/java/io/micronaut/inject/beans/visitor/BeanIntrospectionWriter.java b/core-processor/src/main/java/io/micronaut/inject/beans/visitor/BeanIntrospectionWriter.java
@@ -259,12 +259,13 @@ void visitProperty(
                     .onlyAccessible()
                     .onlyDeclared()
                     .onlyInstance()
-                    .named((n) -> n.startsWith(prefix) && n.equals(prefix + NameUtils.capitalize(name)))
                     .filter((methodElement -> {
                         ParameterElement[] parameters = methodElement.getParameters();
-                        return parameters.length == 1 &&
-                            methodElement.getGenericReturnType().getName().equals(classElement.getName()) &&
-                            type.getType().isAssignable(parameters[0].getType());
+                        String methodName = methodElement.getName();
+                        return methodName.startsWith(prefix) && methodName.equals(prefix + NameUtils.capitalize(name))
+                            && parameters.length == 1
+                            && methodElement.getGenericReturnType().getName().equals(classElement.getName())
+                            && type.getType().isAssignable(parameters[0].getType());
                     }));
                 MethodElement withMethod = classElement.getEnclosedElement(elementQuery).orElse(null);
                 if (withMethod != null) {

diff --git a/...essor/src/main/java/io/micronaut/inject/beans/visitor/IntrospectedTypeElementVisitor.java b/...essor/src/main/java/io/micronaut/inject/beans/visitor/IntrospectedTypeElementVisitor.java
@@ -182,8 +182,7 @@ private void processBuilderDefinition(ClassElement element, VisitorContext conte
             if (builderMethod != null) {
                 MethodElement methodElement = element
                     .getEnclosedElement(ElementQuery.ALL_METHODS.onlyStatic()
-                        .named(builderMethod)
-                        .filter(m -> !m.getGenericReturnType().isVoid())
+                        .filter(m -> m.getName().equals(builderMethod) && !m.getGenericReturnType().isVoid())
                         .onlyAccessible(element))
                     .orElse(null);
                 if (methodElement != null) {
@@ -341,9 +340,9 @@ private void handleBuilder(
                 ElementQuery<MethodElement> builderMethodQuery = ElementQuery.ALL_METHODS
                     .onlyAccessible(classToBuild)
                     .onlyInstance()
-                    .named(n -> Arrays.stream(writePrefixes).anyMatch(n::startsWith))
                     .filter(m ->
-                        builderType.isAssignable(m.getGenericReturnType()) && m.getParameters().length <= 1
+                        Arrays.stream(writePrefixes).anyMatch(m.getName()::startsWith) &&
+                            builderType.isAssignable(m.getGenericReturnType()) && m.getParameters().length <= 1
                     );
                 builderType.getEnclosedElements(builderMethodQuery)
                     .forEach(builderWriter::visitBeanMethod);

diff --git a/core-processor/src/main/java/io/micronaut/inject/processing/FactoryBeanElementCreator.java b/core-processor/src/main/java/io/micronaut/inject/processing/FactoryBeanElementCreator.java
@@ -284,7 +284,7 @@ private void buildProducedBeanDefinition(ClassElement producedType,
                 if (StringUtils.isNotEmpty(destroyMethodName)) {
                     final Optional<MethodElement> destroyMethod = producedType.getEnclosedElement(ElementQuery.ALL_METHODS.onlyAccessible(classElement)
                         .onlyInstance()
-                        .named(destroyMethodName)
+                        .named(destroyMethodName) // Named filtering should avoid processing all method and fail on possible missing classes and compilation errors
                         .filter((e) -> !e.hasParameters()));
                     if (destroyMethod.isPresent()) {
                         MethodElement destroyMethodElement = destroyMethod.get();

diff --git a/core-processor/src/main/java/io/micronaut/inject/writer/BeanDefinitionWriter.java b/core-processor/src/main/java/io/micronaut/inject/writer/BeanDefinitionWriter.java
@@ -15,9 +15,6 @@
  */
 package io.micronaut.inject.writer;
 
-import static io.micronaut.core.util.StringUtils.EMPTY_STRING_ARRAY;
-import static io.micronaut.inject.visitor.BeanElementVisitor.VISITORS;
-
 import io.micronaut.aop.writer.AopProxyWriter;
 import io.micronaut.context.AbstractBeanDefinitionBeanConstructor;
 import io.micronaut.context.AbstractExecutableMethod;
@@ -129,6 +126,14 @@
 import io.micronaut.inject.visitor.BeanElementVisitorContext;
 import io.micronaut.inject.visitor.VisitorContext;
 import jakarta.inject.Singleton;
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.ClassWriter;
+import org.objectweb.asm.Label;
+import org.objectweb.asm.MethodVisitor;
+import org.objectweb.asm.Type;
+import org.objectweb.asm.commons.GeneratorAdapter;
+import org.objectweb.asm.signature.SignatureVisitor;
+
 import java.io.IOException;
 import java.io.OutputStream;
 import java.lang.annotation.Annotation;
@@ -156,13 +161,9 @@
 import java.util.function.Consumer;
 import java.util.function.Predicate;
 import java.util.stream.Stream;
-import org.objectweb.asm.ClassVisitor;
-import org.objectweb.asm.ClassWriter;
-import org.objectweb.asm.Label;
-import org.objectweb.asm.MethodVisitor;
-import org.objectweb.asm.Type;
-import org.objectweb.asm.commons.GeneratorAdapter;
-import org.objectweb.asm.signature.SignatureVisitor;
+
+import static io.micronaut.core.util.StringUtils.EMPTY_STRING_ARRAY;
+import static io.micronaut.inject.visitor.BeanElementVisitor.VISITORS;
 
 /**
  * <p>Responsible for building {@link BeanDefinition} instances at compile time. Uses ASM build the class definition.</p>
@@ -2195,8 +2196,7 @@ public void visitAnnotationMemberPropertyInjectionPoint(TypedElement annotationM
                     ElementQuery.ALL_METHODS
                             .onlyAccessible(beanTypeElement)
                             .onlyInstance()
-                            .named(name -> annotationMemberProperty.equals(NameUtils.getPropertyNameForGetter(name, readPrefixes)))
-                            .filter((e) -> !e.hasParameters())
+                            .filter(m -> annotationMemberProperty.equals(NameUtils.getPropertyNameForGetter(m.getName(), readPrefixes)) && !m.hasParameters())
             ).orElse(null);
         }
 

diff --git a/graal/src/main/java/io/micronaut/graal/reflect/GraalTypeElementVisitor.java b/graal/src/main/java/io/micronaut/graal/reflect/GraalTypeElementVisitor.java
@@ -27,7 +27,6 @@
 import io.micronaut.core.annotation.ReflectionConfig;
 import io.micronaut.core.annotation.ReflectiveAccess;
 import io.micronaut.core.annotation.TypeHint;
-import io.micronaut.core.util.CollectionUtils;
 import io.micronaut.core.util.StringUtils;
 import io.micronaut.inject.annotation.MutableAnnotationMetadata;
 import io.micronaut.inject.ast.ClassElement;

diff --git a/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/KotlinClassElement.kt b/inject-kotlin/src/main/kotlin/io/micronaut/kotlin/processing/visitor/KotlinClassElement.kt
@@ -389,11 +389,9 @@ internal open class KotlinClassElement(
         }
 
         val eq = ElementQuery.of(PropertyElement::class.java)
-            .named { n -> !propertyElementQuery.excludes.contains(n) }
-            .named { n ->
-                propertyElementQuery.includes.isEmpty() || propertyElementQuery.includes.contains(
-                    n
-                )
+            .filter { el ->
+                !propertyElementQuery.excludes.contains(el.name)
+                        && (propertyElementQuery.includes.isEmpty() || propertyElementQuery.includes.contains(el.name))
             }
             .modifiers {
                 if (!propertyElementQuery.isAllowStaticProperties && it.contains(ElementModifier.STATIC)) {
@@ -820,14 +818,15 @@ internal open class KotlinClassElement(
                 FieldElement::class.java -> {
                     classNode.getDeclaredProperties()
                         .filter {
-                            it.hasBackingField &&
-                                    it.origin != Origin.SYNTHETIC
+                            it.hasBackingField && it.origin != Origin.SYNTHETIC
                         }
                         .toList()
                 }
 
                 PropertyElement::class.java -> {
-                    classNode.getDeclaredProperties().filter { !it.isJavaPackagePrivate() }.toList()
+                    classNode.getDeclaredProperties().filter {
+                        !it.isJavaPackagePrivate() && !it.annotations.any { ann -> ann.shortName.asString() == JvmField::class.java.simpleName }
+                    }.toList()
                 }
 
                 ConstructorElement::class.java -> {
@@ -886,23 +885,14 @@ internal open class KotlinClassElement(
                 }
 
                 is KSPropertyDeclaration -> {
-                    if (elementType == PropertyElement::class.java) {
-                        val prop = KotlinPropertyElement(
+                    return if (elementType == PropertyElement::class.java) {
+                        KotlinPropertyElement(
                             owningClass,
                             nativeType,
                             elementAnnotationMetadataFactory, visitorContext
                         )
-                        if (!prop.hasAnnotation(JvmField::class.java)) {
-                            return prop
-                        } else {
-                            return elementFactory.newFieldElement(
-                                owningClass,
-                                nativeType,
-                                elementAnnotationMetadataFactory
-                            )
-                        }
                     } else {
-                        return elementFactory.newFieldElement(
+                        elementFactory.newFieldElement(
                             owningClass,
                             nativeType,
                             elementAnnotationMetadataFactory

diff --git a/...in/src/test/groovy/io/micronaut/kotlin/processing/visitor/KotlinReconstructionSpec.groovy b/...in/src/test/groovy/io/micronaut/kotlin/processing/visitor/KotlinReconstructionSpec.groovy
@@ -62,7 +62,7 @@ class Lst<in E> {
 }
 """, {
                 element ->
-                    def method = element.<MethodElement> getEnclosedElement(ElementQuery.ALL_METHODS.named(s -> s == 'method')).get()
+                    def method = element.<MethodElement> getEnclosedElement(ElementQuery.ALL_METHODS.filter(el -> el.name == 'method')).get()
                     return reconstructTypeSignature(method.declaredTypeVariables[0], true)
             })
         expect: