From 2a80757c06ed6e7725dd140f6abe4e6eb058b414 Mon Sep 17 00:00:00 2001 From: hdulme Date: Thu, 23 Jun 2022 00:59:57 +0200 Subject: [PATCH] Suggest only missing constants for ValueMapping source closes #5 closes #103 --- change-notes.html | 1 + .../ValueMappingSourceReference.java | 14 +- .../util/MapstructAnnotationUtils.java | 48 +++++++ .../intellij/util/ValueMappingUtils.java | 38 ++++++ .../ValueMappingCompletionTestCase.java | 121 ++++++++++++++++++ 5 files changed, 219 insertions(+), 3 deletions(-) create mode 100644 src/main/java/org/mapstruct/intellij/util/ValueMappingUtils.java diff --git a/change-notes.html b/change-notes.html index 6361ea3e..4371a25c 100644 --- a/change-notes.html +++ b/change-notes.html @@ -4,6 +4,7 @@

1.4.0

  • Suppress redundant default parameter value assignment warning for Mapping#constant and Mapping#defaultValue
  • Support for Java records
  • Support MapStruct explicit Builder#disableBuilder through @MapperConfig
    • +
  • @ValueMapping source code completion should only suggest unmapped source constants
  • Bug fix: language injections inside expressions when target is field

    • 1.3.1

    diff --git a/src/main/java/org/mapstruct/intellij/codeinsight/references/ValueMappingSourceReference.java b/src/main/java/org/mapstruct/intellij/codeinsight/references/ValueMappingSourceReference.java index 773815bf..975dacc5 100644 --- a/src/main/java/org/mapstruct/intellij/codeinsight/references/ValueMappingSourceReference.java +++ b/src/main/java/org/mapstruct/intellij/codeinsight/references/ValueMappingSourceReference.java @@ -5,6 +5,8 @@ */ package org.mapstruct.intellij.codeinsight.references; +import java.util.Set; +import java.util.stream.Collectors; import java.util.stream.Stream; import com.intellij.codeInsight.lookup.LookupElement; @@ -16,8 +18,9 @@ import com.intellij.psi.PsiReference; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.mapstruct.intellij.util.MapstructUtil; +import org.mapstruct.intellij.util.ValueMappingUtils; -import static org.mapstruct.intellij.util.MapstructUtil.asLookup; import static org.mapstruct.intellij.util.SourceUtils.getParameterClass; /** @@ -59,9 +62,14 @@ Object[] getVariantsInternal(@NotNull PsiMethod mappingMethod) { return LookupElement.EMPTY_ARRAY; } + Set alreadyDefinedValues = ValueMappingUtils.findAllDefinedValueMappingSources( mappingMethod ) + .collect( Collectors.toSet() ); + return Stream.of( sourceClass.getFields() ) - .filter( psiField -> psiField instanceof PsiEnumConstant ) - .map( psiEnumConstant -> asLookup( (PsiEnumConstant) psiEnumConstant ) ) + .filter( PsiEnumConstant.class::isInstance ) + .map( PsiEnumConstant.class::cast ) + .filter( enumConstant -> !alreadyDefinedValues.contains( enumConstant.getName() ) ) + .map( MapstructUtil::asLookup ) .toArray( LookupElement[]::new ); } diff --git a/src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java b/src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java index 4217f00b..755702df 100644 --- a/src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java +++ b/src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java @@ -40,6 +40,7 @@ import static com.intellij.codeInsight.intention.AddAnnotationPsiFix.addPhysicalAnnotationTo; import static com.intellij.codeInsight.intention.AddAnnotationPsiFix.removePhysicalAnnotations; import static org.mapstruct.intellij.util.MapstructUtil.MAPPING_ANNOTATION_FQN; +import static org.mapstruct.intellij.util.MapstructUtil.VALUE_MAPPING_ANNOTATION_FQN; /** * Utils for working with mapstruct annotation. @@ -269,6 +270,32 @@ private static Stream findMappingAnnotations(@NotNull PsiMethod m .filter( MapstructAnnotationUtils::isMappingAnnotation ); } + public static Stream findAllDefinedValueMappingAnnotations(@NotNull PsiMethod method) { + Stream valueMappingsAnnotations = Stream.empty(); + PsiAnnotation valueMappings = findAnnotation( method, true, MapstructUtil.VALUE_MAPPINGS_ANNOTATION_FQN ); + if ( valueMappings != null ) { + PsiNameValuePair mappingsValue = findDeclaredAttribute( valueMappings, null ); + if ( mappingsValue != null && mappingsValue.getValue() instanceof PsiArrayInitializerMemberValue ) { + valueMappingsAnnotations = Stream.of( ( (PsiArrayInitializerMemberValue) mappingsValue.getValue() ) + .getInitializers() ) + .filter( MapstructAnnotationUtils::isValueMappingPsiAnnotation ) + .map( memberValue -> (PsiAnnotation) memberValue ); + } + else if ( mappingsValue != null && mappingsValue.getValue() instanceof PsiAnnotation ) { + valueMappingsAnnotations = Stream.of( (PsiAnnotation) mappingsValue.getValue() ); + } + } + + Stream valueMappingAnnotations = findValueMappingAnnotations( method ); + + return Stream.concat( valueMappingAnnotations, valueMappingsAnnotations ); + } + + private static Stream findValueMappingAnnotations(@NotNull PsiMethod method) { + return Stream.of( method.getModifierList().getAnnotations() ) + .filter( MapstructAnnotationUtils::isValueMappingAnnotation ); + } + /** * @param memberValue that needs to be checked * @@ -280,6 +307,27 @@ private static boolean isMappingPsiAnnotation(PsiAnnotationMemberValue memberVal && isMappingAnnotation( (PsiAnnotation) memberValue ); } + /** + * @param memberValue that needs to be checked + * + * @return {@code true} if the {@code memberValue} is the {@link org.mapstruct.ValueMapping} {@link PsiAnnotation}, + * {@code false} otherwise + */ + private static boolean isValueMappingPsiAnnotation(PsiAnnotationMemberValue memberValue) { + return memberValue instanceof PsiAnnotation + && isValueMappingAnnotation( (PsiAnnotation) memberValue ); + } + + /** + * @param psiAnnotation that needs to be checked + * + * @return {@code true} if the {@code psiAnnotation} is the {@link org.mapstruct.ValueMapping} annotation, + * {@code false} otherwise + */ + private static boolean isValueMappingAnnotation(PsiAnnotation psiAnnotation) { + return Objects.equals( psiAnnotation.getQualifiedName(), VALUE_MAPPING_ANNOTATION_FQN ); + } + /** * @param psiAnnotation that needs to be checked * diff --git a/src/main/java/org/mapstruct/intellij/util/ValueMappingUtils.java b/src/main/java/org/mapstruct/intellij/util/ValueMappingUtils.java new file mode 100644 index 00000000..3f06cc6c --- /dev/null +++ b/src/main/java/org/mapstruct/intellij/util/ValueMappingUtils.java @@ -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 java.util.Objects; +import java.util.stream.Stream; + +import com.intellij.codeInsight.AnnotationUtil; +import com.intellij.psi.PsiMethod; +import org.jetbrains.annotations.NotNull; + +import static org.mapstruct.intellij.util.MapstructAnnotationUtils.findAllDefinedValueMappingAnnotations; + +/** + * @author Filip Hrisafov + */ +public class ValueMappingUtils { + + private ValueMappingUtils() { + } + + /** + * Find all defined {@link org.mapstruct.ValueMapping#source()} for the given method + * + * @param method that needs to be checked + * + * @return see description + */ + public static Stream findAllDefinedValueMappingSources(@NotNull PsiMethod method) { + return findAllDefinedValueMappingAnnotations( method ) + .map( psiAnnotation -> AnnotationUtil.getDeclaredStringAttributeValue( psiAnnotation, "source" ) ) + .filter( Objects::nonNull ) + .filter( s -> !s.isEmpty() ); + } +} diff --git a/src/test/java/org/mapstruct/intellij/ValueMappingCompletionTestCase.java b/src/test/java/org/mapstruct/intellij/ValueMappingCompletionTestCase.java index 1057b23f..4e8b435c 100644 --- a/src/test/java/org/mapstruct/intellij/ValueMappingCompletionTestCase.java +++ b/src/test/java/org/mapstruct/intellij/ValueMappingCompletionTestCase.java @@ -21,6 +21,33 @@ */ public class ValueMappingCompletionTestCase extends MapstructBaseCompletionTestCase { + @Language("JAVA") + private static final String SOURCE_VALUE_MAPPING_DYNAMIC = "import org.mapstruct.Mapper;\n" + + "import org.mapstruct.ValueMapping;\n" + + "import org.mapstruct.example.ExternalRoofType;\n" + + "import org.mapstruct.example.RoofType;\n" + + "\n" + + "@Mapper\n" + + "public interface RoofTypeMapper {\n" + + "\n" + + " %s" + + " ExternalRoofType map(RoofType type);\n" + + "}"; + + @Language("JAVA") + private static final String SOURCE_VALUE_MAPPINGS_DYNAMIC = "import org.mapstruct.Mapper;\n" + + "import org.mapstruct.ValueMapping;\n" + + "import org.mapstruct.ValueMappings;\n" + + "import org.mapstruct.example.ExternalRoofType;\n" + + "import org.mapstruct.example.RoofType;\n" + + "\n" + + "@Mapper\n" + + "public interface RoofTypeMapper {\n" + + "\n" + + " @ValueMappings({\n%s\n})\n" + + " ExternalRoofType map(RoofType type);\n" + + "}"; + @Language("JAVA") private static final String SOURCE_VALUE_MAPPING = "import org.mapstruct.Mapper;\n" + "import org.mapstruct.ValueMapping;\n" + @@ -82,6 +109,100 @@ public void testSourceValueMappingVariants() { ); } + public void testSourceValueMappingWithExisting() { + String source = String.format( + SOURCE_VALUE_MAPPING_DYNAMIC, + "@ValueMapping(source = \"GAMBREL\", target = \"NORMAL\")\n" + + "@ValueMapping(source = \"%s\", target = \"STANDARD\")\n" + ); + myFixture.configureByText( JavaFileType.INSTANCE, source ); + complete(); + + assertThat( myItems ) + .extracting( LookupElement::getLookupString ) + .containsExactlyInAnyOrder( + "OPEN", + "BOX", + "NORMAL" + ); + assertThat( myItems ) + .extracting( LookupElementPresentation::renderElement ) + .usingElementComparatorIgnoringFields( "myIcon" ) + .containsExactlyInAnyOrder( + createField( "OPEN", "RoofType" ), + createField( "BOX", "RoofType" ), + createField( "NORMAL", "RoofType" ) + ); + } + + public void testSourceValueMappingsWithExisting() { + String source = String.format( + SOURCE_VALUE_MAPPINGS_DYNAMIC, + "@ValueMapping(source = \"GAMBREL\", target = \"NORMAL\"),\n" + + "@ValueMapping(source = \"%s\", target = \"STANDARD\")\n" + ); + myFixture.configureByText( JavaFileType.INSTANCE, source ); + complete(); + + assertThat( myItems ) + .extracting( LookupElement::getLookupString ) + .containsExactlyInAnyOrder( + "OPEN", + "BOX", + "NORMAL" + ); + assertThat( myItems ) + .extracting( LookupElementPresentation::renderElement ) + .usingElementComparatorIgnoringFields( "myIcon" ) + .containsExactlyInAnyOrder( + createField( "OPEN", "RoofType" ), + createField( "BOX", "RoofType" ), + createField( "NORMAL", "RoofType" ) + ); + } + + public void testSourceValueMappingAllValuesAlreadyMapped() { + String source = String.format( + SOURCE_VALUE_MAPPING_DYNAMIC, + "@ValueMapping(source = \"OPEN\", target = \"NORMAL\")\n" + + "@ValueMapping(source = \"BOX\", target = \"NORMAL\")\n" + + "@ValueMapping(source = \"GAMBREL\", target = \"NORMAL\")\n" + + "@ValueMapping(source = \"NORMAL\", target = \"NORMAL\")\n" + + "@ValueMapping(source = \"%s\", target = \"STANDARD\")\n" + ); + myFixture.configureByText( JavaFileType.INSTANCE, source ); + complete(); + + assertThat( myItems ) + .extracting( LookupElement::getLookupString ) + .isEmpty(); + assertThat( myItems ) + .extracting( LookupElementPresentation::renderElement ) + .usingElementComparatorIgnoringFields( "myIcon" ) + .isEmpty(); + } + + public void testSourceValueMappingsAllValuesAlreadyMapped() { + String source = String.format( + SOURCE_VALUE_MAPPINGS_DYNAMIC, + "@ValueMapping(source = \"OPEN\", target = \"NORMAL\"),\n" + + "@ValueMapping(source = \"BOX\", target = \"NORMAL\"),\n" + + "@ValueMapping(source = \"GAMBREL\", target = \"NORMAL\"),\n" + + "@ValueMapping(source = \"NORMAL\", target = \"NORMAL\"),\n" + + "@ValueMapping(source = \"%s\", target = \"STANDARD\")\n" + ); + myFixture.configureByText( JavaFileType.INSTANCE, source ); + complete(); + + assertThat( myItems ) + .extracting( LookupElement::getLookupString ) + .isEmpty(); + assertThat( myItems ) + .extracting( LookupElementPresentation::renderElement ) + .usingElementComparatorIgnoringFields( "myIcon" ) + .isEmpty(); + } + public void testSourceValueMappingResolveToEnum() { myFixture.configureByText( JavaFileType.INSTANCE, String.format( SOURCE_VALUE_MAPPING, "NORMAL" ) );