From 70ea65f7aa5518cac236c277bf040b058907f713 Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Sun, 16 May 2021 17:39:26 +0200 Subject: [PATCH] #2437 Do not visit the same type element twice when retrieving methods When using the diamond inheritance a TypeElement might be visited twice. This is however, incorrect. We need to visit a TypeElement exactly once. --- .../util/AbstractElementUtilsDecorator.java | 15 ++++++++-- .../ap/test/bugs/_2437/Issue2437Test.java | 30 +++++++++++++++++++ .../mapstruct/ap/test/bugs/_2437/Phone.java | 22 ++++++++++++++ .../ap/test/bugs/_2437/PhoneDto.java | 22 ++++++++++++++ .../ap/test/bugs/_2437/PhoneMapper.java | 15 ++++++++++ .../test/bugs/_2437/PhoneParent1Mapper.java | 12 ++++++++ .../test/bugs/_2437/PhoneParent2Mapper.java | 12 ++++++++ .../ap/test/bugs/_2437/PhoneSuperMapper.java | 14 +++++++++ 8 files changed, 140 insertions(+), 2 deletions(-) create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/Issue2437Test.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/Phone.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneDto.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneMapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneParent1Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneParent2Mapper.java create mode 100644 processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneSuperMapper.java diff --git a/processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java b/processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java index 8239dcb41..14734b9c8 100644 --- a/processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java +++ b/processor/src/main/java/org/mapstruct/ap/internal/util/AbstractElementUtilsDecorator.java @@ -7,6 +7,8 @@ package org.mapstruct.ap.internal.util; import java.io.Writer; import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; import java.util.List; import java.util.ListIterator; import java.util.Map; @@ -118,7 +120,7 @@ public abstract class AbstractElementUtilsDecorator implements ElementUtils { public List getAllEnclosedExecutableElements(TypeElement element) { List enclosedElements = new ArrayList<>(); element = replaceTypeElementIfNecessary( element ); - addEnclosedMethodsInHierarchy( enclosedElements, element, element ); + addEnclosedMethodsInHierarchy( enclosedElements, new HashSet<>(), element, element ); return enclosedElements; } @@ -132,7 +134,9 @@ public abstract class AbstractElementUtilsDecorator implements ElementUtils { return enclosedElements; } - private void addEnclosedMethodsInHierarchy(List alreadyAdded, TypeElement element, + private void addEnclosedMethodsInHierarchy(List alreadyAdded, + Collection alreadyVisitedElements, + TypeElement element, TypeElement parentType) { if ( element != parentType ) { // otherwise the element was already checked for replacement element = replaceTypeElementIfNecessary( element ); @@ -142,11 +146,17 @@ public abstract class AbstractElementUtilsDecorator implements ElementUtils { throw new TypeHierarchyErroneousException( element ); } + if ( !alreadyVisitedElements.add( element.getQualifiedName().toString() ) ) { + // If we already visited the element we should not go into it again. + // This can happen when diamond inheritance is used with interfaces + return; + } addMethodNotYetOverridden( alreadyAdded, methodsIn( element.getEnclosedElements() ), parentType ); if ( hasNonObjectSuperclass( element ) ) { addEnclosedMethodsInHierarchy( alreadyAdded, + alreadyVisitedElements, asTypeElement( element.getSuperclass() ), parentType ); @@ -155,6 +165,7 @@ public abstract class AbstractElementUtilsDecorator implements ElementUtils { for ( TypeMirror interfaceType : element.getInterfaces() ) { addEnclosedMethodsInHierarchy( alreadyAdded, + alreadyVisitedElements, asTypeElement( interfaceType ), parentType ); diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/Issue2437Test.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/Issue2437Test.java new file mode 100644 index 000000000..343b8fbf1 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/Issue2437Test.java @@ -0,0 +1,30 @@ +/* + * 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._2437; + +import org.mapstruct.ap.testutil.IssueKey; +import org.mapstruct.ap.testutil.ProcessorTest; +import org.mapstruct.ap.testutil.WithClasses; + +/** + * @author Filip Hrisafov + */ +@IssueKey("2437") +@WithClasses({ + Phone.class, + PhoneDto.class, + PhoneMapper.class, + PhoneParent1Mapper.class, + PhoneParent2Mapper.class, + PhoneSuperMapper.class, +}) +class Issue2437Test { + + @ProcessorTest + void shouldGenerateValidCode() { + + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/Phone.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/Phone.java new file mode 100644 index 000000000..98027861f --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/Phone.java @@ -0,0 +1,22 @@ +/* + * 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._2437; + +/** + * @author Filip Hrisafov + */ +public class Phone { + + private final String number; + + public Phone(String number) { + this.number = number; + } + + public String getNumber() { + return number; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneDto.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneDto.java new file mode 100644 index 000000000..883e086e8 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneDto.java @@ -0,0 +1,22 @@ +/* + * 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._2437; + +/** + * @author Filip Hrisafov + */ +public class PhoneDto { + + private final String number; + + public PhoneDto(String number) { + this.number = number; + } + + public String getNumber() { + return number; + } +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneMapper.java new file mode 100644 index 000000000..16b01658b --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneMapper.java @@ -0,0 +1,15 @@ +/* + * 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._2437; + +import org.mapstruct.Mapper; + +/** + * @author Filip Hrisafov + */ +@Mapper +public interface PhoneMapper extends PhoneParent1Mapper, PhoneParent2Mapper { +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneParent1Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneParent1Mapper.java new file mode 100644 index 000000000..0b4da27c1 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneParent1Mapper.java @@ -0,0 +1,12 @@ +/* + * 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._2437; + +/** + * @author Filip Hrisafov + */ +public interface PhoneParent1Mapper extends PhoneSuperMapper { +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneParent2Mapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneParent2Mapper.java new file mode 100644 index 000000000..77e9db623 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneParent2Mapper.java @@ -0,0 +1,12 @@ +/* + * 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._2437; + +/** + * @author Filip Hrisafov + */ +public interface PhoneParent2Mapper extends PhoneSuperMapper { +} diff --git a/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneSuperMapper.java b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneSuperMapper.java new file mode 100644 index 000000000..ba35b21d9 --- /dev/null +++ b/processor/src/test/java/org/mapstruct/ap/test/bugs/_2437/PhoneSuperMapper.java @@ -0,0 +1,14 @@ +/* + * 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._2437; + +/** + * @author Filip Hrisafov + */ +public interface PhoneSuperMapper { + + Phone map(PhoneDto dto); +}