Skip to content

Commit 34db62f

Browse files
filiphrfiliphr
committed
Fix code completions for builders
* Fix code completion for generic builder * Fix code completion for builder with constructor parameters Fixes #78
1 parent 496370a commit 34db62f

11 files changed

Lines changed: 397 additions & 13 deletions

‎change-notes.html‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ <h2>1.3.0</h2>
33
<ul>
44
<li>Support code completion in <code>BeanMapping#ignoreUnmappedSourceProperties</code></li>
55
<li>Quick Fix: support for configuring the order of source and target in <code>@Mapping</code> for "Add unmapped property" fix</li>
6+
<li>Bug fix: Code completion for generic builder</li>
7+
<li>Bug fix: Code completion uses build constructor parameters</li>
68
</ul>
79
<h2>1.2.4</h2>
810
<ul>

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import org.jetbrains.annotations.Nullable;
2525
import org.mapstruct.intellij.util.MapStructVersion;
2626
import org.mapstruct.intellij.util.MapstructUtil;
27+
import org.mapstruct.intellij.util.TargetType;
2728
import org.mapstruct.intellij.util.TargetUtils;
2829

2930
import static org.mapstruct.intellij.util.MapstructUtil.asLookup;
@@ -59,15 +60,16 @@ private MapstructTargetReference(PsiElement element, MapstructTargetReference pr
5960
@Override
6061
PsiElement resolveInternal(@NotNull String value, @NotNull PsiType psiType) {
6162
boolean builderSupportPresent = mapStructVersion.isBuilderSupported();
62-
Pair<PsiClass, PsiType> pair = resolveBuilderOrSelfClass( psiType, builderSupportPresent );
63+
Pair<PsiClass, TargetType> pair = resolveBuilderOrSelfClass( psiType, builderSupportPresent );
6364
if ( pair == null ) {
6465
return null;
6566
}
6667

6768
PsiClass psiClass = pair.getFirst();
68-
PsiType typeToUse = pair.getSecond();
69+
TargetType targetType = pair.getSecond();
70+
PsiType typeToUse = targetType.type();
6971

70-
if ( mapStructVersion.isConstructorSupported() ) {
72+
if ( mapStructVersion.isConstructorSupported() && !targetType.builder() ) {
7173
PsiMethod constructor = TargetUtils.resolveMappingConstructor( psiClass );
7274
if ( constructor != null && constructor.hasParameters() ) {
7375
for ( PsiParameter parameter : constructor.getParameterList().getParameters() ) {

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import com.intellij.psi.util.CachedValuesManager;
4545
import com.intellij.psi.util.PsiFormatUtil;
4646
import com.intellij.psi.util.PsiFormatUtilBase;
47+
import com.intellij.psi.util.PsiUtil;
4748
import com.intellij.psi.util.TypeConversionUtil;
4849
import com.intellij.util.PlatformIcons;
4950
import org.jetbrains.annotations.NonNls;
@@ -206,7 +207,10 @@ public static boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType)
206207
return !psiType.getCanonicalText().startsWith( "java.lang" ) &&
207208
method.getReturnType() != null &&
208209
!isAdderWithUpperCase4thCharacter( method ) &&
209-
TypeConversionUtil.isAssignable( method.getReturnType(), psiType );
210+
TypeConversionUtil.isAssignable(
211+
PsiUtil.resolveGenericsClassInType( psiType ).getSubstitutor().substitute( method.getReturnType() ),
212+
psiType
213+
);
210214
}
211215

212216
private static boolean isAdderWithUpperCase4thCharacter(@NotNull PsiMethod method) {

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.intellij.util;
7+
8+
import com.intellij.psi.PsiType;
9+
10+
/**
11+
* @author Filip Hrisafov
12+
*/
13+
public class TargetType {
14+
15+
private final PsiType type;
16+
private final boolean builder;
17+
18+
private TargetType(PsiType type, boolean builder) {
19+
this.type = type;
20+
this.builder = builder;
21+
}
22+
23+
public PsiType type() {
24+
return type;
25+
}
26+
27+
public boolean builder() {
28+
return builder;
29+
}
30+
31+
public static TargetType builder(PsiType type) {
32+
return new TargetType( type, true );
33+
}
34+
35+
public static TargetType defaultType(PsiType type) {
36+
return new TargetType( type, false );
37+
}
38+
}

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

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -90,20 +90,21 @@ public static PsiType getRelevantType(@NotNull PsiMethod mappingMethod) {
9090
public static Map<String, Pair<? extends PsiElement, PsiSubstitutor>> publicWriteAccessors(@NotNull PsiType psiType,
9191
MapStructVersion mapStructVersion) {
9292
boolean builderSupportPresent = mapStructVersion.isBuilderSupported();
93-
Pair<PsiClass, PsiType> classAndType = resolveBuilderOrSelfClass( psiType, builderSupportPresent );
93+
Pair<PsiClass, TargetType> classAndType = resolveBuilderOrSelfClass( psiType, builderSupportPresent );
9494
if ( classAndType == null ) {
9595
return Collections.emptyMap();
9696
}
9797

9898
Map<String, Pair<? extends PsiElement, PsiSubstitutor>> publicWriteAccessors = new LinkedHashMap<>();
9999

100100
PsiClass psiClass = classAndType.getFirst();
101-
PsiType typeToUse = classAndType.getSecond();
101+
TargetType targetType = classAndType.getSecond();
102+
PsiType typeToUse = targetType.type();
102103

103104
publicWriteAccessors.putAll( publicSetters( psiClass, typeToUse, builderSupportPresent ) );
104105
publicWriteAccessors.putAll( publicFields( psiClass ) );
105106

106-
if ( mapStructVersion.isConstructorSupported() ) {
107+
if ( mapStructVersion.isConstructorSupported() && !targetType.builder() ) {
107108
publicWriteAccessors.putAll( constructorParameters( psiClass ) );
108109
}
109110

@@ -195,6 +196,9 @@ private static Map<String, Pair<? extends PsiMember, PsiSubstitutor>> publicSett
195196
Map<String, Pair<? extends PsiMember, PsiSubstitutor>> publicSetters = new LinkedHashMap<>();
196197
for ( Pair<PsiMethod, PsiSubstitutor> pair : psiClass.getAllMethodsAndTheirSubstitutors() ) {
197198
PsiMethod method = pair.getFirst();
199+
if ( method.isConstructor() ) {
200+
continue;
201+
}
198202
String propertyName = extractPublicSetterPropertyName( method, typeToUse, builderSupportPresent );
199203

200204
if ( propertyName != null &&
@@ -253,26 +257,26 @@ else if ( methodName.startsWith( "set" ) ) {
253257
*
254258
* @return the pair containing the {@link PsiClass} and the corresponding {@link PsiType}
255259
*/
256-
public static Pair<PsiClass, PsiType> resolveBuilderOrSelfClass(@NotNull PsiType psiType,
260+
public static Pair<PsiClass, TargetType> resolveBuilderOrSelfClass(@NotNull PsiType psiType,
257261
boolean builderSupportPresent) {
258262
PsiClass psiClass = PsiUtil.resolveClassInType( psiType );
259263
if ( psiClass == null ) {
260264
return null;
261265
}
262-
PsiType typeToUse = psiType;
266+
TargetType targetType = TargetType.defaultType( psiType );
263267

264268
if ( builderSupportPresent ) {
265269
for ( PsiMethod classMethod : psiClass.getMethods() ) {
266-
if ( MapstructUtil.isPossibleBuilderCreationMethod( classMethod, typeToUse ) &&
270+
if ( MapstructUtil.isPossibleBuilderCreationMethod( classMethod, targetType.type() ) &&
267271
hasBuildMethod( classMethod.getReturnType(), psiType ) ) {
268-
typeToUse = classMethod.getReturnType();
272+
targetType = TargetType.builder( classMethod.getReturnType() );
269273
break;
270274
}
271275
}
272276
}
273277

274-
psiClass = PsiUtil.resolveClassInType( typeToUse );
275-
return psiClass == null ? null : Pair.createNonNull( psiClass, typeToUse );
278+
psiClass = PsiUtil.resolveClassInType( targetType.type() );
279+
return psiClass == null ? null : Pair.createNonNull( psiClass, targetType );
276280
}
277281

278282
/**

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -919,4 +919,37 @@ public void testOverriddenTarget() {
919919
);
920920
}
921921

922+
public void testMapperWithBuilderWithSingleConstructor() {
923+
configureByTestName();
924+
925+
assertThat( myItems )
926+
.extracting( LookupElement::getLookupString )
927+
.containsExactlyInAnyOrder(
928+
"address",
929+
"city"
930+
);
931+
}
932+
933+
public void testMapperWithBuilderWithMultipleConstructors() {
934+
configureByTestName();
935+
936+
assertThat( myItems )
937+
.extracting( LookupElement::getLookupString )
938+
.containsExactlyInAnyOrder(
939+
"address",
940+
"city"
941+
);
942+
}
943+
944+
public void testMapperWithGenericBuilder() {
945+
configureByTestName();
946+
947+
assertThat( myItems )
948+
.extracting( LookupElement::getLookupString )
949+
.containsExactlyInAnyOrder(
950+
"address",
951+
"city"
952+
);
953+
}
954+
922955
}

‎src/test/java/org/mapstruct/intellij/bugs/_78/CodeCompletionForGenericBuilderTest.java‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
package org.mapstruct.intellij.bugs._78;
7+
8+
import com.intellij.codeInsight.lookup.LookupElement;
9+
import org.mapstruct.intellij.MapstructBaseCompletionTestCase;
10+
11+
import static org.assertj.core.api.Assertions.assertThat;
12+
13+
/**
14+
* @author Filip Hrisafov
15+
*/
16+
public class CodeCompletionForGenericBuilderTest extends MapstructBaseCompletionTestCase {
17+
18+
@Override
19+
protected String getTestDataPath() {
20+
return "testData/bugs/_78";
21+
}
22+
23+
public void testCodeCompletionForGenericBuilder() {
24+
configureByTestName();
25+
assertThat( myItems )
26+
.extracting( LookupElement::getLookupString )
27+
.containsExactlyInAnyOrder(
28+
"withStatus",
29+
"withData"
30+
);
31+
}
32+
}

‎testData/bugs/_78/CodeCompletionForGenericBuilder.java‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* Copyright MapStruct Authors.
3+
*
4+
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
5+
*/
6+
import org.mapstruct.Mapper;
7+
import org.mapstruct.Mapping;
8+
9+
@Mapper
10+
public abstract class DemoMapper {
11+
12+
// code completion for target broken - see comments down below to see the culprit
13+
@Mapping(target = "<caret>", source = "input")
14+
public abstract DemoTypeWithBuilder map(String input);
15+
16+
public static class DemoTypeWithBuilder {
17+
18+
protected String status;
19+
protected int data;
20+
21+
public String getStatus() {
22+
return status;
23+
}
24+
25+
public void setStatus(String value) {
26+
this.status = value;
27+
}
28+
29+
public int getData() {
30+
return data;
31+
}
32+
33+
public void setData(int value) {
34+
this.data = value;
35+
}
36+
37+
/**
38+
* when commenting out this static builder method, then auto complete lists values as expected:
39+
* "status", "data"
40+
*/
41+
public static DemoTypeWithBuilder.Builder<Void> builder() {
42+
return new DemoTypeWithBuilder.Builder<Void>( null, null, false );
43+
}
44+
45+
public static class Builder<_B> {
46+
47+
protected final _B _parentBuilder;
48+
protected final DemoTypeWithBuilder _storedValue;
49+
private String status;
50+
private int data;
51+
52+
public Builder(final _B _parentBuilder, final DemoTypeWithBuilder _other, final boolean _copy) {
53+
this._parentBuilder = _parentBuilder;
54+
if ( _other != null ) {
55+
if ( _copy ) {
56+
_storedValue = null;
57+
this.status = _other.status;
58+
this.data = _other.data;
59+
}
60+
else {
61+
_storedValue = _other;
62+
}
63+
}
64+
else {
65+
_storedValue = null;
66+
}
67+
}
68+
69+
/**
70+
* when commenting out this constructor, then auto-complete lists:
71+
* "_copy", "_other", "_parentBuilder"
72+
*/
73+
public Builder(final _B _parentBuilder, final DemoTypeWithBuilder _other, final boolean _copy,
74+
final PropertyTree _propertyTree, final
75+
PropertyTreeUse _propertyTreeUse) {
76+
this._parentBuilder = _parentBuilder;
77+
if ( _other != null ) {
78+
if ( _copy ) {
79+
_storedValue = null;
80+
}
81+
else {
82+
_storedValue = _other;
83+
}
84+
}
85+
else {
86+
_storedValue = null;
87+
}
88+
}
89+
90+
protected <_P extends DemoTypeWithBuilder> _P init(final _P _product) {
91+
_product.status = this.status;
92+
_product.data = this.data;
93+
return _product;
94+
}
95+
96+
public DemoTypeWithBuilder.Builder<_B> withStatus(final String status) {
97+
this.status = status;
98+
return this;
99+
}
100+
101+
public DemoTypeWithBuilder.Builder<_B> withData(final int data) {
102+
this.data = data;
103+
return this;
104+
}
105+
106+
public DemoTypeWithBuilder build() {
107+
if ( _storedValue == null ) {
108+
return this.init( new DemoTypeWithBuilder() );
109+
}
110+
else {
111+
return _storedValue;
112+
}
113+
}
114+
}
115+
}
116+
117+
// mock for com.kscs.util.jaxb.PropertyTreeUse
118+
public static class PropertyTreeUse {
119+
120+
}
121+
122+
// mock for com.kscs.util.jaxb.PropertyTree
123+
public static class PropertyTree {
124+
125+
}
126+
}

0 commit comments

Comments
 (0)