#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.
This commit is contained in:
Filip Hrisafov 2021-05-16 17:39:26 +02:00
parent a6ac4f3fd6
commit 70ea65f7aa
8 changed files with 140 additions and 2 deletions

View File

@ -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<ExecutableElement> getAllEnclosedExecutableElements(TypeElement element) {
List<ExecutableElement> 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<ExecutableElement> alreadyAdded, TypeElement element,
private void addEnclosedMethodsInHierarchy(List<ExecutableElement> alreadyAdded,
Collection<String> 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
);

View File

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

View File

@ -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;
}
}

View File

@ -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;
}
}

View File

@ -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 {
}

View File

@ -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 {
}

View File

@ -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 {
}

View File

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