From 4e10c8451c5bcd8b5fcb3faaf4e8ef4bd95a1ed0 Mon Sep 17 00:00:00 2001 From: sjaakd Date: Wed, 4 Feb 2015 23:10:37 +0100 Subject: [PATCH] #194 Only generate field / imports for a referenced mapper if it is actually used --- .../org/mapstruct/ap/model/Constructor.java | 30 +++++ .../org/mapstruct/ap/model/Decorator.java | 112 +++++++++++++----- .../ap/model/DecoratorConstructor.java | 5 +- .../java/org/mapstruct/ap/model/Field.java | 19 +++ .../org/mapstruct/ap/model/GeneratedType.java | 21 +++- .../java/org/mapstruct/ap/model/Mapper.java | 3 +- .../ap/processor/MapperCreationProcessor.java | 22 ++-- .../creation/MappingResolverImpl.java | 1 + ...uct.ap.model.AnnotationMapperReference.ftl | 4 +- ...struct.ap.model.DefaultMapperReference.ftl | 2 +- .../org.mapstruct.ap.model.Field.ftl | 2 +- .../org.mapstruct.ap.model.GeneratedType.ftl | 7 +- .../statics/BeerMapperWithNonUsedMapper.java | 53 +++++++++ .../test/references/statics/StaticsTest.java | 28 ++++- .../statics/nonused/NonUsedMapper.java | 27 +++++ 15 files changed, 278 insertions(+), 58 deletions(-) create mode 100644 processor/src/main/java/org/mapstruct/ap/model/Constructor.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/references/statics/BeerMapperWithNonUsedMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/references/statics/nonused/NonUsedMapper.java diff --git a/processor/src/main/java/org/mapstruct/ap/model/Constructor.java b/processor/src/main/java/org/mapstruct/ap/model/Constructor.java new file mode 100644 index 000000000..cf6456659 --- /dev/null +++ b/processor/src/main/java/org/mapstruct/ap/model/Constructor.java @@ -0,0 +1,30 @@ +/** + * Copyright 2012-2015 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.model; + +/** + * Basic interface class that facilitates an empty constructor + * + * @author Sjaak Derksen + */ +public interface Constructor { + + String getName(); + +} diff --git a/processor/src/main/java/org/mapstruct/ap/model/Decorator.java b/processor/src/main/java/org/mapstruct/ap/model/Decorator.java index 4671c2e96..50af2b58b 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/Decorator.java +++ b/processor/src/main/java/org/mapstruct/ap/model/Decorator.java @@ -27,7 +27,6 @@ import javax.lang.model.element.TypeElement; import javax.lang.model.util.Elements; import org.mapstruct.ap.model.common.Accessibility; -import org.mapstruct.ap.model.common.ModelElement; import org.mapstruct.ap.model.common.Type; import org.mapstruct.ap.model.common.TypeFactory; import org.mapstruct.ap.option.Options; @@ -43,9 +42,86 @@ public class Decorator extends GeneratedType { private static final String IMPLEMENTATION_SUFFIX = "Impl"; + public static class Builder { + + private Elements elementUtils; + private TypeFactory typeFactory; + private TypeElement mapperElement; + private DecoratedWithPrism decoratorPrism; + private List methods; + private Options options; + private VersionInformation versionInformation; + private boolean hasDelegateConstructor; + + public Builder elementUtils(Elements elementUtils) { + this.elementUtils = elementUtils; + return this; + } + + public Builder typeFactory(TypeFactory typeFactory) { + this.typeFactory = typeFactory; + return this; + } + + public Builder mapperElement(TypeElement mapperElement) { + this.mapperElement = mapperElement; + return this; + } + + public Builder decoratorPrism(DecoratedWithPrism decoratorPrism) { + this.decoratorPrism = decoratorPrism; + return this; + } + + public Builder methods(List methods) { + this.methods = methods; + return this; + } + + public Builder options(Options options) { + this.options = options; + return this; + } + + public Builder versionInformation(VersionInformation versionInformation) { + this.versionInformation = versionInformation; + return this; + } + + public Builder hasDelegateConstructor(boolean hasDelegateConstructor) { + this.hasDelegateConstructor = hasDelegateConstructor; + return this; + } + + public Decorator build() { + Type decoratorType = typeFactory.getType( decoratorPrism.value() ); + DecoratorConstructor decoratorConstructor = new DecoratorConstructor( + mapperElement.getSimpleName().toString() + IMPLEMENTATION_SUFFIX, + mapperElement.getSimpleName().toString() + "Impl_", + hasDelegateConstructor ); + + + return new Decorator( + typeFactory, + elementUtils.getPackageOf( mapperElement ).getQualifiedName().toString(), + mapperElement.getSimpleName().toString() + IMPLEMENTATION_SUFFIX, + decoratorType.getName(), + mapperElement.getKind() == ElementKind.INTERFACE ? mapperElement.getSimpleName().toString() : null, + methods, + Arrays.asList( new Field( typeFactory.getType( mapperElement ), "delegate", true ) ) , + options, + versionInformation, + Accessibility.fromModifiers( mapperElement.getModifiers() ), + decoratorConstructor + ); + } + } + + @SuppressWarnings( "checkstyle:parameternumber" ) private Decorator(TypeFactory typeFactory, String packageName, String name, String superClassName, - String interfaceName, List methods, List fields, - Options options, VersionInformation versionInformation, Accessibility accessibility) { + String interfaceName, List methods, List fields, + Options options, VersionInformation versionInformation, Accessibility accessibility, + DecoratorConstructor decoratorConstructor) { super( typeFactory, packageName, @@ -57,34 +133,8 @@ public class Decorator extends GeneratedType { options, versionInformation, accessibility, - new TreeSet() - ); - } - - public static Decorator getInstance(Elements elementUtils, TypeFactory typeFactory, TypeElement mapperElement, - DecoratedWithPrism decoratorPrism, List methods, - boolean hasDelegateConstructor, Options options, - VersionInformation versionInformation) { - Type decoratorType = typeFactory.getType( decoratorPrism.value() ); - - return new Decorator( - typeFactory, - elementUtils.getPackageOf( mapperElement ).getQualifiedName().toString(), - mapperElement.getSimpleName().toString() + IMPLEMENTATION_SUFFIX, - decoratorType.getName(), - mapperElement.getKind() == ElementKind.INTERFACE ? mapperElement.getSimpleName().toString() : null, - methods, - Arrays.asList( - new Field( typeFactory.getType( mapperElement ), "delegate", true ), - new DecoratorConstructor( - mapperElement.getSimpleName().toString() + IMPLEMENTATION_SUFFIX, - mapperElement.getSimpleName().toString() + "Impl_", - hasDelegateConstructor - ) - ), - options, - versionInformation, - Accessibility.fromModifiers( mapperElement.getModifiers() ) + new TreeSet(), + decoratorConstructor ); } diff --git a/processor/src/main/java/org/mapstruct/ap/model/DecoratorConstructor.java b/processor/src/main/java/org/mapstruct/ap/model/DecoratorConstructor.java index dca708b55..8c2b3d075 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/DecoratorConstructor.java +++ b/processor/src/main/java/org/mapstruct/ap/model/DecoratorConstructor.java @@ -20,8 +20,8 @@ package org.mapstruct.ap.model; import java.util.Collections; import java.util.Set; - import org.mapstruct.ap.model.common.ModelElement; + import org.mapstruct.ap.model.common.Type; /** @@ -29,7 +29,7 @@ import org.mapstruct.ap.model.common.Type; * * @author Gunnar Morling */ -public class DecoratorConstructor extends ModelElement { +public class DecoratorConstructor extends ModelElement implements Constructor { private final String name; private final String delegateName; @@ -46,6 +46,7 @@ public class DecoratorConstructor extends ModelElement { return Collections.emptySet(); } + @Override public String getName() { return name; } diff --git a/processor/src/main/java/org/mapstruct/ap/model/Field.java b/processor/src/main/java/org/mapstruct/ap/model/Field.java index 1ba5a526b..4e2f2b566 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/Field.java +++ b/processor/src/main/java/org/mapstruct/ap/model/Field.java @@ -34,16 +34,19 @@ public class Field extends ModelElement { private final Type type; private final String variableName; private boolean used; + private boolean typeRequiresImport; public Field(Type type, String variableName, boolean used) { this.type = type; this.variableName = variableName; this.used = used; + this.typeRequiresImport = used; } public Field(Type type, String variableName) { this.type = type; this.variableName = variableName; this.used = false; + this.typeRequiresImport = false; } /** @@ -85,4 +88,20 @@ public class Field extends ModelElement { this.used = isUsed; } + /** + * field needs to be imported + * @return true if the type should be included in the import of the generated type + */ + public boolean isTypeRequiresImport() { + return typeRequiresImport; + } + + /** + * set field needs to be imported + * @param typeRequiresImport needs to be imported + */ + public void setTypeRequiresImport(boolean typeRequiresImport) { + this.typeRequiresImport = typeRequiresImport; + } + } diff --git a/processor/src/main/java/org/mapstruct/ap/model/GeneratedType.java b/processor/src/main/java/org/mapstruct/ap/model/GeneratedType.java index 1124b8403..244e6082c 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/GeneratedType.java +++ b/processor/src/main/java/org/mapstruct/ap/model/GeneratedType.java @@ -49,13 +49,14 @@ public abstract class GeneratedType extends ModelElement { private final List annotations; private final List methods; - private final List fields; + private final List fields; private final SortedSet extraImportedTypes; private final boolean suppressGeneratorTimestamp; private final boolean suppressGeneratorVersionComment; private final VersionInformation versionInformation; private final Accessibility accessibility; + private final Constructor constructor; /** * Type representing the {@code @Generated} annotation @@ -66,11 +67,12 @@ public abstract class GeneratedType extends ModelElement { protected GeneratedType(TypeFactory typeFactory, String packageName, String name, String superClassName, String interfaceName, List methods, - List fields, + List fields, Options options, VersionInformation versionInformation, Accessibility accessibility, - SortedSet extraImportedTypes) { + SortedSet extraImportedTypes, + Constructor constructor ) { this.packageName = packageName; this.name = name; this.superClassName = superClassName; @@ -87,6 +89,7 @@ public abstract class GeneratedType extends ModelElement { this.accessibility = accessibility; this.generatedType = typeFactory.getType( Generated.class ); + this.constructor = constructor; } // CHECKSTYLE:ON @@ -150,9 +153,11 @@ public abstract class GeneratedType extends ModelElement { } } - for ( ModelElement field : fields ) { - for ( Type type : field.getImportTypes() ) { - addWithDependents( importedTypes, type ); + for ( Field field : fields ) { + if ( field.isTypeRequiresImport() ) { + for ( Type type : field.getImportTypes() ) { + addWithDependents( importedTypes, type ); + } } } @@ -167,6 +172,10 @@ public abstract class GeneratedType extends ModelElement { return importedTypes; } + public Constructor getConstructor() { + return constructor; + } + private void addWithDependents(Collection collection, Type typeToAdd) { if ( typeToAdd == null ) { return; diff --git a/processor/src/main/java/org/mapstruct/ap/model/Mapper.java b/processor/src/main/java/org/mapstruct/ap/model/Mapper.java index a898849f0..660fb72f9 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/Mapper.java +++ b/processor/src/main/java/org/mapstruct/ap/model/Mapper.java @@ -63,7 +63,8 @@ public class Mapper extends GeneratedType { options, versionInformation, accessibility, - extraImportedTypes + extraImportedTypes, + null ); this.referencedMappers = referencedMappers; diff --git a/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java b/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java index 35a489748..3844c536e 100644 --- a/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java +++ b/processor/src/main/java/org/mapstruct/ap/processor/MapperCreationProcessor.java @@ -205,16 +205,18 @@ public class MapperCreationProcessor implements ModelElementProcessor getExtraImports(TypeElement element) { diff --git a/processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolverImpl.java b/processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolverImpl.java index 41eefc58d..2ac5c07f2 100755 --- a/processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolverImpl.java +++ b/processor/src/main/java/org/mapstruct/ap/processor/creation/MappingResolverImpl.java @@ -502,6 +502,7 @@ public class MappingResolverImpl implements MappingResolver { for ( MapperReference ref : mapperReferences ) { if ( ref.getType().equals( method.getDeclaringMapper() ) ) { ref.setUsed( !method.isStatic() ); + ref.setTypeRequiresImport( true ); return ref; } } diff --git a/processor/src/main/resources/org.mapstruct.ap.model.AnnotationMapperReference.ftl b/processor/src/main/resources/org.mapstruct.ap.model.AnnotationMapperReference.ftl index 649107d6c..1898e51ac 100644 --- a/processor/src/main/resources/org.mapstruct.ap.model.AnnotationMapperReference.ftl +++ b/processor/src/main/resources/org.mapstruct.ap.model.AnnotationMapperReference.ftl @@ -18,5 +18,5 @@ limitations under the License. --> -<#if used><#nt><@includeModel object=annotation/> -private <@includeModel object=type/> ${variableName}; \ No newline at end of file +<#nt><@includeModel object=annotation/> +private <@includeModel object=type/> ${variableName}; \ No newline at end of file diff --git a/processor/src/main/resources/org.mapstruct.ap.model.DefaultMapperReference.ftl b/processor/src/main/resources/org.mapstruct.ap.model.DefaultMapperReference.ftl index 91a1ce21e..176916cec 100644 --- a/processor/src/main/resources/org.mapstruct.ap.model.DefaultMapperReference.ftl +++ b/processor/src/main/resources/org.mapstruct.ap.model.DefaultMapperReference.ftl @@ -18,4 +18,4 @@ limitations under the License. --> -<#if used>private final <@includeModel object=type/> ${variableName} = <#if annotatedMapper>Mappers.getMapper( <@includeModel object=type/>.class );<#else>new <@includeModel object=type/>(); \ No newline at end of file +private final <@includeModel object=type/> ${variableName} = <#if annotatedMapper>Mappers.getMapper( <@includeModel object=type/>.class );<#else>new <@includeModel object=type/>(); \ No newline at end of file diff --git a/processor/src/main/resources/org.mapstruct.ap.model.Field.ftl b/processor/src/main/resources/org.mapstruct.ap.model.Field.ftl index 1c2bd07fe..aa81400e1 100644 --- a/processor/src/main/resources/org.mapstruct.ap.model.Field.ftl +++ b/processor/src/main/resources/org.mapstruct.ap.model.Field.ftl @@ -18,4 +18,4 @@ limitations under the License. --> -<#if used>private final <@includeModel object=type/> ${variableName}; \ No newline at end of file +private final <@includeModel object=type/> ${variableName}; \ No newline at end of file diff --git a/processor/src/main/resources/org.mapstruct.ap.model.GeneratedType.ftl b/processor/src/main/resources/org.mapstruct.ap.model.GeneratedType.ftl index 1b60e51f2..b6af86c22 100644 --- a/processor/src/main/resources/org.mapstruct.ap.model.GeneratedType.ftl +++ b/processor/src/main/resources/org.mapstruct.ap.model.GeneratedType.ftl @@ -34,9 +34,10 @@ import ${importedType.importName}; <#lt>${accessibility.keyword} class ${name}<#if superClassName??> extends ${superClassName}<#if interfaceName??> implements ${interfaceName} { -<#list fields as field> -<#nt> <@includeModel object=field/> - +<#list fields as field><#if field.used><#nt> <@includeModel object=field/> + + +<#if constructor??><#nt> <@includeModel object=constructor/> <#list methods as method> <#nt> <@includeModel object=method/> diff --git a/processor/src/test/java/org/mapstruct/ap/test/references/statics/BeerMapperWithNonUsedMapper.java b/processor/src/test/java/org/mapstruct/ap/test/references/statics/BeerMapperWithNonUsedMapper.java new file mode 100644 index 000000000..a273a9a7b --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/references/statics/BeerMapperWithNonUsedMapper.java @@ -0,0 +1,53 @@ +/** + * Copyright 2012-2015 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.references.statics; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.ap.test.references.statics.nonused.NonUsedMapper; +import org.mapstruct.factory.Mappers; + +/** + * + * @author Sjaak Derksen + */ +@Mapper(uses = NonUsedMapper.class ) +public abstract class BeerMapperWithNonUsedMapper { + + public static final BeerMapperWithNonUsedMapper INSTANCE = Mappers.getMapper( BeerMapperWithNonUsedMapper.class ); + + @Mapping( target = "category", source = "percentage") + public abstract BeerDto mapBeer(Beer beer); + + public static Category toCategory(float in) { + + if ( in < 2.5 ) { + return Category.LIGHT; + } + else if ( in < 5.5 ) { + return Category.LAGER; + } + else if ( in < 10 ) { + return Category.STRONG; + } + else { + return Category.BARLEY_WINE; + } + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/references/statics/StaticsTest.java b/processor/src/test/java/org/mapstruct/ap/test/references/statics/StaticsTest.java index 820320a33..b6cc69a84 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/references/statics/StaticsTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/references/statics/StaticsTest.java @@ -23,18 +23,29 @@ import org.mapstruct.ap.testutil.IssueKey; import org.mapstruct.ap.testutil.WithClasses; import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; import static org.fest.assertions.Assertions.assertThat; +import org.junit.Rule; import org.junit.Test; +import org.mapstruct.ap.test.references.statics.nonused.NonUsedMapper; +import org.mapstruct.ap.testutil.runner.GeneratedSource; /** * * @author Sjaak Derksen */ @IssueKey( "410" ) -@WithClasses( { Beer.class, BeerDto.class, BeerMapper.class, Category.class, CustomMapper.class } ) +@WithClasses( { Beer.class, BeerDto.class, Category.class } ) @RunWith(AnnotationProcessorTestRunner.class) public class StaticsTest { + private final GeneratedSource generatedSource = new GeneratedSource(); + + @Rule + public GeneratedSource getGeneratedSource() { + return generatedSource; + } + @Test + @WithClasses( { BeerMapper.class, CustomMapper.class } ) public void shouldUseStaticMethod() { Beer beer = new Beer(); // what the heck, open another one.. @@ -44,4 +55,19 @@ public class StaticsTest { assertThat( result ).isNotNull(); assertThat( result.getCategory() ).isEqualTo( Category.STRONG ); // why settle for less? } + + @Test + @WithClasses( { BeerMapperWithNonUsedMapper.class, NonUsedMapper.class } ) + public void shouldNotImportNonUsed() { + + Beer beer = new Beer(); // what the heck, open another one.. + beer.setPercentage( 7 ); + + BeerDto result = BeerMapperWithNonUsedMapper.INSTANCE.mapBeer( beer ); + assertThat( result ).isNotNull(); + assertThat( result.getCategory() ).isEqualTo( Category.STRONG ); // I could shurly use one now.. + generatedSource.forMapper( BeerMapperWithNonUsedMapper.class ).containsNoImportFor( NonUsedMapper.class ); + + + } } diff --git a/processor/src/test/java/org/mapstruct/ap/test/references/statics/nonused/NonUsedMapper.java b/processor/src/test/java/org/mapstruct/ap/test/references/statics/nonused/NonUsedMapper.java new file mode 100644 index 000000000..19b084293 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/references/statics/nonused/NonUsedMapper.java @@ -0,0 +1,27 @@ +/** + * Copyright 2012-2015 Gunnar Morling (http://www.gunnarmorling.de/) + * and/or other contributors as indicated by the @authors tag. See the + * copyright.txt file in the distribution for a full listing of all + * contributors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.mapstruct.ap.test.references.statics.nonused; + +/** + * + * @author Sjaak Derksen + */ +public class NonUsedMapper { + +}