Skip to content

Commit 4aab074

Browse files
filiphrfiliphr
committed
#217: Make sure that Collection / Map getter write accessors are properly resolved
1 parent db7e77c commit 4aab074

File tree

7 files changed

+189
-7
lines changed

7 files changed

+189
-7
lines changed

‎change-notes.html

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
<html lang="en">
2+
<h2>1.9.1</h2>
3+
<ul>
4+
<li>Fix collection / map getter write accessor not properly resolved</li>
5+
</ul>
26
<h2>1.9.0</h2>
37
<ul>
48
<li>

‎src/main/java/org/mapstruct/intellij/codeinsight/references/MapstructTargetReference.java

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import com.intellij.psi.PsiField;
1919
import com.intellij.psi.PsiMethod;
2020
import com.intellij.psi.PsiParameter;
21+
import com.intellij.psi.PsiParameterList;
2122
import com.intellij.psi.PsiRecordComponent;
2223
import com.intellij.psi.PsiReference;
2324
import com.intellij.psi.PsiSubstitutor;
@@ -98,11 +99,18 @@ builderSupportPresent && isBuilderEnabled( getMappingMethod() )
9899
}
99100
}
100101

101-
PsiMethod[] methods = psiClass.findMethodsByName( "set" + MapstructUtil.capitalize( value ), true );
102+
String capitalizedName = MapstructUtil.capitalize( value );
103+
PsiMethod[] methods = psiClass.findMethodsByName( "set" + capitalizedName, true );
102104
if ( methods.length != 0 && isPublicNonStatic( methods[0] ) ) {
103105
return methods[0];
104106
}
105107

