Skip to content

Commit 7f65542

Browse files
filiphrfiliphr
committed
#220: Use PsiSubstitutor to properly resolve PsiMethod return types
1 parent f178b56 commit 7f65542

11 files changed

+169
-20
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,9 +112,13 @@ builderSupportPresent && isBuilderEnabled( getMappingMethod() )
112112
}
113113

114114
if ( builderSupportPresent ) {
115-
for ( PsiMethod method : psiClass.findMethodsByName( value, true ) ) {
115+
for ( Pair<PsiMethod, PsiSubstitutor> builderPair : psiClass.findMethodsAndTheirSubstitutorsByName(
116+
value,
117+
true
118+
) ) {
119+
PsiMethod method = builderPair.getFirst();
116120
if ( method.getParameterList().getParametersCount() == 1 &&
117-
mapstructUtil.isFluentSetter( method, typeToUse ) ) {
121+
mapstructUtil.isFluentSetter( method, typeToUse, builderPair.getSecond() ) ) {
118122
return method;
119123
}
120124
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.mapstruct.intellij.util;
77

88
import com.intellij.psi.PsiMethod;
9+
import com.intellij.psi.PsiSubstitutor;
910
import com.intellij.psi.PsiType;
1011
import org.jetbrains.annotations.NotNull;
1112

@@ -35,7 +36,7 @@ private FreeBuildersMapstructUtil() {
3536
}
3637

3738
@Override
38-
public boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType) {
39+
public boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType, @NotNull PsiSubstitutor substitutor) {
3940
// When using FreeBuilder one needs to use the JavaBean convention,
4041
// which means that all setters will start with set
4142
return false;

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
package org.mapstruct.intellij.util;
77

88
import com.intellij.psi.PsiMethod;
9+
import com.intellij.psi.PsiSubstitutor;
910
import com.intellij.psi.PsiType;
1011
import org.jetbrains.annotations.NotNull;
1112

@@ -25,7 +26,7 @@ private ImmutablesMapstructUtil() {
2526
}
2627

2728
@Override
28-
public boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType) {
29-
return super.isFluentSetter( method, psiType ) && !method.getName().equals( "from" );
29+
public boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType, @NotNull PsiSubstitutor substitutor) {
30+
return super.isFluentSetter( method, psiType, substitutor ) && !method.getName().equals( "from" );
3031
}
3132
}

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

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -225,14 +225,14 @@ public static boolean isPublicModifiable(@NotNull PsiField field) {
225225
!field.hasModifierProperty( PsiModifier.FINAL );
226226
}
227227

228-
public boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType) {
228+
public boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType, @NotNull PsiSubstitutor substitutor) {
229229
return !psiType.getCanonicalText().startsWith( "java.lang" ) &&
230230
method.getReturnType() != null &&
231231
!isAdderWithUpperCase4thCharacter( method ) &&
232-
isAssignableFromReturnTypeOrSuperTypes( psiType, method.getReturnType() );
232+
isAssignableFromReturnTypeOrSuperTypes( psiType, substitutor.substitute( method.getReturnType() ) );
233233
}
234234

