From a332533dda0281f691b781f35da366aecca14601 Mon Sep 17 00:00:00 2001 From: Gunnar Morling Date: Sun, 1 Mar 2015 19:18:28 +0100 Subject: [PATCH] #304 Raising an error in case a cycle is specified via dependsOn() --- .../mapstruct/ap/model/BeanMappingMethod.java | 41 ++++++++++++------- .../java/org/mapstruct/ap/util/Message.java | 1 + .../AddressMapperWithCyclicDependency.java | 37 +++++++++++++++++ .../ap/test/dependency/OrderingTest.java | 20 +++++++++ 4 files changed, 85 insertions(+), 14 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/dependency/AddressMapperWithCyclicDependency.java diff --git a/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java b/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java index dbbfcff7b..4fed8e94f 100644 --- a/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java +++ b/processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java @@ -177,23 +177,36 @@ public class BeanMappingMethod extends MappingMethod { graphAnalyzer.analyze(); - Collections.sort( - propertyMappings, new Comparator() { + if ( !graphAnalyzer.getCycles().isEmpty() ) { + Set cycles = new HashSet( graphAnalyzer.getCycles().size() ); + for ( List cycle : graphAnalyzer.getCycles() ) { + cycles.add( Strings.join( cycle, " -> " ) ); + } - @Override - public int compare(PropertyMapping o1, PropertyMapping o2) { - if ( graphAnalyzer.getAllDescendants( o1.getName() ).contains( o2.getName() ) ) { - return 1; - } - else if ( graphAnalyzer.getAllDescendants( o2.getName() ).contains( o1.getName() ) ) { - return -1; - } - else { - return 0; + ctx.getMessager().printMessage( + method.getExecutable(), + Message.BEANMAPPING_CYCLE_BETWEEN_PROPERTIES, Strings.join( cycles, ", " ) + ); + } + else { + Collections.sort( + propertyMappings, new Comparator() { + + @Override + public int compare(PropertyMapping o1, PropertyMapping o2) { + if ( graphAnalyzer.getAllDescendants( o1.getName() ).contains( o2.getName() ) ) { + return 1; + } + else if ( graphAnalyzer.getAllDescendants( o2.getName() ).contains( o1.getName() ) ) { + return -1; + } + else { + return 0; + } } } - } - ); + ); + } } /** diff --git a/processor/src/main/java/org/mapstruct/ap/util/Message.java b/processor/src/main/java/org/mapstruct/ap/util/Message.java index 7f75126ce..356a74dce 100644 --- a/processor/src/main/java/org/mapstruct/ap/util/Message.java +++ b/processor/src/main/java/org/mapstruct/ap/util/Message.java @@ -35,6 +35,7 @@ public enum Message { BEANMAPPING_SEVERAL_POSSIBLE_TARGET_ACCESSORS( "Found several matching getters for property \"%s\"." ), BEANMAPPING_UNMAPPED_TARGETS_WARNING( "Unmapped target %s.", Diagnostic.Kind.WARNING ), BEANMAPPING_UNMAPPED_TARGETS_ERROR( "Unmapped target %s." ), + BEANMAPPING_CYCLE_BETWEEN_PROPERTIES( "Cycle(s) between properties given via dependsOn(): %s." ), PROPERTYMAPPING_MAPPING_NOT_FOUND( "Can't map %s to \"%s %s\". Consider to declare/implement a mapping method: \"%s map(%s value)\"." ), PROPERTYMAPPING_DUPLICATE_TARGETS( "Target property \"%s\" must not be mapped more than once." ), diff --git a/processor/src/test/java/org/mapstruct/ap/test/dependency/AddressMapperWithCyclicDependency.java b/processor/src/test/java/org/mapstruct/ap/test/dependency/AddressMapperWithCyclicDependency.java new file mode 100644 index 000000000..cb74b690e --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/dependency/AddressMapperWithCyclicDependency.java @@ -0,0 +1,37 @@ +/** + * 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.dependency; + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.Mappings; +import org.mapstruct.factory.Mappers; + +@Mapper +public interface AddressMapperWithCyclicDependency { + + AddressMapperWithCyclicDependency INSTANCE = Mappers.getMapper( AddressMapperWithCyclicDependency.class ); + + @Mappings({ + @Mapping(target = "lastName", dependsOn = "middleName"), + @Mapping(target = "middleName", dependsOn = "firstName"), + @Mapping(target = "firstName", dependsOn = "lastName") + }) + PersonDto personToDto(Person person); +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/dependency/OrderingTest.java b/processor/src/test/java/org/mapstruct/ap/test/dependency/OrderingTest.java index b7feb526c..22aa99dcf 100644 --- a/processor/src/test/java/org/mapstruct/ap/test/dependency/OrderingTest.java +++ b/processor/src/test/java/org/mapstruct/ap/test/dependency/OrderingTest.java @@ -23,6 +23,9 @@ import org.junit.runner.RunWith; import org.mapstruct.Mapping; import org.mapstruct.ap.testutil.IssueKey; import org.mapstruct.ap.testutil.WithClasses; +import org.mapstruct.ap.testutil.compilation.annotation.CompilationResult; +import org.mapstruct.ap.testutil.compilation.annotation.Diagnostic; +import org.mapstruct.ap.testutil.compilation.annotation.ExpectedCompilationOutcome; import org.mapstruct.ap.testutil.runner.AnnotationProcessorTestRunner; import static org.fest.assertions.Assertions.assertThat; @@ -63,4 +66,21 @@ public class OrderingTest { assertThat( target ).isNotNull(); assertThat( target.getFullName() ).isEqualTo( "Bob J. McRobb" ); } + + @Test + @IssueKey("304") + @WithClasses(AddressMapperWithCyclicDependency.class) + @ExpectedCompilationOutcome( + value = CompilationResult.FAILED, + diagnostics = { + @Diagnostic(type = AddressMapperWithCyclicDependency.class, + kind = javax.tools.Diagnostic.Kind.ERROR, + line = 36, + messageRegExp = "Cycle\\(s\\) between properties given via dependsOn\\(\\): firstName -> lastName -> " + + "middleName -> firstName" + ) + } + ) + public void shouldReportErrorIfDependenciesContainCycle() { + } }