#304 Raising an error in case a cycle is specified via dependsOn()

processor/src/main/java/org/mapstruct/ap/model/BeanMappingMethod.java

@@ -177,23 +177,36 @@ public class BeanMappingMethod extends MappingMethod {
 
             graphAnalyzer.analyze();
 
-            Collections.sort(
+            if ( !graphAnalyzer.getCycles().isEmpty() ) {
-                propertyMappings, new Comparator<PropertyMapping>() {
+                Set<String> cycles = new HashSet<String>( graphAnalyzer.getCycles().size() );
+                for ( List<String> cycle : graphAnalyzer.getCycles() ) {
+                    cycles.add( Strings.join( cycle, " -> " ) );
+                }
 
-                    @Override
+                ctx.getMessager().printMessage(
-                    public int compare(PropertyMapping o1, PropertyMapping o2) {
+                    method.getExecutable(),
-                        if ( graphAnalyzer.getAllDescendants( o1.getName() ).contains( o2.getName() ) ) {
+                    Message.BEANMAPPING_CYCLE_BETWEEN_PROPERTIES, Strings.join( cycles, ", " )
-                            return 1;
+                );
-                        }
+            }
-                        else if ( graphAnalyzer.getAllDescendants( o2.getName() ).contains( o1.getName() ) ) {
+            else {
-                            return -1;
+                Collections.sort(
-                        }
+                    propertyMappings, new Comparator<PropertyMapping>() {
-                        else {
+
-                            return 0;
+                        @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;
+                            }
                         }
                     }
-                }
+                );
-            );
+            }
         }
 
         /**

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." ),

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);
+}

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() {
+    }
 }