235-
private static boolean isAssignableFromReturnTypeOrSuperTypes(PsiType psiType, PsiType returnType) {
235+
private static boolean isAssignableFromReturnTypeOrSuperTypes(PsiType psiType, @NotNull PsiType returnType) {
236236

237237
if ( isAssignableFrom( psiType, returnType ) ) {
238238
return true;
@@ -246,7 +246,7 @@ private static boolean isAssignableFromReturnTypeOrSuperTypes(PsiType psiType, P
246246
return false;
247247
}
248248

249-
private static boolean isAssignableFrom(PsiType psiType, @Nullable PsiType returnType) {
249+
private static boolean isAssignableFrom(PsiType psiType, @NotNull PsiType returnType) {
250250
return TypeConversionUtil.isAssignable(
251251
psiType,
252252
PsiUtil.resolveGenericsClassInType( psiType ).getSubstitutor().substitute( returnType )
@@ -298,11 +298,15 @@ public static boolean isPossibleBuilderCreationMethod(@NotNull PsiMethod method,
298298
* @return {@code true} if the {@code buildMethod} is a build method for {@code typeToBuild}, {@code false}
299299
* otherwise
300300
*/
301-
public static boolean isBuildMethod(@NotNull PsiMethod buildMethod, @NotNull PsiType typeToBuild) {
301+
public static boolean isBuildMethod(@NotNull PsiMethod buildMethod, @NotNull PsiSubstitutor buildMethodSubstitutor,
302+
@NotNull PsiType typeToBuild) {
302303
return buildMethod.getParameterList().isEmpty() &&
303304
isPublic( buildMethod ) &&
304305
buildMethod.getReturnType() != null &&
305-
TypeConversionUtil.isAssignable( typeToBuild, buildMethod.getReturnType() );
306+
TypeConversionUtil.isAssignable(
307+
typeToBuild,
308+
buildMethodSubstitutor.substitute( buildMethod.getReturnType() )
309+
);
306310
}
307311

308312
@NotNull

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,7 @@ private static Map<String, Pair<? extends PsiMember, PsiSubstitutor>> publicSett
281281
String propertyName = extractPublicSetterPropertyName(
282282
method,
283283
typeToUse,
284+
pair.getSecond(),
284285
mapstructUtil,
285286
builderSupportPresent
286287
);
@@ -317,6 +318,7 @@ private static PsiClassType getTypeByName(@NotNull String qName, @NotNull PsiMet
317318

318319
@Nullable
319320
private static String extractPublicSetterPropertyName(PsiMethod method, @NotNull PsiType typeToUse,
321+
PsiSubstitutor psiTypeSubstitutor,
320322
MapstructUtil mapstructUtil, boolean builderSupportPresent) {
321323
if (!MapstructUtil.isPublicNonStatic( method )) {
322324
// If the method is not public then there is no property
@@ -337,7 +339,7 @@ private static String extractPublicSetterPropertyName(PsiMethod method, @NotNull
337339
}
338340

339341
// This logic is aligned with the DefaultAccessorNamingStrategy
340-
if ( builderSupportPresent && mapstructUtil.isFluentSetter( method, typeToUse )) {
342+
if ( builderSupportPresent && mapstructUtil.isFluentSetter( method, typeToUse, psiTypeSubstitutor ) ) {
341343
if ( methodName.startsWith( "set" )
342344
&& methodName.length() > 3
343345
&& Character.isUpperCase( methodName.charAt( 3 ) ) ) {
@@ -401,8 +403,11 @@ private static boolean hasBuildMethod(@Nullable PsiType builderType, @NotNull Ps
401403
return false;
402404
}
403405

404-
for ( PsiMethod buildMethod : builderClass.getAllMethods() ) {
405-
if ( MapstructUtil.isBuildMethod( buildMethod, type ) ) {
406+
for ( Pair<PsiMethod, PsiSubstitutor> pair : builderClass.getAllMethodsAndTheirSubstitutors() ) {
407+
PsiMethod buildMethod = pair.getFirst();
408+
PsiSubstitutor buildMethodSubstitutor = pair.getSecond();
409+
410+
if ( MapstructUtil.isBuildMethod( buildMethod, buildMethodSubstitutor, type ) ) {
406411
return true;
407412
}
408413
}

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -988,6 +988,28 @@ public void testMapperWithGenericBuilder() {
988988
);
989989
}
990990

991+
public void testMapperWithSuperBuilder() {
992+
configureByTestName();
993+
994+
assertThat( myItems )
995+
.extracting( LookupElement::getLookupString )
996+
.containsExactlyInAnyOrder(
997+
"baseValue",
998+
"value"
999+
);
1000+
1001+
PsiElement reference = myFixture.getElementAtCaret();
1002+
1003+
assertThat( reference )
1004+
.isInstanceOfSatisfying( PsiMethod.class, method -> {
1005+
assertThat( method.getName() ).isEqualTo( "baseValue" );
1006+
assertThat( method.getPresentation() ).isNotNull();
1007+
assertThat( method.getPresentation().getPresentableText() ).isEqualTo( "baseValue(String)" );
1008+
assertThat( method.getParameterList().getParametersCount() ).isEqualTo( 1 );
1009+
assertThat( method.getReturnType() ).isNotNull();
1010+
} );
1011+
}
1012+
9911013
public void testMapperWithBuilderAndBeanMappingDisabledBuilder() {
9921014
configureByTestName();
9931015

‎src/test/java/org/mapstruct/intellij/inspection/UnmappedSuperBuilderTargetPropertiesInspectionTest.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,15 +45,20 @@ public void testUnmappedSuperBuilderTargetProperties() {
4545
"Add unmapped target property: 'testName'",
4646
"Ignore unmapped target property: 'moreTarget'",
4747
"Add unmapped target property: 'moreTarget'",
48+
"Ignore unmapped target property: 'baseValue'",
49+
"Add unmapped target property: 'baseValue'",
4850
"Ignore unmapped target property: 'moreTarget'",
4951
"Add unmapped target property: 'moreTarget'",
5052
"Ignore unmapped target property: 'testName'",
5153
"Add unmapped target property: 'testName'",
5254
"Ignore all unmapped target properties",
5355
"Ignore unmapped target property: 'testName'",
5456
"Add unmapped target property: 'testName'",
57+
"Ignore unmapped target property: 'baseValue'",
58+
"Add unmapped target property: 'baseValue'",
5559
"Ignore unmapped target property: 'moreTarget'",
56-
"Add unmapped target property: 'moreTarget'"
60+
"Add unmapped target property: 'moreTarget'",
61+
"Ignore all unmapped target properties"
5762
);
5863

5964
allQuickFixes.forEach( myFixture::launchAction );

‎testData/inspection/UnmappedSuperBuilderTargetProperties.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ interface NotMapStructMapper {
2222
interface SingleMappingsMapper {
2323

2424
@Mappings({
25-
@Mapping(target = "moreTarget", source = "moreSource")
25+
@Mapping(target = "moreTarget", source = "moreSource"),
26+
@Mapping(target = "baseValue", source = "name")
2627
})
2728
Target <warning descr="Unmapped target property: testName">map</warning>(Source source);
2829
}
@@ -31,13 +32,14 @@ interface SingleMappingsMapper {
3132
interface SingleMappingMapper {
3233

3334
@Mapping(target = "testName", source = "name")
35+
@Mapping(target = "baseValue", source = "name")
3436
Target <warning descr="Unmapped target property: moreTarget">map</warning>(Source source);
3537
}
3638

3739
@Mapper
3840
interface NoMappingMapper {
3941

40-
Target <warning descr="Unmapped target properties: moreTarget, testName">map</warning>(Source source);
42+
Target <warning descr="Unmapped target properties: baseValue, moreTarget, testName">map</warning>(Source source);
4143

4244
@InheritInverseConfiguration
4345
Source reverse(Target target);
@@ -48,18 +50,20 @@ interface AllMappingMapper {
4850

4951
@Mapping(target = "testName", source = "name")
5052
@Mapping(target = "moreTarget", source = "moreSource")
53+
@Mapping(target = "baseValue", source = "name")
5154
Target mapWithAllMapping(Source source);
5255
}
5356

5457
@Mapper
5558
interface UpdateMapper {
5659

5760
@Mapping(target = "moreTarget", source = "moreSource")
61+
@Mapping(target = "baseValue", source = "name")
5862
void <warning descr="Unmapped target property: testName">update</warning>(@MappingTarget Target target, Source source);
5963
}
6064

6165
@Mapper
6266
interface MultiSourceUpdateMapper {
6367

64-
void <warning descr="Unmapped target property: moreTarget">update</warning>(@MappingTarget Target moreTarget, Source source, String testName, @Context String matching);
68+
void <warning descr="Unmapped target properties: baseValue, moreTarget">update</warning>(@MappingTarget Target moreTarget, Source source, String testName, @Context String matching);
6569
}

‎testData/inspection/UnmappedSuperBuilderTargetPropertiesData.java‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,13 +46,38 @@ public void setOnlyInSource(String onlyInSource) {
4646
}
4747
}
4848

49-
public static class Target {
49+
public static abstract class Base {
50+
51+
private final String baseValue;
52+
53+
protected Base(BaseBuilder<?, ?> b) {
54+
this.baseValue = b.baseValue;
55+
}
56+
57+
public static abstract class BaseBuilder<C extends Base, B extends BaseBuilder<C, B>> {
58+
59+
private String baseValue;
60+
61+
public B baseValue(String baseValue) {
62+
this.baseValue = baseValue;
63+
return self();
64+
}
65+
66+
protected abstract B self();
67+
68+
public abstract C build();
69+
70+
}
71+
}
72+
73+
public static class Target extends Base {
5074

5175
private String testName;
5276
private String matching;
5377
private String moreTarget;
5478

5579
protected Target(TargetBuilder<?, ?> b) {
80+
super(b);
5681
this.testName = b.testName;
5782
this.matching = b.matching;
5883
this.moreTarget = b.moreTarget;
@@ -86,7 +111,7 @@ public void setMoreTarget(String moreTarget) {
86111
this.moreTarget = moreTarget;
87112
}
88113

89-
public static abstract class TargetBuilder<C extends Target, B extends TargetBuilder<C, B>> {
114+
public static abstract class TargetBuilder<C extends Target, B extends TargetBuilder<C, B>> extends BaseBuilder<C, B> {
90115
private String testName;
91116
private String matching;
92117
private String moreTarget;

‎testData/inspection/UnmappedSuperBuilderTargetProperties_after.java‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ interface SingleMappingsMapper {
2323

2424
@Mappings({
2525
@Mapping(target = "moreTarget", source = "moreSource"),
26+
@Mapping(target = "baseValue", source = "name"),
2627
@Mapping(target = "testName", ignore = true),
2728
@Mapping(target = "testName", source = "")
2829
})
@@ -35,6 +36,7 @@ interface SingleMappingMapper {
3536
@Mapping(target = "moreTarget", source = "")
3637
@Mapping(target = "moreTarget", ignore = true)
3738
@Mapping(target = "testName", source = "name")
39+
@Mapping(target = "baseValue", source = "name")
3840
Target map(Source source);
3941
}
4042

@@ -43,10 +45,13 @@ interface NoMappingMapper {
4345

4446
@Mapping(target = "testName", ignore = true)
4547
@Mapping(target = "moreTarget", ignore = true)
48+
@Mapping(target = "baseValue", ignore = true)
4649
@Mapping(target = "testName", source = "")
4750
@Mapping(target = "testName", ignore = true)
4851
@Mapping(target = "moreTarget", source = "")
4952
@Mapping(target = "moreTarget", ignore = true)
53+
@Mapping(target = "baseValue", source = "")
54+
@Mapping(target = "baseValue", ignore = true)
5055
Target map(Source source);
5156

5257
@InheritInverseConfiguration
@@ -58,6 +63,7 @@ interface AllMappingMapper {
5863

5964
@Mapping(target = "testName", source = "name")
6065
@Mapping(target = "moreTarget", source = "moreSource")
66+
@Mapping(target = "baseValue", source = "name")
6167
Target mapWithAllMapping(Source source);
6268
}
6369

@@ -67,13 +73,18 @@ interface UpdateMapper {
6773
@Mapping(target = "testName", source = "")
6874
@Mapping(target = "testName", ignore = true)
6975
@Mapping(target = "moreTarget", source = "moreSource")
76+
@Mapping(target = "baseValue", source = "name")
7077
void update(@MappingTarget Target target, Source source);
7178
}
7279

7380
@Mapper
7481
interface MultiSourceUpdateMapper {
7582

83+
@Mapping(target = "moreTarget", ignore = true)
84+
@Mapping(target = "baseValue", ignore = true)
7685
@Mapping(target = "moreTarget", source = "")
7786
@Mapping(target = "moreTarget", ignore = true)
87+
@Mapping(target = "baseValue", source = "")
88+
@Mapping(target = "baseValue", ignore = true)
7889
void update(@MappingTarget Target moreTarget, Source source, String testName, @Context String matching);
7990
}

0 commit comments

Comments
 (0)