Skip to content

Commit 046df58

Browse files
committed
Polish
1 parent 623a765 commit 046df58

File tree

10 files changed

+91
-117
lines changed

10 files changed

+91
-117
lines changed

NEXT_RELEASE_CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
### Enhancements
77

88
* Add support for locale parameter for numberFormat and dateFormat (#3628)
9+
* Detect Builder without a factory method (#3729) - With this if there is an inner class that ends with `Builder` and has a constructor with parameters,
10+
it will be treated as a potential builder.
11+
Builders through static methods on the type have a precedence.
912
* Behaviour change: Warning when the target has no target properties (#1140)
1013

1114

processor/src/main/java/org/mapstruct/ap/spi/DefaultBuilderProvider.java

Lines changed: 79 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,8 @@
99
import java.util.Collection;
1010
import java.util.Collections;
1111
import java.util.List;
12-
import java.util.Optional;
1312
import java.util.regex.Pattern;
14-
import java.util.stream.Collectors;
15-
import java.util.stream.Stream;
13+
import javax.lang.model.element.Element;
1614
import javax.lang.model.element.ElementKind;
1715
import javax.lang.model.element.ExecutableElement;
1816
import javax.lang.model.element.Modifier;
@@ -29,7 +27,7 @@
2927

3028
/**
3129
* Default implementation of {@link BuilderProvider}.
32-
* <p>
30+
*
3331
* The default builder provider considers all public static parameterless methods of a {@link TypeMirror}
3432
* as potential builder creation methods. For each potential builder creation method checks in the return type
3533
* of the method if there exists a method that returns the initial {@link TypeMirror} if such a combination is found
@@ -51,7 +49,7 @@
5149
* public static Builder builder() {
5250
* return new Builder();
5351
* }
54-
*
52+
5553
* public static class Builder {
5654
*
5755
* private String firstName;
@@ -67,7 +65,7 @@
6765
* }
6866
* }
6967
* </code></pre>
70-
* <p>
68+
*
7169
* In the example above, when searching for a builder for the {@code Person} type. The {@code Person#builder} method
7270
* would be a builder creation candidate. Then the return type of {@code Person#builder}, {@code Builder}, is
7371
* investigated for a parameterless method that returns {@code Person}. When {@code Builder#create} is found
@@ -105,8 +103,10 @@ public BuilderInfo findBuilderInfo(TypeMirror type) {
105103
* Find the {@link TypeElement} for the given {@link TypeMirror}.
106104
*
107105
* @param type for which the {@link TypeElement} needs to be found/
106+
*
108107
* @return the type element or {@code null} if the {@link TypeMirror} is not a {@link DeclaredType}
109108
* and the declared type element is not {@link TypeElement}
109+
*
110110
* @throws TypeHierarchyErroneousException if the {@link TypeMirror} is of kind {@link TypeKind#ERROR}
111111
*/
112112
protected TypeElement getTypeElement(TypeMirror type) {
@@ -186,82 +186,96 @@ protected BuilderInfo findBuilderInfo(TypeElement typeElement, boolean checkPare
186186
return null;
187187
}
188188

189-
BuilderInfo staticFactoryMethodBuilderInfo = findStaticFactoryMethodBuilderInfo( typeElement );
190-
if ( staticFactoryMethodBuilderInfo != null ) {
191-
return staticFactoryMethodBuilderInfo;
189+
// Builder infos which are determined by a static method on the type itself
190+
List<BuilderInfo> methodBuilderInfos = new ArrayList<>();
191+
// Builder infos which are determined by an inner builder class in the type itself
192+
List<BuilderInfo> innerClassBuilderInfos = new ArrayList<>();
193+
194+
for ( Element enclosedElement : typeElement.getEnclosedElements() ) {
195+
if ( ElementKind.METHOD == enclosedElement.getKind() ) {
196+
ExecutableElement method = (ExecutableElement) enclosedElement;
197+
BuilderInfo builderInfo = determineMethodBuilderInfo( method, typeElement );
198+
if ( builderInfo != null ) {
199+
methodBuilderInfos.add( builderInfo );
200+
}
201+
}
202+
else if ( ElementKind.CLASS == enclosedElement.getKind() ) {
203+
if ( !methodBuilderInfos.isEmpty() ) {
204+
// Small optimization to not check the inner classes
205+
// if we already have at least one builder through a method
206+
continue;
207+
}
208+
TypeElement classElement = (TypeElement) enclosedElement;
209+
BuilderInfo builderInfo = determineInnerClassBuilderInfo( classElement, typeElement );
210+
if ( builderInfo != null ) {
211+
innerClassBuilderInfos.add( builderInfo );
212+
}
213+
}
214+
192215
}
193216

194-
BuilderInfo innerBuilderClassBuilderInfo = findInnerBuilderClassBuilderInfo( typeElement );
195-
if ( innerBuilderClassBuilderInfo != null ) {
196-
return innerBuilderClassBuilderInfo;
217+
if ( methodBuilderInfos.size() == 1 ) {
218+
return methodBuilderInfos.get( 0 );
219+
}
220+
else if ( methodBuilderInfos.size() > 1 ) {
221+
throw new MoreThanOneBuilderCreationMethodException( typeElement.asType(), methodBuilderInfos );
222+
}
223+
else if ( innerClassBuilderInfos.size() == 1 ) {
224+
return innerClassBuilderInfos.get( 0 );
225+
}
226+
else if ( innerClassBuilderInfos.size() > 1 ) {
227+
throw new MoreThanOneBuilderCreationMethodException( typeElement.asType(), innerClassBuilderInfos );
197228
}
198229

199230
if ( checkParent ) {
200231
return findBuilderInfo( typeElement.getSuperclass() );
201232
}
233+
202234
return null;
203235
}
204236

205-
private BuilderInfo findStaticFactoryMethodBuilderInfo(TypeElement typeElement) {
206-
List<ExecutableElement> methods = ElementFilter.methodsIn( typeElement.getEnclosedElements() );
207-
List<BuilderInfo> builderInfo = new ArrayList<>();
208-
for ( ExecutableElement method : methods ) {
209-
if ( isPossibleBuilderCreationMethod( method, typeElement ) ) {
210-
TypeElement builderElement = getTypeElement( method.getReturnType() );
211-
Collection<ExecutableElement> buildMethods = findBuildMethods( builderElement, typeElement );
212-
if ( !buildMethods.isEmpty() ) {
213-
builderInfo.add( new BuilderInfo.Builder()
214-
.builderCreationMethod( method )
215-
.buildMethod( buildMethods )
216-
.build()
217-
);
218-
}
237+
private BuilderInfo determineMethodBuilderInfo(ExecutableElement method,
238+
TypeElement typeElement) {
239+
if ( isPossibleBuilderCreationMethod( method, typeElement ) ) {
240+
TypeElement builderElement = getTypeElement( method.getReturnType() );
241+
Collection<ExecutableElement> buildMethods = findBuildMethods( builderElement, typeElement );
242+
if ( !buildMethods.isEmpty() ) {
243+
return new BuilderInfo.Builder()
244+
.builderCreationMethod( method )
245+
.buildMethod( buildMethods )
246+
.build();
219247
}
220248
}
221249

222-
if ( builderInfo.size() == 1 ) {
223-
return builderInfo.get( 0 );
224-
}
225-
else if ( builderInfo.size() > 1 ) {
226-
throw new MoreThanOneBuilderCreationMethodException( typeElement.asType(), builderInfo );
227-
}
228250
return null;
229251
}
230252

231-
private BuilderInfo findInnerBuilderClassBuilderInfo(TypeElement typeElement) {
232-
List<BuilderInfo> builderInfos = typeElement.getEnclosedElements().stream()
233-
.filter( element -> ElementKind.CLASS.equals( element.getKind() ) )
234-
.map( TypeElement.class::cast )
235-
.filter( classElement -> classElement.getModifiers().contains( Modifier.PUBLIC )
236-
&& classElement.getModifiers().contains( Modifier.STATIC ) )
237-
.filter( classElement -> classElement.getSimpleName().toString().endsWith( "Builder" ) )
238-
.flatMap( builderElement -> {
239-
Optional<ExecutableElement> noArgConstructor = ElementFilter.constructorsIn(
240-
builderElement.getEnclosedElements()
241-
).stream()
242-
.filter( executableElement -> executableElement.getParameters().isEmpty() )
243-
.findAny();
244-
if ( !noArgConstructor.isPresent() ) {
245-
return Stream.empty();
253+
private BuilderInfo determineInnerClassBuilderInfo(TypeElement innerClassElement,
254+
TypeElement typeElement) {
255+
if ( innerClassElement.getModifiers().contains( Modifier.PUBLIC )
256+
&& innerClassElement.getModifiers().contains( Modifier.STATIC )
257+
&& innerClassElement.getSimpleName().toString().endsWith( "Builder" ) ) {
258+
for ( Element element : innerClassElement.getEnclosedElements() ) {
259+
if ( ElementKind.CONSTRUCTOR == element.getKind() ) {
260+
ExecutableElement constructor = (ExecutableElement) element;
261+
if ( constructor.getParameters().isEmpty() ) {
262+
// We have a no-arg constructor
263+
// Now check if we have build methods
264+
Collection<ExecutableElement> buildMethods = findBuildMethods( innerClassElement, typeElement );
265+
if ( !buildMethods.isEmpty() ) {
266+
return new BuilderInfo.Builder()
267+
.builderCreationMethod( constructor )
268+
.buildMethod( buildMethods )
269+
.build();
270+
}
271+
// If we don't have any build methods
272+
// then we can stop since we are only interested in the no-arg constructor
273+
return null;
274+
}
246275
}
247-
Collection<ExecutableElement> buildMethods = findBuildMethods( builderElement, typeElement );
248-
if ( buildMethods.isEmpty() ) {
249-
return Stream.empty();
250-
}
251-
return Stream.of( new BuilderInfo.Builder()
252-
.builderCreationMethod( noArgConstructor.get() )
253-
.buildMethod( buildMethods )
254-
.build() );
255-
} )
256-
.collect( Collectors.toList() );
257-
258-
if ( builderInfos.size() > 1 ) {
259-
throw new MoreThanOneBuilderCreationMethodException( typeElement.asType(), builderInfos );
276+
}
260277
}
261278

262-
if ( builderInfos.size() == 1 ) {
263-
return builderInfos.get( 0 );
264-
}
265279
return null;
266280
}
267281

@@ -276,7 +290,7 @@ private BuilderInfo findInnerBuilderClassBuilderInfo(TypeElement typeElement) {
276290
* <li></li>
277291
* </ul>
278292
*
279-
* @param method The method that needs to be checked
293+
* @param method The method that needs to be checked
280294
* @param typeElement the enclosing element of the method, i.e. the type in which the method is located in
281295
* @return {@code true} if the {@code method} is a possible builder creation method, {@code false} otherwise
282296
*/
@@ -304,9 +318,8 @@ protected boolean isPossibleBuilderCreationMethod(ExecutableElement method, Type
304318
* The default implementation uses {@link DefaultBuilderProvider#shouldIgnore(TypeElement)} to check if the
305319
* {@code builderElement} should be ignored, i.e. not checked for build elements.
306320
* <p>
307-
*
308321
* @param builderElement the element for the builder
309-
* @param typeElement the element for the type that is being built
322+
* @param typeElement the element for the type that is being built
310323
* @return the build method for the {@code typeElement} if it exists, or {@code null} if it does not
311324
* {@code build}
312325
*/
@@ -416,7 +429,6 @@ protected boolean isBuildMethod(ExecutableElement buildMethod, DeclaredType buil
416429
* <p>
417430
* The default implementations ignores {@code null} elements and elements that belong to the {@code java} and
418431
* {@code javax} packages
419-
*
420432
* @param typeElement that needs to be checked
421433
* @return {@code true} if the element should be ignored, {@code false} otherwise
422434
*/

processor/src/test/java/org/mapstruct/ap/test/builder/simple/innerclass/SimpleMutablePerson.java renamed to processor/src/test/java/org/mapstruct/ap/test/builder/simple/SimpleMutablePerson.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*
44
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
55
*/
6-
package org.mapstruct.ap.test.builder.simple.innerclass;
6+
package org.mapstruct.ap.test.builder.simple;
77

88
import java.util.List;
99

processor/src/test/java/org/mapstruct/ap/test/builder/simple/innerclass/ErroneousSimpleBuilderMapper.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.mapstruct.Mapping;
1010
import org.mapstruct.Mappings;
1111
import org.mapstruct.ReportingPolicy;
12+
import org.mapstruct.ap.test.builder.simple.SimpleMutablePerson;
1213

1314
@Mapper(unmappedTargetPolicy = ReportingPolicy.ERROR)
1415
public interface ErroneousSimpleBuilderMapper {

processor/src/test/java/org/mapstruct/ap/test/builder/simple/innerclass/SimpleBuilderMapper.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.mapstruct.Mapper;
1010
import org.mapstruct.Mapping;
1111
import org.mapstruct.Mappings;
12+
import org.mapstruct.ap.test.builder.simple.SimpleMutablePerson;
1213

1314
@Mapper(collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED)
1415
public interface SimpleBuilderMapper {

processor/src/test/java/org/mapstruct/ap/test/builder/simple/innerclass/SimpleImmutableBuilderThroughInnerClassConstructorTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.Arrays;
99

1010
import org.junit.jupiter.api.extension.RegisterExtension;
11+
import org.mapstruct.ap.test.builder.simple.SimpleMutablePerson;
1112
import org.mapstruct.ap.testutil.ProcessorTest;
1213
import org.mapstruct.ap.testutil.WithClasses;
1314
import org.mapstruct.ap.testutil.compilation.annotation.CompilationResult;
@@ -53,7 +54,7 @@ public void testSimpleImmutableBuilderThroughInnerClassConstructorHappyPath() {
5354
diagnostics = @Diagnostic(
5455
kind = javax.tools.Diagnostic.Kind.ERROR,
5556
type = ErroneousSimpleBuilderMapper.class,
56-
line = 21,
57+
line = 22,
5758
message = "Unmapped target property: \"name\"."))
5859
public void testSimpleImmutableBuilderThroughInnerClassConstructorMissingPropertyFailsToCompile() {
5960
}

processor/src/test/java/org/mapstruct/ap/test/builder/simple/staticfactorymethod/ErroneousSimpleBuilderMapper.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.mapstruct.Mapping;
1010
import org.mapstruct.Mappings;
1111
import org.mapstruct.ReportingPolicy;
12+
import org.mapstruct.ap.test.builder.simple.SimpleMutablePerson;
1213

1314
@Mapper(unmappedTargetPolicy = ReportingPolicy.ERROR)
1415
public interface ErroneousSimpleBuilderMapper {

processor/src/test/java/org/mapstruct/ap/test/builder/simple/staticfactorymethod/SimpleBuilderMapper.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import org.mapstruct.Mapper;
1010
import org.mapstruct.Mapping;
1111
import org.mapstruct.Mappings;
12+
import org.mapstruct.ap.test.builder.simple.SimpleMutablePerson;
1213

1314
@Mapper(collectionMappingStrategy = CollectionMappingStrategy.ADDER_PREFERRED)
1415
public interface SimpleBuilderMapper {

processor/src/test/java/org/mapstruct/ap/test/builder/simple/staticfactorymethod/SimpleImmutableBuilderThroughStaticFactoryMethodTest.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import java.util.Arrays;
99

1010
import org.junit.jupiter.api.extension.RegisterExtension;
11+
import org.mapstruct.ap.test.builder.simple.SimpleMutablePerson;
1112
import org.mapstruct.ap.testutil.ProcessorTest;
1213
import org.mapstruct.ap.testutil.WithClasses;
1314
import org.mapstruct.ap.testutil.compilation.annotation.CompilationResult;
@@ -53,7 +54,7 @@ public void testSimpleImmutableBuilderThroughStaticFactoryMethodHappyPath() {
5354
diagnostics = @Diagnostic(
5455
kind = javax.tools.Diagnostic.Kind.ERROR,
5556
type = ErroneousSimpleBuilderMapper.class,
56-
line = 21,
57+
line = 22,
5758
message = "Unmapped target property: \"name\"."))
5859
public void testSimpleImmutableBuilderThroughStaticFactoryMethodMissingPropertyFailsToCompile() {
5960
}

processor/src/test/java/org/mapstruct/ap/test/builder/simple/staticfactorymethod/SimpleMutablePerson.java

Lines changed: 0 additions & 47 deletions
This file was deleted.

0 commit comments

Comments
 (0)