Repository: mapstruct/mapstruct (language: java)

Issue:
Redundant if condition in Java record mapping with RETURN_DEFAULT strategy
### Expected behavior

The generated code should avoid unnecessary empty `if ` blocks. It should generate something like these:

```java
    @Override
    public ApiCategoryRecord mapRecord(Category category) {
        Long id = null;
        String title = null;
        if ( category != null ) {
            id = category.getId();
            title = category.getTitle();
        }

        ApiCategoryRecord apiCategoryRecord = new ApiCategoryRecord( id, title );

        return apiCategoryRecord;
    }
```

OR

```java
    @Override
    public ApiCategoryRecord mapRecord(Category category) {
        if ( category != null ) {
            return new ApiCategoryRecord( category.getId(), category.getTitle() );
        }
        else {
            return new ApiCategoryRecord( null, null );
        }
    }
```


### Actual behavior

MapStruct generates an empty and unnecessary `if` block after constructing the record:

```java
    @Override
    public ApiCategoryRecord mapRecord(Category category) {
        Long id = null;
        String title = null;
        if ( category != null ) {
            id = category.getId();
            title = category.getTitle();
        }
        ApiCategoryRecord apiCategoryRecord = new ApiCategoryRecord( id, title );
        if ( category != null ) {
        }
        return apiCategoryRecord;
    }
```

Literally this is not a bug but a minor inefficiency in the generated code.
It does not affect functionality, removing the redundant if condition would improve code clarity and doesn’t suggest anything went wrong during generation.

### Steps to reproduce the problem

Use the following setup

```java
public class Category {
    private Long id;
    private String title;
    public Long getId() { return id; }
    public String getTitle() { return title; }
}

public record ApiCategoryRecord(Long id, String title) {}

@Mapper(nullValueMappingStrategy = NullValueMappingStrategy.RETURN_DEFAULT)
public interface ICategoryMapper {
    ApiCategoryRecord mapRecord(Category category);
}
```

### MapStruct Version

1.6.2


Discussion / hints:
`version: 1.6.2, compiler: javac, environment: Java 21.0.5 (BellSoft)`
Thanks for reporting this @primavistalab.

>Literally this is not a bug but a minor inefficiency in the generated code.
It does not affect functionality, removing the redundant if condition would improve code clarity and doesn’t suggest anything went wrong during generation.

We strive to generate code as it has been written by a human. The fact that we generate

```java
if ( category != null ) {
}
```

is a bug for us.

The fix should be relatively easy. We'll look into it

Fix patch:
diff --git a/processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl b/processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl
index 61d9cc1837..3036e4a2c8 100644
--- a/processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl
+++ b/processor/src/main/resources/org/mapstruct/ap/internal/model/BeanMappingMethod.ftl
@@ -116,7 +116,7 @@
                 </#list>
             </#if>
         </#list>
-    <#else>
+    <#elseif !propertyMappingsByParameter(sourceParameters[0]).empty>
         <#if mapNullToDefault>if ( <@includeModel object=getPresenceCheckByParameter(sourceParameters[0]) /> ) {</#if>
         <#list propertyMappingsByParameter(sourceParameters[0]) as propertyMapping>
             <@includeModel object=propertyMapping targetBeanName=resultName existingInstanceMapping=existingInstanceMapping defaultValueAssignment=propertyMapping.defaultValueAssignment/>


Test patch:
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_3747/Issue3747Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_3747/Issue3747Mapper.java
new file mode 100644
index 0000000000..04addc43f3
--- /dev/null
+++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_3747/Issue3747Mapper.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._3747;
+
+import org.mapstruct.Mapper;
+import org.mapstruct.NullValueMappingStrategy;
+
+/**
+ * @author Filip Hrisafov
+ */
+@Mapper(nullValueMappingStrategy = NullValueMappingStrategy.RETURN_DEFAULT)
+public interface Issue3747Mapper {
+
+    Target map(Source source);
+
+    class Source {
+        private final String value;
+
+        public Source(String value) {
+            this.value = value;
+        }
+
+        public String getValue() {
+            return value;
+        }
+    }
+
+    class Target {
+        private final String value;
+
+        public Target(String value) {
+            this.value = value;
+        }
+
+        public String getValue() {
+            return value;
+        }
+    }
+}
diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_3747/Issue3747Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_3747/Issue3747Test.java
new file mode 100644
index 0000000000..57650e4dc0
--- /dev/null
+++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_3747/Issue3747Test.java
@@ -0,0 +1,28 @@
+/*
+ * 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._3747;
+
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.mapstruct.ap.testutil.IssueKey;
+import org.mapstruct.ap.testutil.ProcessorTest;
+import org.mapstruct.ap.testutil.WithClasses;
+import org.mapstruct.ap.testutil.runner.GeneratedSource;
+
+/**
+ * @author Filip Hrisafov
+ */
+@IssueKey("3747")
+@WithClasses(Issue3747Mapper.class)
+class Issue3747Test {
+
+    @RegisterExtension
+    final GeneratedSource generatedSource = new GeneratedSource();
+
+    @ProcessorTest
+    void shouldNotGenerateObsoleteCode() {
+        generatedSource.addComparisonToFixtureFor( Issue3747Mapper.class );
+    }
+}
diff --git a/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_3747/Issue3747MapperImpl.java b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_3747/Issue3747MapperImpl.java
new file mode 100644
index 0000000000..adc937dab1
--- /dev/null
+++ b/processor/src/test/resources/fixtures/org/mapstruct/ap/test/bugs/_3747/Issue3747MapperImpl.java
@@ -0,0 +1,29 @@
+/*
+ * 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._3747;
+
+import javax.annotation.processing.Generated;
+
+@Generated(
+    value = "org.mapstruct.ap.MappingProcessor",
+    date = "2024-11-03T12:50:02+0100",
+    comments = "version: , compiler: javac, environment: Java 21.0.3 (N/A)"
+)
+public class Issue3747MapperImpl implements Issue3747Mapper {
+
+    @Override
+    public Target map(Source source) {
+
+        String value = null;
+        if ( source != null ) {
+            value = source.getValue();
+        }
+
+        Target target = new Target( value );
+
+        return target;
+    }
+}