108+
// If there is no such setter we need to check if there is a collection getter
109+
methods = psiClass.findMethodsByName( "get" + capitalizedName, true );
110+
if ( methods.length != 0 && isCollectionGetterWriteAccessor( methods[0] ) ) {
111+
return methods[0];
112+
}
113+
106114
if ( builderSupportPresent ) {
107115
for ( PsiMethod method : psiClass.findMethodsByName( value, true ) ) {
108116
if ( method.getParameterList().getParametersCount() == 1 &&
@@ -219,7 +227,7 @@ static PsiReference[] create(PsiElement psiElement) {
219227

220228
private static PsiType memberPsiType(PsiElement psiMember) {
221229
if ( psiMember instanceof PsiMethod psiMemberMethod ) {
222-
return firstParameterPsiType( psiMemberMethod );
230+
return resolveMethodType( psiMemberMethod );
223231
}
224232
else if ( psiMember instanceof PsiVariable psiMemberVariable ) {
225233
return psiMemberVariable.getType();
@@ -229,4 +237,23 @@ else if ( psiMember instanceof PsiVariable psiMemberVariable ) {
229237
}
230238

231239
}
240+
241+
private static boolean isCollectionGetterWriteAccessor(@NotNull PsiMethod method) {
242+
if ( !isPublicNonStatic( method ) ) {
243+
return false;
244+
}
245+
PsiParameterList parameterList = method.getParameterList();
246+
if ( parameterList.getParametersCount() > 0 ) {
247+
return false;
248+
}
249+
return TargetUtils.isMethodReturnTypeAssignableToCollectionOrMap( method );
250+
}
251+
252+
private static PsiType resolveMethodType(PsiMethod psiMethod) {
253+
PsiParameter[] psiParameters = psiMethod.getParameterList().getParameters();
254+
if ( psiParameters.length == 0 ) {
255+
return psiMethod.getReturnType();
256+
}
257+
return psiParameters[0].getType();
258+
}
232259
}

‎src/main/java/org/mapstruct/intellij/util/TargetUtils.java

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import com.intellij.psi.PsiAnnotation;
2626
import com.intellij.psi.PsiAnnotationMemberValue;
2727
import com.intellij.psi.PsiClass;
28+
import com.intellij.psi.PsiClassType;
2829
import com.intellij.psi.PsiElement;
2930
import com.intellij.psi.PsiJavaCodeReferenceElement;
3031
import com.intellij.psi.PsiMember;
@@ -295,6 +296,25 @@ private static Map<String, Pair<? extends PsiMember, PsiSubstitutor>> publicSett
295296
return publicSetters;
296297
}
297298

299+
public static boolean isMethodReturnTypeAssignableToCollectionOrMap(@NotNull PsiMethod method) {
300+
PsiType returnType = method.getReturnType();
301+
if ( returnType == null ) {
302+
return false;
303+
}
304+
if ( getTypeByName( "java.util.Collection", method ).isAssignableFrom( returnType ) ) {
305+
return true;
306+
}
307+
return getTypeByName( "java.util.Map", method ).isAssignableFrom( returnType );
308+
}
309+
310+
private static PsiClassType getTypeByName(@NotNull String qName, @NotNull PsiMethod method) {
311+
return PsiType.getTypeByName(
312+
qName,
313+
method.getProject(),
314+
method.getResolveScope()
315+
);
316+
}
317+
298318
@Nullable
299319
private static String extractPublicSetterPropertyName(PsiMethod method, @NotNull PsiType typeToUse,
300320
MapstructUtil mapstructUtil, boolean builderSupportPresent) {
@@ -304,10 +324,10 @@ private static String extractPublicSetterPropertyName(PsiMethod method, @NotNull
304324
}
305325
String methodName = method.getName();
306326
int parametersCount = method.getParameterList().getParametersCount();
307-
PsiType returnType = method.getReturnType();
308-
if (parametersCount == 0 && methodName.startsWith( "get" ) && returnType != null &&
309-
returnType.isConvertibleFrom( PsiType.getTypeByName( "java.util.Collection",
310-
method.getProject(), method.getResolveScope() ) )) {
327+
328+
if ( parametersCount == 0 && methodName.startsWith( "get" ) &&
329+
isMethodReturnTypeAssignableToCollectionOrMap( method ) ) {
330+
311331
// If the methode returns a collection
312332
return Introspector.decapitalize( methodName.substring( 3 ) );
313333
}

‎src/test/java/org/mapstruct/intellij/MapstructCompletionTestCase.java

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -700,6 +700,66 @@ public void testTargetPropertyReferencesTargetParameter() {
700700
} );
701701
}
702702

703+
public void testTargetWithCollectionGetterMapper() {
704+
configureByTestName();
705+
assertThat( myItems )
706+
.extracting( LookupElement::getLookupString )
707+
.containsExactlyInAnyOrder(
708+
"myStringList",
709+
"myStringSet"
710+
);
711+
712+
assertThat( myItems )
713+
.extracting( LookupElementPresentation::renderElement )
714+
.usingRecursiveFieldByFieldElementComparator()
715+
.containsExactlyInAnyOrder(
716+
createVariable( "myStringList", "List<String>" ),
717+
createVariable( "myStringSet", "Set<String>" )
718+
);
719+
720+
PsiElement reference = myFixture.getElementAtCaret();
721+
assertThat( reference )
722+
.isInstanceOfSatisfying(
723+
PsiMethod.class, method -> {
724+
assertThat( method.getName() ).isEqualTo( "getMyStringList" );
725+
assertThat( method.getPresentation() ).isNotNull();
726+
assertThat( method.getPresentation().getPresentableText() ).isEqualTo( "getMyStringList()" );
727+
assertThat( method.getParameterList().getParametersCount() ).isEqualTo( 0 );
728+
assertThat( method.getReturnType() ).isNotNull();
729+
assertThat( method.getReturnType().getPresentableText() ).isEqualTo( "List<String>" );
730+
}
731+
);
732+
}
733+
734+
public void testTargetWithMapGetterMapper() {
735+
configureByTestName();
736+
assertThat( myItems )
737+
.extracting( LookupElement::getLookupString )
738+
.containsExactlyInAnyOrder(
739+
"myMap"
740+
);
741+
742+
assertThat( myItems )
743+
.extracting( LookupElementPresentation::renderElement )
744+
.usingRecursiveFieldByFieldElementComparator()
745+
.containsExactlyInAnyOrder(
746+
createVariable( "myMap", "Map<String, String>" )
747+
);
748+
749+
PsiElement reference = myFixture.getElementAtCaret();
750+
assertThat( reference )
751+
.isInstanceOfSatisfying(
752+
PsiMethod.class, method -> {
753+
assertThat( method.getName() ).isEqualTo( "getMyMap" );
754+
assertThat( method.getPresentation() ).isNotNull();
755+
assertThat( method.getPresentation().getPresentableText() ).isEqualTo( "getMyMap()" );
756+
assertThat( method.getParameterList().getParametersCount() ).isEqualTo( 0 );
757+
assertThat( method.getReturnType() ).isNotNull();
758+
assertThat( method.getReturnType().getPresentableText() ).isEqualTo( "Map<String, String>" );
759+
}
760+
);
761+
}
762+
703763
public void testCarMapperReferenceBooleanSourceCar() {
704764
myFixture.configureByFile( "CarMapperReferenceBooleanSourceCar.java" );
705765
PsiElement reference = myFixture.getElementAtCaret();

‎testData/inspection/UnmappedCollectionGetterPropertiesData.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ public List<String> getListTarget() {
4040
return listTarget;
4141
}
4242

43-
public Set<Strinf> getSetTarget() {
43+
public Set<String> getSetTarget() {
4444
return setTarget;
4545
}
4646

@@ -51,6 +51,10 @@ public Map<String, String> getMapTarget() {
5151
public String getStringTarget() {
5252
return stringTarget;
5353
}
54+
55+
public Object getObjectTarget() {
56+
return null;
57+
}
5458
}
5559

5660
}

‎testData/mapping/TargetWithCollectionGetterMapper.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at https://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.ap.test.complex;
7+
8+
import java.util.List;
9+
import java.util.Set;
10+
import org.mapstruct.Mapper;
11+
import org.mapstruct.Mapping;
12+
13+
@Mapper
14+
public interface TestMapper {
15+
16+
@Mapping(target = "<caret>myStringList")
17+
Target carToCarDto(Object value);
18+
19+
public class Target {
20+
public List<String> getMyStringList() {
21+
return null;
22+
}
23+
24+
public Set<String> getMyStringSet() {
25+
return null;
26+
}
27+
28+
public String getName() {
29+
return null;
30+
}
31+
32+
public Object getValue() {
33+
return null;
34+
}
35+
}
36+
}

‎testData/mapping/TargetWithMapGetterMapper.java

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at https://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.ap.test.complex;
7+
8+
import java.util.Map;
9+
import org.mapstruct.Mapper;
10+
import org.mapstruct.Mapping;
11+
12+
@Mapper
13+
public interface TestMapper {
14+
15+
@Mapping(target = "<caret>myMap")
16+
Target carToCarDto(Object value);
17+
18+
public class Target {
19+
public Map<String, String> getMyMap() {
20+
return null;
21+
}
22+
23+
public String getName() {
24+
return null;
25+
}
26+
27+
public Object getValue() {
28+
return null;
29+
}
30+
}
31+
}

0 commit comments

Comments
 (0)