Use correct type for the Record component read accessors

filiphr · Sep 1, 2024 · 478648f · 478648f
1 parent c6010c9
commit 478648f
Show file tree

Hide file tree

Showing 18 changed files with 265 additions and 29 deletions.
diff --git a/integrationtest/src/test/java/org/mapstruct/itest/tests/MavenIntegrationTest.java b/integrationtest/src/test/java/org/mapstruct/itest/tests/MavenIntegrationTest.java
@@ -131,6 +131,13 @@ void recordsTest() {
     void recordsCrossModuleTest() {
     }
 
+    @ProcessorTest(baseDir = "recordsCrossModuleInterfaceTest", processorTypes = {
+        ProcessorTest.ProcessorType.JAVAC
+    })
+    @EnabledForJreRange(min = JRE.JAVA_17)
+    void recordsCrossModuleInterfaceTest() {
+    }
+
     @ProcessorTest(baseDir = "expressionTextBlocksTest", processorTypes = {
         ProcessorTest.ProcessorType.JAVAC
     })

diff --git a/integrationtest/src/test/resources/recordsCrossModuleInterfaceTest/module-1/pom.xml b/integrationtest/src/test/resources/recordsCrossModuleInterfaceTest/module-1/pom.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright MapStruct Authors.
+
+    Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <artifactId>recordsCrossModuleInterfaceTest</artifactId>
+        <groupId>org.mapstruct</groupId>
+        <version>1.0.0</version>
+    </parent>
+
+    <artifactId>records-cross-module-1</artifactId>
+
+</project>
diff --git a/...rfaceTest/module-1/src/main/java/org/mapstruct/itest/records/module1/NestedInterface.java b/...rfaceTest/module-1/src/main/java/org/mapstruct/itest/records/module1/NestedInterface.java
@@ -0,0 +1,10 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.itest.records.module1;
+
+public interface NestedInterface {
+    String field();
+}
diff --git a/...terfaceTest/module-1/src/main/java/org/mapstruct/itest/records/module1/RootInterface.java b/...terfaceTest/module-1/src/main/java/org/mapstruct/itest/records/module1/RootInterface.java
@@ -0,0 +1,10 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.itest.records.module1;
+
+public interface RootInterface {
+    NestedInterface nested();
+}
diff --git a/...ceTest/module-1/src/main/java/org/mapstruct/itest/records/module1/SourceNestedRecord.java b/...ceTest/module-1/src/main/java/org/mapstruct/itest/records/module1/SourceNestedRecord.java
@@ -0,0 +1,11 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.itest.records.module1;
+
+public record SourceNestedRecord(
+        String field
+) implements NestedInterface {
+}
diff --git a/...faceTest/module-1/src/main/java/org/mapstruct/itest/records/module1/SourceRootRecord.java b/...faceTest/module-1/src/main/java/org/mapstruct/itest/records/module1/SourceRootRecord.java
@@ -0,0 +1,11 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.itest.records.module1;
+
+public record SourceRootRecord(
+        SourceNestedRecord nested
+) implements RootInterface {
+}
diff --git a/integrationtest/src/test/resources/recordsCrossModuleInterfaceTest/module-2/pom.xml b/integrationtest/src/test/resources/recordsCrossModuleInterfaceTest/module-2/pom.xml
@@ -0,0 +1,30 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright MapStruct Authors.
+
+    Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <artifactId>recordsCrossModuleInterfaceTest</artifactId>
+        <groupId>org.mapstruct</groupId>
+        <version>1.0.0</version>
+    </parent>
+
+    <artifactId>records-cross-module-2</artifactId>
+
+
+    <dependencies>
+        <dependency>
+            <groupId>org.mapstruct</groupId>
+            <artifactId>records-cross-module-1</artifactId>
+            <version>1.0.0</version>
+        </dependency>
+    </dependencies>
+</project>
diff --git a/...odule-2/src/main/java/org/mapstruct/itest/records/module2/RecordInterfaceIssueMapper.java b/...odule-2/src/main/java/org/mapstruct/itest/records/module2/RecordInterfaceIssueMapper.java
@@ -0,0 +1,18 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.itest.records.module2;
+
+import org.mapstruct.itest.records.module1.SourceRootRecord;
+import org.mapstruct.Mapper;
+import org.mapstruct.factory.Mappers;
+
+@Mapper
+public interface RecordInterfaceIssueMapper {
+
+    RecordInterfaceIssueMapper INSTANCE = Mappers.getMapper(RecordInterfaceIssueMapper.class);
+
+    TargetRootRecord map(SourceRootRecord source);
+}
diff --git a/...ceTest/module-2/src/main/java/org/mapstruct/itest/records/module2/TargetNestedRecord.java b/...ceTest/module-2/src/main/java/org/mapstruct/itest/records/module2/TargetNestedRecord.java
@@ -0,0 +1,11 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.itest.records.module2;
+
+public record TargetNestedRecord(
+        String field
+) {
+}
diff --git a/...faceTest/module-2/src/main/java/org/mapstruct/itest/records/module2/TargetRootRecord.java b/...faceTest/module-2/src/main/java/org/mapstruct/itest/records/module2/TargetRootRecord.java
@@ -0,0 +1,11 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.itest.records.module2;
+
+public record TargetRootRecord(
+        TargetNestedRecord nested
+) {
+}
diff --git a/...InterfaceTest/module-2/src/test/java/org/mapstruct/itest/records/module2/RecordsTest.java b/...InterfaceTest/module-2/src/test/java/org/mapstruct/itest/records/module2/RecordsTest.java
@@ -0,0 +1,26 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.itest.records.module2;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import org.junit.Test;
+import org.mapstruct.itest.records.module1.SourceRootRecord;
+import org.mapstruct.itest.records.module1.SourceNestedRecord;
+
+public class RecordsTest {
+
+    @Test
+    public void shouldMap() {
+        SourceRootRecord source = new SourceRootRecord( new SourceNestedRecord( "test" ) );
+        TargetRootRecord target = RecordInterfaceIssueMapper.INSTANCE.map( source );
+
+        assertThat( target ).isNotNull();
+        assertThat( target.nested() ).isNotNull();
+        assertThat( target.nested().field() ).isEqualTo( "test" );
+    }
+
+}
diff --git a/integrationtest/src/test/resources/recordsCrossModuleInterfaceTest/pom.xml b/integrationtest/src/test/resources/recordsCrossModuleInterfaceTest/pom.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    Copyright MapStruct Authors.
+
+    Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+
+-->
+<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+
+    <parent>
+        <groupId>org.mapstruct</groupId>
+        <artifactId>mapstruct-it-parent</artifactId>
+        <version>1.0.0</version>
+        <relativePath>../pom.xml</relativePath>
+    </parent>
+
+    <artifactId>recordsCrossModuleInterfaceTest</artifactId>
+    <packaging>pom</packaging>
+
+    <modules>
+        <module>module-1</module>
+        <module>module-2</module>
+    </modules>
+</project>
diff --git a/...uct/itest/records/mapper/RecordsTest.java → ...uct/itest/records/mapper/RecordsTest.java b/...uct/itest/records/mapper/RecordsTest.java → ...uct/itest/records/mapper/RecordsTest.java
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java b/processor/src/main/java/org/mapstruct/ap/internal/model/common/Type.java
@@ -50,7 +50,7 @@
 import org.mapstruct.ap.internal.util.TypeUtils;
 import org.mapstruct.ap.internal.util.accessor.Accessor;
 import org.mapstruct.ap.internal.util.accessor.AccessorType;
-import org.mapstruct.ap.internal.util.accessor.FieldElementAccessor;
+import org.mapstruct.ap.internal.util.accessor.ElementAccessor;
 import org.mapstruct.ap.internal.util.accessor.MapValueAccessor;
 import org.mapstruct.ap.internal.util.accessor.PresenceCheckAccessor;
 import org.mapstruct.ap.internal.util.accessor.ReadAccessor;
@@ -1047,7 +1047,7 @@ private List<Accessor> getAlternativeTargetAccessors() {
             List<Accessor> setterMethods = getSetters();
             List<Accessor> readAccessors = new ArrayList<>( getPropertyReadAccessors().values() );
             // All the fields are also alternative accessors
-            readAccessors.addAll( filters.fieldsIn( getAllFields(), FieldElementAccessor::new ) );
+            readAccessors.addAll( filters.fieldsIn( getAllFields(), ElementAccessor::new ) );
 
             // there could be a read accessor (field or  method) for a list/map that is not present as setter.
             // an accessor could substitute the setter in that case and act as setter.

diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/Filters.java b/processor/src/main/java/org/mapstruct/ap/internal/util/Filters.java
@@ -39,22 +39,16 @@
 public class Filters {
 
     private static final Method RECORD_COMPONENTS_METHOD;
-    private static final Method RECORD_COMPONENT_ACCESSOR_METHOD;
 
     static {
         Method recordComponentsMethod;
-        Method recordComponentAccessorMethod;
         try {
             recordComponentsMethod = TypeElement.class.getMethod( "getRecordComponents" );
-            recordComponentAccessorMethod = Class.forName( "javax.lang.model.element.RecordComponentElement" )
-                .getMethod( "getAccessor" );
         }
-        catch ( NoSuchMethodException | ClassNotFoundException e ) {
+        catch ( NoSuchMethodException e ) {
             recordComponentsMethod = null;
-            recordComponentAccessorMethod = null;
         }
         RECORD_COMPONENTS_METHOD = recordComponentsMethod;
-        RECORD_COMPONENT_ACCESSOR_METHOD = recordComponentAccessorMethod;
     }
 
     private final AccessorNamingUtils accessorNaming;
@@ -89,25 +83,18 @@ public List<Element> recordComponentsIn(TypeElement typeElement) {
     }
 
     public Map<String, ReadAccessor> recordAccessorsIn(Collection<Element> recordComponents) {
-        if ( RECORD_COMPONENT_ACCESSOR_METHOD == null ) {
+        if ( recordComponents.isEmpty() ) {
             return java.util.Collections.emptyMap();
         }
-        try {
-            Map<String, ReadAccessor> recordAccessors = new LinkedHashMap<>();
-            for ( Element recordComponent : recordComponents ) {
-                ExecutableElement recordExecutableElement =
-                    (ExecutableElement) RECORD_COMPONENT_ACCESSOR_METHOD.invoke( recordComponent );
-                recordAccessors.put(
-                    recordComponent.getSimpleName().toString(),
-                    ReadAccessor.fromGetter( recordExecutableElement, getReturnType( recordExecutableElement ) )
-                );
-            }
-
-            return recordAccessors;
-        }
-        catch ( IllegalAccessException | InvocationTargetException e ) {
-            return java.util.Collections.emptyMap();
+        Map<String, ReadAccessor> recordAccessors = new LinkedHashMap<>();
+        for ( Element recordComponent : recordComponents ) {
+            recordAccessors.put(
+                recordComponent.getSimpleName().toString(),
+                ReadAccessor.fromRecordComponent( recordComponent )
+            );
         }
+
+        return recordAccessors;
     }
 
     private TypeMirror getReturnType(ExecutableElement executableElement) {

diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/ElementAccessor.java b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/ElementAccessor.java
@@ -0,0 +1,45 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.ap.internal.util.accessor;
+
+import javax.lang.model.element.Element;
+import javax.lang.model.element.VariableElement;
+import javax.lang.model.type.TypeMirror;
+
+/**
+ * An {@link Accessor} that wraps a {@link VariableElement}.
+ *
+ * @author Filip Hrisafov
+ */
+public class ElementAccessor extends AbstractAccessor<Element> {
+
+    private final AccessorType accessorType;
+
+    public ElementAccessor(VariableElement variableElement) {
+        this( variableElement, AccessorType.FIELD );
+    }
+
+    public ElementAccessor(Element element, AccessorType accessorType) {
+        super( element );
+        this.accessorType = accessorType;
+    }
+
+    @Override
+    public TypeMirror getAccessedType() {
+        return element.asType();
+    }
+
+    @Override
+    public String toString() {
+        return element.toString();
+    }
+
+    @Override
+    public AccessorType getAccessorType() {
+        return accessorType;
+    }
+
+}
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/ReadAccessor.java b/processor/src/main/java/org/mapstruct/ap/internal/util/accessor/ReadAccessor.java
@@ -5,6 +5,7 @@
  */
 package org.mapstruct.ap.internal.util.accessor;
 
+import javax.lang.model.element.Element;
 import javax.lang.model.element.ExecutableElement;
 import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.TypeMirror;
@@ -17,14 +18,23 @@ public interface ReadAccessor extends Accessor {
     String getReadValueSource();
 
     static ReadAccessor fromField(VariableElement variableElement) {
-        return new ReadDelegateAccessor( new FieldElementAccessor( variableElement ) ) {
+        return new ReadDelegateAccessor( new ElementAccessor( variableElement ) ) {
             @Override
             public String getReadValueSource() {
                 return getSimpleName();
             }
         };
     }
 
+    static ReadAccessor fromRecordComponent(Element element) {
+        return new ReadDelegateAccessor( new ElementAccessor( element, AccessorType.GETTER ) ) {
+            @Override
+            public String getReadValueSource() {
+                return getSimpleName() + "()";
+            }
+        };
+    }
+
     static ReadAccessor fromGetter(ExecutableElement element, TypeMirror accessedType) {
         return new ReadDelegateAccessor( new ExecutableElementAccessor( element, accessedType, AccessorType.GETTER ) ) {
             @Override

diff --git a/...l/util/accessor/FieldElementAccessor.java → .../util/accessor/RecordElementAccessor.java b/...l/util/accessor/FieldElementAccessor.java → .../util/accessor/RecordElementAccessor.java
@@ -5,6 +5,7 @@
  */
 package org.mapstruct.ap.internal.util.accessor;
 
+import javax.lang.model.element.Element;
 import javax.lang.model.element.VariableElement;
 import javax.lang.model.type.TypeMirror;
 
@@ -13,9 +14,9 @@
  *
  * @author Filip Hrisafov
  */
-public class FieldElementAccessor extends AbstractAccessor<VariableElement> {
+public class RecordElementAccessor extends AbstractAccessor<Element> {
 
-    public FieldElementAccessor(VariableElement element) {
+    public RecordElementAccessor(Element element) {
         super( element );
     }
 
@@ -31,7 +32,7 @@ public String toString() {
 
     @Override
     public AccessorType getAccessorType() {
-        return AccessorType.FIELD;
+        return AccessorType.GETTER;
     }
 
 }