mapstruct#2245 Local variable should be created when using default value

filiphr · filiphr · commit 85890dd44273 · 2020-10-25T14:42:48.000+01:00
diff --git a/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java b/processor/src/main/java/org/mapstruct/ap/internal/model/PropertyMapping.java
@@ -374,6 +374,11 @@ private Assignment getDefaultValueAssignment( Assignment rhs ) {
             return null;
         }
 
+        private boolean hasDefaultValueAssignment(Assignment rhs) {
+            return ( defaultValue != null || defaultJavaExpression != null ) &&
+                ( !rhs.getSourceType().isPrimitive() || rhs.getSourcePresenceCheckerReference() != null);
+        }
+
         private Assignment assignToPlain(Type targetType, AccessorType targetAccessorType,
                                          Assignment rightHandSide) {
 
@@ -415,7 +420,9 @@ private Assignment assignToPlainViaSetter(Type targetType, Assignment rhs) {
                 );
             }
             else {
-                boolean includeSourceNullCheck = SetterWrapper.doSourceNullCheck( rhs, nvcs, nvpms, targetType );
+                // If the property mapping has a default value assignment then we have to do a null value check
+                boolean includeSourceNullCheck = SetterWrapper.doSourceNullCheck( rhs, nvcs, nvpms, targetType )
+                    || hasDefaultValueAssignment( rhs );
                 if ( !includeSourceNullCheck ) {
                     // solution for #834 introduced a local var and null check for nested properties always.
                     // however, a local var is not needed if there's no need to check for null.
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2245/Issue2245Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2245/Issue2245Test.java
@@ -0,0 +1,42 @@
+/*
+ * 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.test.bugs._2245;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mapstruct.ap.testutil.IssueKey;
+import org.mapstruct.ap.testutil.WithClasses;
+import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner;
+import org.mapstruct.ap.testutil.runner.GeneratedSource;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+/**
+ * @author Filip Hrisafov
+ */
+@IssueKey("2245")
+@RunWith(AnnotationProcessorTestRunner.class)
+@WithClasses({
+    TestMapper.class
+})
+public class Issue2245Test {
+
+    @Rule
+    public final GeneratedSource generatedSource = new GeneratedSource()
+        .addComparisonToFixtureFor( TestMapper.class );
+
+    @Test
+    public void shouldGenerateSourceGetMethodOnce() {
+
+        TestMapper.Tenant tenant =
+            TestMapper.INSTANCE.map( new TestMapper.TenantDTO( new TestMapper.Inner( "acme" ) ) );
+
+        assertThat( tenant ).isNotNull();
+        assertThat( tenant.getId() ).isEqualTo( "acme" );
+
+    }
+}
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2245/TestMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2245/TestMapper.java
@@ -0,0 +1,55 @@
+/*
+ * 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.test.bugs._2245;
+
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.mapstruct.factory.Mappers;
+
+@Mapper
+public interface TestMapper {
+
+    TestMapper INSTANCE = Mappers.getMapper( TestMapper.class );
+
+    class Tenant {
+        private String id;
+
+        public String getId() {
+            return id;
+        }
+
+        public void setId(String id) {
+            this.id = id;
+        }
+    }
+
+    class Inner {
+        private final String id;
+
+        public Inner(String id) {
+            this.id = id;
+        }
+
+        public String getId() {
+            return id;
+        }
+    }
+
+    class TenantDTO {
+        private final Inner inner;
+
+        public TenantDTO(Inner inner) {
+            this.inner = inner;
+        }
+
+        public Inner getInner() {
+            return inner;
+        }
+    }
+
+    @Mapping(target = "id", source = "inner.id", defaultValue = "test")
+    Tenant map(TenantDTO tenant);
+}
diff --git a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_2245/TestMapperImpl.java b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_2245/TestMapperImpl.java
@@ -0,0 +1,50 @@
+/*
+ * 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.test.bugs._2245;
+
+import javax.annotation.Generated;
+
+@Generated(
+    value = "org.mapstruct.ap.MappingProcessor",
+    date = "2020-10-24T17:57:23+0200",
+    comments = "version: , compiler: javac, environment: Java 1.8.0_202 (AdoptOpenJdk)"
+)
+public class TestMapperImpl implements TestMapper {
+
+    @Override
+    public Tenant map(TenantDTO tenant) {
+        if ( tenant == null ) {
+            return null;
+        }
+
+        Tenant tenant1 = new Tenant();
+
+        String id = tenantInnerId( tenant );
+        if ( id != null ) {
+            tenant1.setId( id );
+        }
+        else {
+            tenant1.setId( "test" );
+        }
+
+        return tenant1;
+    }
+
+    private String tenantInnerId(TenantDTO tenantDTO) {
+        if ( tenantDTO == null ) {
+            return null;
+        }
+        Inner inner = tenantDTO.getInner();
+        if ( inner == null ) {
+            return null;
+        }
+        String id = inner.getId();
+        if ( id == null ) {
+            return null;
+        }
+        return id;
+    }
+}
