From 51f436cefe957bd82d21a557a67735cca067c572 Mon Sep 17 00:00:00 2001 From: hduelme Date: Sun, 7 Jul 2024 01:42:14 +0200 Subject: [PATCH 1/9] Target property mapped more than once inspection --- README.md | 1 + description.html | 1 + ...tPropertyMappedMoreThanOnceInspection.java | 113 ++++++++++++++++++ .../util/MapstructAnnotationUtils.java | 34 +++--- .../intellij/util/MapstructUtil.java | 2 +- .../mapstruct/intellij/util/TargetUtils.java | 7 +- src/main/resources/META-INF/plugin.xml | 8 ++ ...tPropertyMappedMoreThanOnceInspection.html | 29 +++++ .../messages/MapStructBundle.properties | 2 + ...pertyMappedMoreThanOnceInspectionTest.java | 24 ++++ .../TargetPropertyMappedMoreThanOnce.java | 97 +++++++++++++++ 11 files changed, 300 insertions(+), 18 deletions(-) create mode 100644 src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java create mode 100644 src/main/resources/inspectionDescriptions/TargetPropertyMappedMoreThanOnceInspection.html create mode 100644 src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java create mode 100644 testData/inspection/TargetPropertyMappedMoreThanOnce.java diff --git a/README.md b/README.md index 47f98315..76d9936a 100644 --- a/README.md +++ b/README.md @@ -37,6 +37,7 @@ To learn more about MapStruct have a look at the [mapstruct](https://github.com/ * No `source` defined in `@Mapping` annotation * More than one `source` in `@Mapping` annotation defined with quick fixes: Remove `source`. Remove `constant`. Remove `expression`. Use `constant` as `defaultValue`. Use `expression` as `defaultExpression`. * More than one default source in `@Mapping` annotation defined with quick fixes: Remove `defaultValue`. Remove `defaultExpression`. + * `target` mapped more than once by `@Mapping` annotations ## Requirements diff --git a/description.html b/description.html index cf79fbad..380d4a64 100644 --- a/description.html +++ b/description.html @@ -41,6 +41,7 @@
  • No source defined in @Mapping annotation
  • More than one source in @Mapping annotation defined with quick fixes: Remove source. Remove constant. Remove expression. Use constant as defaultValue. Use expression as defaultExpression.
  • More than one default source in @Mapping annotation defined with quick fixes: Remove defaultValue. Remove defaultExpression.
    • +
  • target mapped more than once by @Mapping annotations
    • diff --git a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java new file mode 100644 index 00000000..5cdcad2c --- /dev/null +++ b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java @@ -0,0 +1,113 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at https://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.intellij.inspection; + +import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.psi.JavaElementVisitor; +import com.intellij.psi.PsiAnnotation; +import com.intellij.psi.PsiAnnotationMemberValue; +import com.intellij.psi.PsiClass; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiElementVisitor; +import com.intellij.psi.PsiMethod; +import com.intellij.psi.PsiType; +import org.jetbrains.annotations.NotNull; +import org.mapstruct.intellij.MapStructBundle; +import org.mapstruct.intellij.util.MapStructVersion; +import org.mapstruct.intellij.util.MapstructUtil; +import org.mapstruct.intellij.util.TargetUtils; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static com.intellij.codeInsight.AnnotationUtil.getStringAttributeValue; +import static org.mapstruct.intellij.util.MapstructAnnotationUtils.extractMappingAnnotationsFromMappings; +import static org.mapstruct.intellij.util.MapstructUtil.MAPPINGS_ANNOTATION_FQN; +import static org.mapstruct.intellij.util.MapstructUtil.MAPPING_ANNOTATION_FQN; +import static org.mapstruct.intellij.util.TargetUtils.getTargetType; + +/** + * @author hduelme + */ +public class TargetPropertyMappedMoreThanOnceInspection extends InspectionBase { + @NotNull + @Override + PsiElementVisitor buildVisitorInternal(@NotNull ProblemsHolder holder, boolean isOnTheFly) { + return new TargetPropertyMappedMoreThanOnceInspection.MyJavaElementVisitor( holder, + MapstructUtil.resolveMapStructProjectVersion( holder.getFile() ) ); + } + + private static class MyJavaElementVisitor extends JavaElementVisitor { + private final ProblemsHolder holder; + private final MapStructVersion mapStructVersion; + + private MyJavaElementVisitor(ProblemsHolder holder, MapStructVersion mapStructVersion) { + this.holder = holder; + this.mapStructVersion = mapStructVersion; + } + + @Override + public void visitMethod(PsiMethod method) { + if ( !MapstructUtil.isMapper( method.getContainingClass() ) ) { + return; + } + PsiType targetType = getTargetType( method ); + if ( targetType == null ) { + return; + } + Map> problemMap = new HashMap<>(); + for (PsiAnnotation psiAnnotation :method.getAnnotations()) { + String qualifiedName = psiAnnotation.getQualifiedName(); + if ( MAPPING_ANNOTATION_FQN.equals( qualifiedName ) ) { + handleMappingAnnotation( psiAnnotation, problemMap ); + } + else if (MAPPINGS_ANNOTATION_FQN.equals( qualifiedName )) { + extractMappingAnnotationsFromMappings( psiAnnotation ) + .forEach( a -> handleMappingAnnotation( a, problemMap ) ); + } + else { + // Handle annotations containing at least one Mapping annotation + handleAnnotationWithMappingAnnotation( psiAnnotation, problemMap ); + } + } + for (Map.Entry> problem : problemMap.entrySet()) { + List problemElements = problem.getValue(); + if (problemElements.size() > 1) { + for (PsiElement problemElement : problemElements) { + holder.registerProblem( problemElement, + MapStructBundle.message( "inspection.target.property.mapped.more.than.once", + problem.getKey() ) ); + } + } + } + } + + private void handleAnnotationWithMappingAnnotation(PsiAnnotation psiAnnotation, + Map> problemMap) { + PsiClass annotationClass = psiAnnotation.resolveAnnotationType(); + if (annotationClass == null) { + return; + } + TargetUtils.findAllDefinedMappingTargets( annotationClass, mapStructVersion ) + .forEach( target -> + problemMap.computeIfAbsent( target, k -> new ArrayList<>() ).add( psiAnnotation ) ); + } + + private static void handleMappingAnnotation(PsiAnnotation psiAnnotation, + Map> problemMap) { + PsiAnnotationMemberValue value = psiAnnotation.findDeclaredAttributeValue( "target" ); + if (value != null) { + String target = getStringAttributeValue( value ); + if (target != null) { + problemMap.computeIfAbsent( target, k -> new ArrayList<>() ).add( value ); + } + } + + } + } +} diff --git a/src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java b/src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java index ba9c2b1f..caba7770 100644 --- a/src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java +++ b/src/main/java/org/mapstruct/intellij/util/MapstructAnnotationUtils.java @@ -296,26 +296,32 @@ public static Stream findAllDefinedMappingAnnotations(@NotNull Ps private static Stream findAllDefinedMappingAnnotations(@NotNull PsiModifierListOwner owner, boolean includeMetaAnnotations) { //TODO cache - Stream mappingsAnnotations = Stream.empty(); PsiAnnotation mappings = findAnnotation( owner, true, MapstructUtil.MAPPINGS_ANNOTATION_FQN ); - if ( mappings != null ) { - //TODO maybe there is a better way to do this, but currently I don't have that much knowledge - PsiAnnotationMemberValue mappingsValue = mappings.findDeclaredAttributeValue( null ); - if ( mappingsValue instanceof PsiArrayInitializerMemberValue mappingsArrayInitializerMemberValue ) { - mappingsAnnotations = Stream.of( mappingsArrayInitializerMemberValue.getInitializers() ) - .filter( MapstructAnnotationUtils::isMappingPsiAnnotation ) - .map( PsiAnnotation.class::cast ); - } - else if ( mappingsValue instanceof PsiAnnotation mappingsAnnotation ) { - mappingsAnnotations = Stream.of( mappingsAnnotation ); - } - } - + Stream mappingsAnnotations = extractMappingAnnotationsFromMappings( mappings ); Stream mappingAnnotations = findMappingAnnotations( owner, includeMetaAnnotations ); return Stream.concat( mappingAnnotations, mappingsAnnotations ); } + @NotNull + public static Stream extractMappingAnnotationsFromMappings(@Nullable PsiAnnotation mappings) { + if (mappings == null) { + return Stream.empty(); + } + //TODO maybe there is a better way to do this, but currently I don't have that much knowledge + PsiAnnotationMemberValue mappingsValue = mappings.findDeclaredAttributeValue( null ); + if ( mappingsValue instanceof PsiArrayInitializerMemberValue mappingsArrayInitializerMemberValue) { + return Stream.of( mappingsArrayInitializerMemberValue + .getInitializers() ) + .filter( MapstructAnnotationUtils::isMappingPsiAnnotation ) + .map( PsiAnnotation.class::cast ); + } + else if ( mappingsValue instanceof PsiAnnotation mappingsAnnotation ) { + return Stream.of( mappingsAnnotation ); + } + return Stream.empty(); + } + private static Stream findMappingAnnotations(@NotNull PsiModifierListOwner method, boolean includeMetaAnnotations) { diff --git a/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java b/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java index ceb4559a..1af750e1 100644 --- a/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java +++ b/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java @@ -93,7 +93,7 @@ public final class MapstructUtil { public static final String INHERIT_CONFIGURATION_FQN = InheritConfiguration.class.getName(); public static final String INHERIT_INVERSE_CONFIGURATION_FQN = InheritInverseConfiguration.class.getName(); - static final String MAPPINGS_ANNOTATION_FQN = Mappings.class.getName(); + public static final String MAPPINGS_ANNOTATION_FQN = Mappings.class.getName(); static final String VALUE_MAPPING_ANNOTATION_FQN = ValueMapping.class.getName(); static final String VALUE_MAPPINGS_ANNOTATION_FQN = ValueMappings.class.getName(); private static final String MAPPING_TARGET_ANNOTATION_FQN = MappingTarget.class.getName(); diff --git a/src/main/java/org/mapstruct/intellij/util/TargetUtils.java b/src/main/java/org/mapstruct/intellij/util/TargetUtils.java index 0432fd1b..4193356d 100644 --- a/src/main/java/org/mapstruct/intellij/util/TargetUtils.java +++ b/src/main/java/org/mapstruct/intellij/util/TargetUtils.java @@ -387,14 +387,15 @@ private static boolean hasBuildMethod(@Nullable PsiType builderType, @NotNull Ps /** * Find all defined {@link org.mapstruct.Mapping#target()} for the given method * - * @param method that needs to be checked + * @param owner that needs to be checked * @param mapStructVersion the MapStruct project version * * @return see description */ - public static Stream findAllDefinedMappingTargets(@NotNull PsiMethod method, + @NotNull + public static Stream findAllDefinedMappingTargets(@NotNull PsiModifierListOwner owner, MapStructVersion mapStructVersion) { - return findAllDefinedMappingAnnotations( method, mapStructVersion ) + return findAllDefinedMappingAnnotations( owner, mapStructVersion ) .map( psiAnnotation -> AnnotationUtil.getDeclaredStringAttributeValue( psiAnnotation, "target" ) ) .filter( Objects::nonNull ) .filter( s -> !s.isEmpty() ); diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml index 3a561998..cef1b5a4 100644 --- a/src/main/resources/META-INF/plugin.xml +++ b/src/main/resources/META-INF/plugin.xml @@ -128,6 +128,14 @@ key="inspection.this.target.mapping.no.source.property" shortName="TargetThisMappingNoSourcePropertyInspection" implementationClass="org.mapstruct.intellij.inspection.TargetThisMappingNoSourcePropertyInspection"/> + diff --git a/src/main/resources/inspectionDescriptions/TargetPropertyMappedMoreThanOnceInspection.html b/src/main/resources/inspectionDescriptions/TargetPropertyMappedMoreThanOnceInspection.html new file mode 100644 index 00000000..43578fb0 --- /dev/null +++ b/src/main/resources/inspectionDescriptions/TargetPropertyMappedMoreThanOnceInspection.html @@ -0,0 +1,29 @@ + + +

    + This inspection reports when a target property is explicit mapped more than once +

    +

    +

    
+//wrong
+@Mapper
+public interface EmployeeMapper {
+    @Mapping(source = "employeeName", target = "name")
+    @Mapping(source = "employeeNameLast", target = "name")
+    Employee toEmployee(EmployeeDto employeeDto, @Context CycleAvoidingMappingContext context);
+}
+
    +

    +

    +

    
+//correct
+@Mapper
+public interface EmployeeMapper {
+    @Mapping(source = "employeeName", target = "name")
+    Employee toEmployee(EmployeeDto employeeDto, @Context CycleAvoidingMappingContext context);
+}
+
    +

    + + + diff --git a/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties b/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties index 5732d130..a06f82d1 100644 --- a/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties +++ b/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties @@ -25,6 +25,8 @@ inspection.wrong.map.mapping.map.type.raw=Raw map used for mapping Map to Bean inspection.wrong.map.mapping.map.type.raw.set.default=Replace {0} with {0} inspection.wrong.map.mapping.map.key=Key must be of type String for mapping Map to Bean inspection.wrong.map.mapping.map.key.change.to.string=Change key type to String +inspection.target.property.mapped.more.than.once=Target property ''{0}'' must not be mapped more than once. +inspection.target.property.mapped.more.than.once.title=Target properties must not be mapped more than once. intention.add.ignore.all.unmapped.target.properties=Add ignore all unmapped target properties intention.add.ignore.unmapped.target.property=Add ignore unmapped target property intention.add.unmapped.target.property=Add unmapped target property diff --git a/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java b/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java new file mode 100644 index 00000000..a2849fdb --- /dev/null +++ b/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java @@ -0,0 +1,24 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at https://www.apache.org/licenses/LICENSE-2.0 + */ +package org.mapstruct.intellij.inspection; + +import com.intellij.codeInspection.LocalInspectionTool; +import org.jetbrains.annotations.NotNull; + +/** + * @author hduelme + */ +public class TargetPropertyMappedMoreThanOnceInspectionTest extends BaseInspectionTest { + + @Override + protected @NotNull Class getInspection() { + return TargetPropertyMappedMoreThanOnceInspection.class; + } + + public void testTargetPropertyMappedMoreThanOnce() { + doTest(); + } +} diff --git a/testData/inspection/TargetPropertyMappedMoreThanOnce.java b/testData/inspection/TargetPropertyMappedMoreThanOnce.java new file mode 100644 index 00000000..2437a335 --- /dev/null +++ b/testData/inspection/TargetPropertyMappedMoreThanOnce.java @@ -0,0 +1,97 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at https://www.apache.org/licenses/LICENSE-2.0 + */ + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.Mappings; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +class Target { + private String testName; + + public String getTestName() { + return testName; + } + + public void setTestName(String testName) { + this.testName = testName; + } +} + +class Source { + private String name; + private String lastName; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getLastName() { + return lastName; + } + + public void setLastName(String lastName) { + this.lastName = lastName; + } +} + +@Retention(RetentionPolicy.CLASS) +@Mapping(target = "testName", source = "name") +@interface MyMappingAnnotation { +} + + +@Mapper +interface TargetMappedMoreThanOnceByMappingAnnotationMapper { + + @Mapping(target = "testName", source = "name") + @Mapping(target = "testName", source = "lastName") + Target map(Source source); +} + +@Mapper +interface TargetMappedMoreThanOnceByMappingsAnnotationsMapper { + + @Mappings({ + @Mapping(target = "testName", source = "name"), + @Mapping(target = "testName", source = "lastName") + }) + Target map(Source source); +} + +@Mapper +interface TargetMappedMoreThanOnceByMappingsAnnotationsAndMappingAnnotationMapper { + + @Mapping(target = "testName", source = "name") + @Mappings({ + @Mapping(target = "testName", source = "lastName") + }) + Target map(Source source); +} + +@Mapper +interface TargetMappedMoreThanOnceByMyMappingAnnotationAndMappingAnnotationMapper { + + @Mapping(target = "testName", source = "lastName") + @MyMappingAnnotation + Target map(Source source); +} + +@Mapper +interface TargetMappedMoreThanOnceByMyMappingAnnotationAndMappingsAnnotationMapper { + + @Mappings({ + @Mapping(target = "testName", source = "lastName") + }) + @MyMappingAnnotation + Target map(Source source); +} \ No newline at end of file From f5dc8a55458177e1f05378f26a8541582f90159a Mon Sep 17 00:00:00 2001 From: hduelme Date: Sun, 7 Jul 2024 17:15:58 +0200 Subject: [PATCH 2/9] add change target quick fix and remove annotation quick fix --- ...tPropertyMappedMoreThanOnceInspection.java | 101 +++++++++++++++++- .../messages/MapStructBundle.properties | 2 + ...pertyMappedMoreThanOnceInspectionTest.java | 44 ++++++++ ...argetPropertyMappedMoreThanOnce_after.java | 93 ++++++++++++++++ 4 files changed, 238 insertions(+), 2 deletions(-) create mode 100644 testData/inspection/TargetPropertyMappedMoreThanOnce_after.java diff --git a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java index 5cdcad2c..461daedf 100644 --- a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java +++ b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java @@ -5,13 +5,30 @@ */ package org.mapstruct.intellij.inspection; +import com.intellij.codeInsight.intention.QuickFixFactory; +import com.intellij.codeInspection.LocalQuickFix; +import com.intellij.codeInspection.LocalQuickFixAndIntentionActionOnPsiElement; +import com.intellij.codeInspection.LocalQuickFixOnPsiElement; import com.intellij.codeInspection.ProblemsHolder; +import com.intellij.codeInspection.util.IntentionFamilyName; +import com.intellij.codeInspection.util.IntentionName; +import com.intellij.openapi.editor.CaretState; +import com.intellij.openapi.editor.Editor; +import com.intellij.openapi.editor.LogicalPosition; +import com.intellij.openapi.editor.ScrollType; +import com.intellij.openapi.fileEditor.FileEditor; +import com.intellij.openapi.fileEditor.FileEditorManager; +import com.intellij.openapi.fileEditor.TextEditor; +import com.intellij.openapi.project.Project; +import com.intellij.openapi.util.TextRange; +import com.intellij.openapi.util.text.Strings; import com.intellij.psi.JavaElementVisitor; import com.intellij.psi.PsiAnnotation; import com.intellij.psi.PsiAnnotationMemberValue; import com.intellij.psi.PsiClass; import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementVisitor; +import com.intellij.psi.PsiFile; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiType; import org.jetbrains.annotations.NotNull; @@ -21,9 +38,12 @@ import org.mapstruct.intellij.util.TargetUtils; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Objects; +import java.util.Optional; import static com.intellij.codeInsight.AnnotationUtil.getStringAttributeValue; import static org.mapstruct.intellij.util.MapstructAnnotationUtils.extractMappingAnnotationsFromMappings; @@ -75,18 +95,44 @@ else if (MAPPINGS_ANNOTATION_FQN.equals( qualifiedName )) { handleAnnotationWithMappingAnnotation( psiAnnotation, problemMap ); } } + QuickFixFactory quickFixFactory = QuickFixFactory.getInstance(); for (Map.Entry> problem : problemMap.entrySet()) { List problemElements = problem.getValue(); if (problemElements.size() > 1) { for (PsiElement problemElement : problemElements) { - holder.registerProblem( problemElement, + LocalQuickFix[] quickFixes = getLocalQuickFixes( problemElement, quickFixFactory ); + holder.registerProblem( problemElement, MapStructBundle.message( "inspection.target.property.mapped.more.than.once", - problem.getKey() ) ); + problem.getKey() ), quickFixes ); } } } } + private static @NotNull LocalQuickFix[] getLocalQuickFixes(PsiElement problemElement, + QuickFixFactory quickFixFactory) { + List quickFixes = new ArrayList<>(2); + if (problemElement instanceof PsiAnnotation) { + quickFixes.add( getDeleteFix( problemElement, quickFixFactory ) ); + } + else if (problemElement instanceof PsiAnnotationMemberValue) { + Optional.ofNullable( problemElement.getParent() ).map( PsiElement::getParent ) + .map( PsiElement::getParent ).filter( PsiAnnotation.class::isInstance ) + .ifPresent( annotation -> quickFixes.add( + getDeleteFix( annotation, quickFixFactory ) ) ); + quickFixes.add( new ChangeTargetQuickFix( (PsiAnnotationMemberValue) problemElement) ); + } + return quickFixes.toArray( new LocalQuickFix[]{} ); + } + + private static @NotNull LocalQuickFixAndIntentionActionOnPsiElement getDeleteFix( + PsiElement problemElement, QuickFixFactory quickFixFactory) { + String qualifiedName = Objects.requireNonNullElse( + ( (PsiAnnotation) problemElement).getQualifiedName(), "unknown" ); + return quickFixFactory.createDeleteFix( problemElement, + MapStructBundle.message( "intention.remove.annotation", qualifiedName ) ); + } + private void handleAnnotationWithMappingAnnotation(PsiAnnotation psiAnnotation, Map> problemMap) { PsiClass annotationClass = psiAnnotation.resolveAnnotationType(); @@ -107,7 +153,58 @@ private static void handleMappingAnnotation(PsiAnnotation psiAnnotation, problemMap.computeIfAbsent( target, k -> new ArrayList<>() ).add( value ); } } + } + + private static class ChangeTargetQuickFix extends LocalQuickFixOnPsiElement { + + private final String myText; + private final String myFamilyName; + + private ChangeTargetQuickFix(@NotNull PsiAnnotationMemberValue element) { + super( element ); + myText = MapStructBundle.message( "intention.change.target.property" ); + myFamilyName = MapStructBundle.message( "inspection.target.property.mapped.more.than.once", + element.getText() ); + } + @Override + public @IntentionName @NotNull String getText() { + return myText; + } + + @Override + public void invoke(@NotNull Project project, @NotNull PsiFile psiFile, @NotNull PsiElement psiElement, + @NotNull PsiElement psiElement1) { + FileEditor selectedEditor = FileEditorManager.getInstance( project ).getSelectedEditor(); + if ( selectedEditor instanceof TextEditor) { + Editor editor = ( (TextEditor) selectedEditor ).getEditor(); + + TextRange textRange = psiElement.getTextRange(); + String textOfElement = String.valueOf( editor.getDocument() + .getCharsSequence() + .subSequence( textRange.getStartOffset(), textRange.getEndOffset() ) ); + int targetStart = Strings.indexOf( textOfElement, "\"" ) + 1; + int targetEnd = textOfElement.lastIndexOf( "\"" ); + + editor.getCaretModel().moveToOffset( textRange.getStartOffset() + targetStart ); + LogicalPosition startPosition = editor.getCaretModel().getLogicalPosition(); + editor.getCaretModel().moveToOffset( textRange.getStartOffset() + targetEnd ); + editor.getCaretModel().setCaretsAndSelections( + Collections.singletonList( new CaretState(startPosition, startPosition, + editor.getCaretModel().getLogicalPosition() ) ) ); + editor.getScrollingModel().scrollToCaret( ScrollType.MAKE_VISIBLE ); + } + } + + @Override + public @IntentionFamilyName @NotNull String getFamilyName() { + return myFamilyName; + } + + @Override + public boolean availableInBatchMode() { + return false; + } } } } diff --git a/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties b/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties index a06f82d1..8664108a 100644 --- a/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties +++ b/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties @@ -36,6 +36,8 @@ intention.not.null.checkable.property.source.used.with.default.property=Remove d intention.java.expression.remove.unnecessary.whitespace=Remove unnecessary whitespaces intention.wrong.map.mapping.map.type.raw=Add type to Map for mapping Map to Bean intention.wrong.map.mapping.map.key=Use Map with key of type String for mapping Map to Bean +intention.remove.annotation=Remove {0} annotation +intention.change.target.property=Change target property plugin.settings.title=MapStruct plugin.settings.quickFix.title=Quick fix properties plugin.settings.quickFix.preferSourceBeforeTargetInMapping=Prefer source before target in @Mapping diff --git a/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java b/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java index a2849fdb..cf4ced0a 100644 --- a/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java +++ b/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java @@ -5,9 +5,15 @@ */ package org.mapstruct.intellij.inspection; +import com.intellij.codeInsight.intention.IntentionAction; import com.intellij.codeInspection.LocalInspectionTool; +import com.intellij.openapi.editor.Caret; import org.jetbrains.annotations.NotNull; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + /** * @author hduelme */ @@ -20,5 +26,43 @@ public class TargetPropertyMappedMoreThanOnceInspectionTest extends BaseInspecti public void testTargetPropertyMappedMoreThanOnce() { doTest(); + String testName = getTestName( false ); + List allQuickFixes = myFixture.getAllQuickFixes(); + + assertThat( allQuickFixes ) + .extracting( IntentionAction::getText ) + .as( "Intent Text" ) + .containsExactly( + "Remove org.mapstruct.Mapping annotation", + "Change target property", + "Remove org.mapstruct.Mapping annotation", + "Change target property", + "Remove org.mapstruct.Mapping annotation", + "Change target property", + "Remove org.mapstruct.Mapping annotation", + "Change target property", + "Remove org.mapstruct.Mapping annotation", + "Change target property", + "Remove org.mapstruct.Mapping annotation", + "Change target property", + "Remove org.mapstruct.Mapping annotation", + "Change target property", + "Remove MyMappingAnnotation annotation", + "Remove org.mapstruct.Mapping annotation", + "Change target property", + "Remove MyMappingAnnotation annotation" + ); + + // Delete annotations + myFixture.launchAction(allQuickFixes.get(0)); + myFixture.launchAction(allQuickFixes.get(3)); + myFixture.launchAction(allQuickFixes.get(6)); + myFixture.launchAction(allQuickFixes.get(8)); + myFixture.launchAction(allQuickFixes.get(14)); + // Set cursor + myFixture.launchAction(allQuickFixes.get(16)); + myFixture.checkResultByFile( testName + "_after.java" ); + Caret currentCaret = myFixture.getEditor().getCaretModel().getCurrentCaret(); + assertThat(currentCaret.getSelectedText()).isEqualTo("testName"); } } diff --git a/testData/inspection/TargetPropertyMappedMoreThanOnce_after.java b/testData/inspection/TargetPropertyMappedMoreThanOnce_after.java new file mode 100644 index 00000000..669c79b9 --- /dev/null +++ b/testData/inspection/TargetPropertyMappedMoreThanOnce_after.java @@ -0,0 +1,93 @@ +/* + * Copyright MapStruct Authors. + * + * Licensed under the Apache License version 2.0, available at https://www.apache.org/licenses/LICENSE-2.0 + */ + +import org.mapstruct.Mapper; +import org.mapstruct.Mapping; +import org.mapstruct.Mappings; + +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +class Target { + private String testName; + + public String getTestName() { + return testName; + } + + public void setTestName(String testName) { + this.testName = testName; + } +} + +class Source { + private String name; + private String lastName; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public String getLastName() { + return lastName; + } + + public void setLastName(String lastName) { + this.lastName = lastName; + } +} + +@Retention(RetentionPolicy.CLASS) +@Mapping(target = "testName", source = "name") +@interface MyMappingAnnotation { +} + + +@Mapper +interface TargetMappedMoreThanOnceByMappingAnnotationMapper { + + @Mapping(target = "testName", source = "lastName") + Target map(Source source); +} + +@Mapper +interface TargetMappedMoreThanOnceByMappingsAnnotationsMapper { + + @Mappings({ + @Mapping(target = "testName", source = "name") + }) + Target map(Source source); +} + +@Mapper +interface TargetMappedMoreThanOnceByMappingsAnnotationsAndMappingAnnotationMapper { + + @Mappings({ + @Mapping(target = "testName", source = "lastName") + }) + Target map(Source source); +} + +@Mapper +interface TargetMappedMoreThanOnceByMyMappingAnnotationAndMappingAnnotationMapper { + + @Mapping(target = "testName", source = "lastName") + Target map(Source source); +} + +@Mapper +interface TargetMappedMoreThanOnceByMyMappingAnnotationAndMappingsAnnotationMapper { + + @Mappings({ + @Mapping(target = "testName", source = "lastName") + }) + @MyMappingAnnotation + Target map(Source source); +} \ No newline at end of file From 8ff979bcc71e5f1b3c3ae22240ce88026ddea99a Mon Sep 17 00:00:00 2001 From: hduelme Date: Sun, 7 Jul 2024 17:19:03 +0200 Subject: [PATCH 3/9] formatting --- ...etPropertyMappedMoreThanOnceInspectionTest.java | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java b/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java index cf4ced0a..142693c9 100644 --- a/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java +++ b/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java @@ -54,15 +54,15 @@ public void testTargetPropertyMappedMoreThanOnce() { ); // Delete annotations - myFixture.launchAction(allQuickFixes.get(0)); - myFixture.launchAction(allQuickFixes.get(3)); - myFixture.launchAction(allQuickFixes.get(6)); - myFixture.launchAction(allQuickFixes.get(8)); - myFixture.launchAction(allQuickFixes.get(14)); + myFixture.launchAction( allQuickFixes.get( 0 ) ); + myFixture.launchAction( allQuickFixes.get( 3 ) ); + myFixture.launchAction( allQuickFixes.get( 6 ) ); + myFixture.launchAction( allQuickFixes.get( 8 ) ); + myFixture.launchAction( allQuickFixes.get( 14 ) ); // Set cursor - myFixture.launchAction(allQuickFixes.get(16)); + myFixture.launchAction( allQuickFixes.get( 16 ) ); myFixture.checkResultByFile( testName + "_after.java" ); Caret currentCaret = myFixture.getEditor().getCaretModel().getCurrentCaret(); - assertThat(currentCaret.getSelectedText()).isEqualTo("testName"); + assertThat( currentCaret.getSelectedText( ) ).isEqualTo( "testName" ); } } From 4a13b8341a61c561846b8271d720803ea77fa5ee Mon Sep 17 00:00:00 2001 From: hduelme Date: Sun, 7 Jul 2024 17:28:28 +0200 Subject: [PATCH 4/9] documentation --- README.md | 2 +- description.html | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 76d9936a..57f246d3 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ To learn more about MapStruct have a look at the [mapstruct](https://github.com/ * No `source` defined in `@Mapping` annotation * More than one `source` in `@Mapping` annotation defined with quick fixes: Remove `source`. Remove `constant`. Remove `expression`. Use `constant` as `defaultValue`. Use `expression` as `defaultExpression`. * More than one default source in `@Mapping` annotation defined with quick fixes: Remove `defaultValue`. Remove `defaultExpression`. - * `target` mapped more than once by `@Mapping` annotations + * `target` mapped more than once by `@Mapping` annotations with quick fixes: Remove annotation and change target property. ## Requirements diff --git a/description.html b/description.html index 380d4a64..e658e17a 100644 --- a/description.html +++ b/description.html @@ -41,7 +41,7 @@
  • No source defined in @Mapping annotation
  • More than one source in @Mapping annotation defined with quick fixes: Remove source. Remove constant. Remove expression. Use constant as defaultValue. Use expression as defaultExpression.
  • More than one default source in @Mapping annotation defined with quick fixes: Remove defaultValue. Remove defaultExpression.
    • -
  • target mapped more than once by @Mapping annotations
    • +
  • target mapped more than once by @Mapping annotations with quick fixes: Remove annotation and change target property.
    • From ea778cc8b52f64c2582012e161a14aef4641074f Mon Sep 17 00:00:00 2001 From: hduelme Date: Mon, 15 Jul 2024 17:25:38 +0200 Subject: [PATCH 5/9] formatting --- .../inspection/TargetPropertyMappedMoreThanOnceInspection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java index 461daedf..1b38fd32 100644 --- a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java +++ b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java @@ -81,7 +81,7 @@ public void visitMethod(PsiMethod method) { return; } Map> problemMap = new HashMap<>(); - for (PsiAnnotation psiAnnotation :method.getAnnotations()) { + for (PsiAnnotation psiAnnotation : method.getAnnotations()) { String qualifiedName = psiAnnotation.getQualifiedName(); if ( MAPPING_ANNOTATION_FQN.equals( qualifiedName ) ) { handleMappingAnnotation( psiAnnotation, problemMap ); From c422a205b89926352cf6c092160f17f66c92e86f Mon Sep 17 00:00:00 2001 From: hduelme Date: Mon, 15 Jul 2024 18:20:43 +0200 Subject: [PATCH 6/9] use simple annotation name --- ...rgetPropertyMappedMoreThanOnceInspection.java | 11 ++++++----- ...PropertyMappedMoreThanOnceInspectionTest.java | 16 ++++++++-------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java index 1b38fd32..7f973b2f 100644 --- a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java +++ b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java @@ -29,8 +29,10 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementVisitor; import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiJavaCodeReferenceElement; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiType; +import com.intellij.psi.impl.source.tree.java.PsiAnnotationImpl; import org.jetbrains.annotations.NotNull; import org.mapstruct.intellij.MapStructBundle; import org.mapstruct.intellij.util.MapStructVersion; @@ -42,7 +44,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Optional; import static com.intellij.codeInsight.AnnotationUtil.getStringAttributeValue; @@ -126,11 +127,11 @@ else if (problemElement instanceof PsiAnnotationMemberValue) { } private static @NotNull LocalQuickFixAndIntentionActionOnPsiElement getDeleteFix( - PsiElement problemElement, QuickFixFactory quickFixFactory) { - String qualifiedName = Objects.requireNonNullElse( - ( (PsiAnnotation) problemElement).getQualifiedName(), "unknown" ); + @NotNull PsiElement problemElement, @NotNull QuickFixFactory quickFixFactory) { + + String annotationName = PsiAnnotationImpl.getAnnotationShortName( problemElement.getText() ); return quickFixFactory.createDeleteFix( problemElement, - MapStructBundle.message( "intention.remove.annotation", qualifiedName ) ); + MapStructBundle.message( "intention.remove.annotation", annotationName ) ); } private void handleAnnotationWithMappingAnnotation(PsiAnnotation psiAnnotation, diff --git a/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java b/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java index 142693c9..486f524b 100644 --- a/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java +++ b/src/test/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspectionTest.java @@ -33,22 +33,22 @@ public void testTargetPropertyMappedMoreThanOnce() { .extracting( IntentionAction::getText ) .as( "Intent Text" ) .containsExactly( - "Remove org.mapstruct.Mapping annotation", + "Remove Mapping annotation", "Change target property", - "Remove org.mapstruct.Mapping annotation", + "Remove Mapping annotation", "Change target property", - "Remove org.mapstruct.Mapping annotation", + "Remove Mapping annotation", "Change target property", - "Remove org.mapstruct.Mapping annotation", + "Remove Mapping annotation", "Change target property", - "Remove org.mapstruct.Mapping annotation", + "Remove Mapping annotation", "Change target property", - "Remove org.mapstruct.Mapping annotation", + "Remove Mapping annotation", "Change target property", - "Remove org.mapstruct.Mapping annotation", + "Remove Mapping annotation", "Change target property", "Remove MyMappingAnnotation annotation", - "Remove org.mapstruct.Mapping annotation", + "Remove Mapping annotation", "Change target property", "Remove MyMappingAnnotation annotation" ); From 5181d7d227b74e27133e5674b221bd4a3526de54 Mon Sep 17 00:00:00 2001 From: hduelme Date: Mon, 15 Jul 2024 18:25:44 +0200 Subject: [PATCH 7/9] formatting --- src/main/java/org/mapstruct/intellij/util/MapstructUtil.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java b/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java index 1af750e1..e78be940 100644 --- a/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java +++ b/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java @@ -94,6 +94,7 @@ public final class MapstructUtil { public static final String INHERIT_INVERSE_CONFIGURATION_FQN = InheritInverseConfiguration.class.getName(); public static final String MAPPINGS_ANNOTATION_FQN = Mappings.class.getName(); + static final String VALUE_MAPPING_ANNOTATION_FQN = ValueMapping.class.getName(); static final String VALUE_MAPPINGS_ANNOTATION_FQN = ValueMappings.class.getName(); private static final String MAPPING_TARGET_ANNOTATION_FQN = MappingTarget.class.getName(); From 371ae588692a9cea7b6a1ac2e824c2e71c5864c5 Mon Sep 17 00:00:00 2001 From: hduelme Date: Mon, 15 Jul 2024 18:31:31 +0200 Subject: [PATCH 8/9] formatting --- .../inspection/TargetPropertyMappedMoreThanOnceInspection.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java index 7f973b2f..cba75876 100644 --- a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java +++ b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java @@ -29,7 +29,6 @@ import com.intellij.psi.PsiElement; import com.intellij.psi.PsiElementVisitor; import com.intellij.psi.PsiFile; -import com.intellij.psi.PsiJavaCodeReferenceElement; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiType; import com.intellij.psi.impl.source.tree.java.PsiAnnotationImpl; From 05521ae6d7c8867d3ffcc22838677786704ff7e4 Mon Sep 17 00:00:00 2001 From: hduelme Date: Sun, 21 Jul 2024 14:25:27 +0200 Subject: [PATCH 9/9] use Java 17 features --- .../TargetPropertyMappedMoreThanOnceInspection.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java index cba75876..72a59816 100644 --- a/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java +++ b/src/main/java/org/mapstruct/intellij/inspection/TargetPropertyMappedMoreThanOnceInspection.java @@ -115,12 +115,12 @@ else if (MAPPINGS_ANNOTATION_FQN.equals( qualifiedName )) { if (problemElement instanceof PsiAnnotation) { quickFixes.add( getDeleteFix( problemElement, quickFixFactory ) ); } - else if (problemElement instanceof PsiAnnotationMemberValue) { + else if (problemElement instanceof PsiAnnotationMemberValue problemPsiAnnotationMemberValue) { Optional.ofNullable( problemElement.getParent() ).map( PsiElement::getParent ) .map( PsiElement::getParent ).filter( PsiAnnotation.class::isInstance ) .ifPresent( annotation -> quickFixes.add( getDeleteFix( annotation, quickFixFactory ) ) ); - quickFixes.add( new ChangeTargetQuickFix( (PsiAnnotationMemberValue) problemElement) ); + quickFixes.add( new ChangeTargetQuickFix( problemPsiAnnotationMemberValue ) ); } return quickFixes.toArray( new LocalQuickFix[]{} ); } @@ -176,8 +176,8 @@ private ChangeTargetQuickFix(@NotNull PsiAnnotationMemberValue element) { public void invoke(@NotNull Project project, @NotNull PsiFile psiFile, @NotNull PsiElement psiElement, @NotNull PsiElement psiElement1) { FileEditor selectedEditor = FileEditorManager.getInstance( project ).getSelectedEditor(); - if ( selectedEditor instanceof TextEditor) { - Editor editor = ( (TextEditor) selectedEditor ).getEditor(); + if ( selectedEditor instanceof TextEditor textEditor) { + Editor editor = textEditor.getEditor(); TextRange textRange = psiElement.getTextRange(); String textOfElement = String.valueOf( editor.getDocument()