Skip to content

Commit

Permalink
Fix code completions for builders
Browse files Browse the repository at this point in the history
* Fix code completion for generic builder
* Fix code completion for builder with constructor parameters

Fixes #78
  • Loading branch information
filiphr committed Aug 22, 2021
1 parent 100a425 commit 29d1848
Show file tree
Hide file tree
Showing 11 changed files with 397 additions and 13 deletions.
2 changes: 2 additions & 0 deletions change-notes.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
<h2>1.3.0</h2>
<ul>
<li>Quick Fix: support for configuring the order of source and target in <code>@Mapping</code> for "Add unmapped property" fix</li>
<li>Bug fix: Code completion for generic builder</li>
<li>Bug fix: Code completion uses build constructor parameters</li>
</ul>
<h2>1.2.4</h2>
<ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import org.jetbrains.annotations.Nullable;
import org.mapstruct.intellij.util.MapStructVersion;
import org.mapstruct.intellij.util.MapstructUtil;
import org.mapstruct.intellij.util.TargetType;
import org.mapstruct.intellij.util.TargetUtils;

import static org.mapstruct.intellij.util.MapstructUtil.asLookup;
Expand Down Expand Up @@ -59,15 +60,16 @@ private MapstructTargetReference(PsiElement element, MapstructTargetReference pr
@Override
PsiElement resolveInternal(@NotNull String value, @NotNull PsiType psiType) {
boolean builderSupportPresent = mapStructVersion.isBuilderSupported();
Pair<PsiClass, PsiType> pair = resolveBuilderOrSelfClass( psiType, builderSupportPresent );
Pair<PsiClass, TargetType> pair = resolveBuilderOrSelfClass( psiType, builderSupportPresent );
if ( pair == null ) {
return null;
}

PsiClass psiClass = pair.getFirst();
PsiType typeToUse = pair.getSecond();
TargetType targetType = pair.getSecond();
PsiType typeToUse = targetType.type();

if ( mapStructVersion.isConstructorSupported() ) {
if ( mapStructVersion.isConstructorSupported() && !targetType.builder() ) {
PsiMethod constructor = TargetUtils.resolveMappingConstructor( psiClass );
if ( constructor != null && constructor.hasParameters() ) {
for ( PsiParameter parameter : constructor.getParameterList().getParameters() ) {
Expand Down
6 changes: 5 additions & 1 deletion src/main/java/org/mapstruct/intellij/util/MapstructUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.intellij.psi.util.CachedValuesManager;
import com.intellij.psi.util.PsiFormatUtil;
import com.intellij.psi.util.PsiFormatUtilBase;
import com.intellij.psi.util.PsiUtil;
import com.intellij.psi.util.TypeConversionUtil;
import com.intellij.util.PlatformIcons;
import org.jetbrains.annotations.NonNls;
Expand Down Expand Up @@ -206,7 +207,10 @@ public static boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType)
return !psiType.getCanonicalText().startsWith( "java.lang" ) &&
method.getReturnType() != null &&
!isAdderWithUpperCase4thCharacter( method ) &&
TypeConversionUtil.isAssignable( method.getReturnType(), psiType );
TypeConversionUtil.isAssignable(
PsiUtil.resolveGenericsClassInType( psiType ).getSubstitutor().substitute( method.getReturnType() ),
psiType
);
}

private static boolean isAdderWithUpperCase4thCharacter(@NotNull PsiMethod method) {
Expand Down
38 changes: 38 additions & 0 deletions src/main/java/org/mapstruct/intellij/util/TargetType.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
/*
* Copyright MapStruct Authors.
*
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
*/
package org.mapstruct.intellij.util;

import com.intellij.psi.PsiType;

/**
* @author Filip Hrisafov
*/
public class TargetType {

private final PsiType type;
private final boolean builder;

private TargetType(PsiType type, boolean builder) {
this.type = type;
this.builder = builder;
}

public PsiType type() {
return type;
}

public boolean builder() {
return builder;
}

public static TargetType builder(PsiType type) {
return new TargetType( type, true );
}

public static TargetType defaultType(PsiType type) {
return new TargetType( type, false );
}
}
22 changes: 13 additions & 9 deletions src/main/java/org/mapstruct/intellij/util/TargetUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,21 @@ public static PsiType getRelevantType(@NotNull PsiMethod mappingMethod) {
public static Map<String, Pair<? extends PsiElement, PsiSubstitutor>> publicWriteAccessors(@NotNull PsiType psiType,
MapStructVersion mapStructVersion) {
boolean builderSupportPresent = mapStructVersion.isBuilderSupported();
Pair<PsiClass, PsiType> classAndType = resolveBuilderOrSelfClass( psiType, builderSupportPresent );
Pair<PsiClass, TargetType> classAndType = resolveBuilderOrSelfClass( psiType, builderSupportPresent );
if ( classAndType == null ) {
return Collections.emptyMap();
}

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

PsiClass psiClass = classAndType.getFirst();
PsiType typeToUse = classAndType.getSecond();
TargetType targetType = classAndType.getSecond();
PsiType typeToUse = targetType.type();

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

if ( mapStructVersion.isConstructorSupported() ) {
if ( mapStructVersion.isConstructorSupported() && !targetType.builder() ) {
publicWriteAccessors.putAll( constructorParameters( psiClass ) );
}

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

if ( propertyName != null &&
Expand Down Expand Up @@ -253,26 +257,26 @@ else if ( methodName.startsWith( "set" ) ) {
*
* @return the pair containing the {@link PsiClass} and the corresponding {@link PsiType}
*/
public static Pair<PsiClass, PsiType> resolveBuilderOrSelfClass(@NotNull PsiType psiType,
public static Pair<PsiClass, TargetType> resolveBuilderOrSelfClass(@NotNull PsiType psiType,
boolean builderSupportPresent) {
PsiClass psiClass = PsiUtil.resolveClassInType( psiType );
if ( psiClass == null ) {
return null;
}
PsiType typeToUse = psiType;
TargetType targetType = TargetType.defaultType( psiType );

if ( builderSupportPresent ) {
for ( PsiMethod classMethod : psiClass.getMethods() ) {
if ( MapstructUtil.isPossibleBuilderCreationMethod( classMethod, typeToUse ) &&
if ( MapstructUtil.isPossibleBuilderCreationMethod( classMethod, targetType.type() ) &&
hasBuildMethod( classMethod.getReturnType(), psiType ) ) {
typeToUse = classMethod.getReturnType();
targetType = TargetType.builder( classMethod.getReturnType() );
break;
}
}
}

psiClass = PsiUtil.resolveClassInType( typeToUse );
return psiClass == null ? null : Pair.createNonNull( psiClass, typeToUse );
psiClass = PsiUtil.resolveClassInType( targetType.type() );
return psiClass == null ? null : Pair.createNonNull( psiClass, targetType );
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -919,4 +919,37 @@ public void testOverriddenTarget() {
);
}

public void testMapperWithBuilderWithSingleConstructor() {
configureByTestName();

assertThat( myItems )
.extracting( LookupElement::getLookupString )
.containsExactlyInAnyOrder(
"address",
"city"
);
}

public void testMapperWithBuilderWithMultipleConstructors() {
configureByTestName();

assertThat( myItems )
.extracting( LookupElement::getLookupString )
.containsExactlyInAnyOrder(
"address",
"city"
);
}

public void testMapperWithGenericBuilder() {
configureByTestName();

assertThat( myItems )
.extracting( LookupElement::getLookupString )
.containsExactlyInAnyOrder(
"address",
"city"
);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright MapStruct Authors.
*
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
*/
package org.mapstruct.intellij.bugs._78;

import com.intellij.codeInsight.lookup.LookupElement;
import org.mapstruct.intellij.MapstructBaseCompletionTestCase;

import static org.assertj.core.api.Assertions.assertThat;

/**
* @author Filip Hrisafov
*/
public class CodeCompletionForGenericBuilderTest extends MapstructBaseCompletionTestCase {

@Override
protected String getTestDataPath() {
return "testData/bugs/_78";
}

public void testCodeCompletionForGenericBuilder() {
configureByTestName();
assertThat( myItems )
.extracting( LookupElement::getLookupString )
.containsExactlyInAnyOrder(
"withStatus",
"withData"
);
}
}
126 changes: 126 additions & 0 deletions testData/bugs/_78/CodeCompletionForGenericBuilder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
/*
* Copyright MapStruct Authors.
*
* Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
*/
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;

@Mapper
public abstract class DemoMapper {

// code completion for target broken - see comments down below to see the culprit
@Mapping(target = "<caret>", source = "input")
public abstract DemoTypeWithBuilder map(String input);

public static class DemoTypeWithBuilder {

protected String status;
protected int data;

public String getStatus() {
return status;
}

public void setStatus(String value) {
this.status = value;
}

public int getData() {
return data;
}

public void setData(int value) {
this.data = value;
}

/**
* when commenting out this static builder method, then auto complete lists values as expected:
* "status", "data"
*/
public static DemoTypeWithBuilder.Builder<Void> builder() {
return new DemoTypeWithBuilder.Builder<Void>( null, null, false );
}

public static class Builder<_B> {

protected final _B _parentBuilder;
protected final DemoTypeWithBuilder _storedValue;
private String status;
private int data;

public Builder(final _B _parentBuilder, final DemoTypeWithBuilder _other, final boolean _copy) {
this._parentBuilder = _parentBuilder;
if ( _other != null ) {
if ( _copy ) {
_storedValue = null;
this.status = _other.status;
this.data = _other.data;
}
else {
_storedValue = _other;
}
}
else {
_storedValue = null;
}
}

/**
* when commenting out this constructor, then auto-complete lists:
* "_copy", "_other", "_parentBuilder"
*/
public Builder(final _B _parentBuilder, final DemoTypeWithBuilder _other, final boolean _copy,
final PropertyTree _propertyTree, final
PropertyTreeUse _propertyTreeUse) {
this._parentBuilder = _parentBuilder;
if ( _other != null ) {
if ( _copy ) {
_storedValue = null;
}
else {
_storedValue = _other;
}
}
else {
_storedValue = null;
}
}

protected <_P extends DemoTypeWithBuilder> _P init(final _P _product) {
_product.status = this.status;
_product.data = this.data;
return _product;
}

public DemoTypeWithBuilder.Builder<_B> withStatus(final String status) {
this.status = status;
return this;
}

public DemoTypeWithBuilder.Builder<_B> withData(final int data) {
this.data = data;
return this;
}

public DemoTypeWithBuilder build() {
if ( _storedValue == null ) {
return this.init( new DemoTypeWithBuilder() );
}
else {
return _storedValue;
}
}
}
}

// mock for com.kscs.util.jaxb.PropertyTreeUse
public static class PropertyTreeUse {

}

// mock for com.kscs.util.jaxb.PropertyTree
public static class PropertyTree {

}
}
Loading

0 comments on commit 29d1848

Please sign in to comment.