diff --git a/README.md b/README.md index 57f246d..3f44c83 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,7 @@ To learn more about MapStruct have a look at the [mapstruct](https://github.com/ * 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 with quick fixes: Remove annotation and change target property. + * `*` used as a source in `@Mapping` annotation with quick fixes: Replace `*` with method parameter name. ## Requirements diff --git a/description.html b/description.html index e658e17..ef0e473 100644 --- a/description.html +++ b/description.html @@ -42,6 +42,7 @@
  • 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 with quick fixes: Remove annotation and change target property.
    • +
  • * used as a source in @Mapping annotations with quick fixes: Replace * with method parameter name.
    • diff --git a/src/main/java/org/mapstruct/intellij/inspection/NoSourcePropertyDefinedInspection.java b/src/main/java/org/mapstruct/intellij/inspection/NoSourcePropertyDefinedInspection.java index f4fdec0..5df4504 100644 --- a/src/main/java/org/mapstruct/intellij/inspection/NoSourcePropertyDefinedInspection.java +++ b/src/main/java/org/mapstruct/intellij/inspection/NoSourcePropertyDefinedInspection.java @@ -8,17 +8,16 @@ import com.intellij.codeInspection.ProblemsHolder; import com.intellij.psi.PsiAnnotation; import com.intellij.psi.PsiAnnotationMemberValue; -import com.intellij.psi.PsiElement; import com.intellij.psi.PsiLiteralExpression; import com.intellij.psi.PsiMethod; import com.intellij.psi.PsiNameValuePair; -import com.intellij.psi.impl.source.tree.java.PsiAnnotationParamListImpl; import org.jetbrains.annotations.NotNull; -import org.jetbrains.annotations.Nullable; import org.mapstruct.intellij.MapStructBundle; import org.mapstruct.intellij.util.MapstructUtil; import org.mapstruct.intellij.util.SourceUtils; +import static org.mapstruct.intellij.util.MapstructAnnotationUtils.getAnnotatedMethod; + /** * Inspection that checks if inside a @Mapping at least one source property is defined * @@ -59,48 +58,4 @@ private static boolean isIgnoreByDefaultEnabled( @NotNull PsiMethod annotatedMet && Boolean.TRUE.equals( ((PsiLiteralExpression) ignoreByDefault).getValue() ); } - @Nullable - private static PsiMethod getAnnotatedMethod(@NotNull PsiAnnotation psiAnnotation) { - PsiElement psiAnnotationParent = psiAnnotation.getParent(); - if (psiAnnotationParent == null) { - return null; - } - PsiElement psiAnnotationParentParent = psiAnnotationParent.getParent(); - if (psiAnnotationParentParent instanceof PsiMethod) { - // directly annotated with @Mapping - return (PsiMethod) psiAnnotationParentParent; - } - - PsiElement psiAnnotationParentParentParent = psiAnnotationParentParent.getParent(); - if (psiAnnotationParentParentParent instanceof PsiAnnotation) { - // inside @Mappings without array - PsiElement mappingsAnnotationParent = psiAnnotationParentParentParent.getParent(); - if (mappingsAnnotationParent == null) { - return null; - } - PsiElement mappingsAnnotationParentParent = mappingsAnnotationParent.getParent(); - if (mappingsAnnotationParentParent instanceof PsiMethod) { - return (PsiMethod) mappingsAnnotationParentParent; - } - return null; - } - else if (psiAnnotationParentParentParent instanceof PsiAnnotationParamListImpl) { - // inside @Mappings wit array - PsiElement mappingsArray = psiAnnotationParentParentParent.getParent(); - if (mappingsArray == null) { - return null; - } - PsiElement mappingsAnnotationParent = mappingsArray.getParent(); - if (mappingsAnnotationParent == null) { - return null; - } - PsiElement mappingsAnnotationParentParent = mappingsAnnotationParent.getParent(); - if (mappingsAnnotationParentParent instanceof PsiMethod) { - return (PsiMethod) mappingsAnnotationParentParent; - } - return null; - - } - return null; - } } diff --git a/src/main/java/org/mapstruct/intellij/inspection/ThisUsedAsSourcePropertyInspection.java b/src/main/java/org/mapstruct/intellij/inspection/ThisUsedAsSourcePropertyInspection.java new file mode 100644 index 0000000..7a3bcb0 --- /dev/null +++ b/src/main/java/org/mapstruct/intellij/inspection/ThisUsedAsSourcePropertyInspection.java @@ -0,0 +1,105 @@ +/* + * 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.LocalQuickFix; +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.project.Project; +import com.intellij.psi.PsiAnnotation; +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiFile; +import com.intellij.psi.PsiMethod; +import com.intellij.psi.PsiNameValuePair; +import com.intellij.psi.PsiParameter; +import com.intellij.psi.impl.source.tree.java.PsiAnnotationParamListImpl; +import org.jetbrains.annotations.NotNull; +import org.mapstruct.intellij.MapStructBundle; + +import java.util.ArrayList; +import java.util.List; + +import static com.intellij.psi.PsiElementFactory.getInstance; +import static org.mapstruct.intellij.util.MapstructAnnotationUtils.getAnnotatedMethod; +import static org.mapstruct.intellij.util.MapstructUtil.getSourceParameters; + +/** + * @author hduelme + */ +public class ThisUsedAsSourcePropertyInspection extends MappingAnnotationInspectionBase { + @Override + void visitMappingAnnotation(@NotNull ProblemsHolder problemsHolder, @NotNull PsiAnnotation psiAnnotation, + @NotNull MappingAnnotation mappingAnnotation) { + PsiNameValuePair sourceProperty = mappingAnnotation.getSourceProperty(); + if (sourceProperty == null || sourceProperty.getValue() == null) { + return; + } + if ( !".".equals( sourceProperty.getLiteralValue() ) ) { + return; + } + List fixes = new ArrayList<>(); + PsiMethod annotatedMethod = getAnnotatedMethod( psiAnnotation ); + if (annotatedMethod != null) { + for (PsiParameter sourceParameter : getSourceParameters( annotatedMethod )) { + fixes.add( new ReplaceSourceParameterValueQuickFix(sourceProperty, sourceParameter.getName() ) ); + } + } + problemsHolder.registerProblem( sourceProperty.getValue(), + MapStructBundle.message( "inspection.source.property.this.used" ), + fixes.toArray( new LocalQuickFix[0] ) ); + } + + private static class ReplaceSourceParameterValueQuickFix extends LocalQuickFixOnPsiElement { + + private final String targetMethodeParameterName; + private final String text; + private final String family; + + private ReplaceSourceParameterValueQuickFix(@NotNull PsiNameValuePair element, + @NotNull String targetMethodeParameterName) { + super( element ); + this.targetMethodeParameterName = targetMethodeParameterName; + this.text = MapStructBundle.message( "intention.replace.source.property", targetMethodeParameterName ); + this.family = MapStructBundle.message( "inspection.source.property.this.used" ); + } + + @Override + public boolean isAvailable(@NotNull Project project, @NotNull PsiFile file, @NotNull PsiElement startElement, + @NotNull PsiElement endElement ) { + if ( !endElement.isValid() ) { + return false; + } + PsiElement parent = endElement.getParent(); + return parent.isValid() && parent instanceof PsiAnnotationParamListImpl; + } + + @Override + public @IntentionName @NotNull String getText() { + return text; + } + + @Override + public void invoke( @NotNull Project project, @NotNull PsiFile file, @NotNull PsiElement startElement, + @NotNull PsiElement endElement ) { + if (endElement instanceof PsiNameValuePair end) { + PsiAnnotationParamListImpl parent = (PsiAnnotationParamListImpl) end.getParent(); + PsiElement parent1 = parent.getParent(); + + // don't replace inside of strings. Only the constant value name + String annotationText = parent1.getText().replaceFirst( "(? + diff --git a/src/main/resources/inspectionDescriptions/ThisUsedAsSourcePropertyInspection.html b/src/main/resources/inspectionDescriptions/ThisUsedAsSourcePropertyInspection.html new file mode 100644 index 0000000..16fef4b --- /dev/null +++ b/src/main/resources/inspectionDescriptions/ThisUsedAsSourcePropertyInspection.html @@ -0,0 +1,28 @@ + + +

    + This inspection reports when "." is used as a source in @Mapping +

    +

    +

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

    +

    +

    
+//correct
+@Mapper
+public interface EmployeeMapper {
+    @Mapping(source = "employeeDto", target = "dto")
+    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 8664108..d9311be 100644 --- a/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties +++ b/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties @@ -27,6 +27,7 @@ inspection.wrong.map.mapping.map.key=Key must be of type String for mapping Map 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. +inspection.source.property.this.used=''.'' should not be used as a source. 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 @@ -38,6 +39,7 @@ 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 +intention.replace.source.property=Replace source ''.'' with ''{0}'' 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/ThisUsedAsSourcePropertyInspectionTest.java b/src/test/java/org/mapstruct/intellij/inspection/ThisUsedAsSourcePropertyInspectionTest.java new file mode 100644 index 0000000..acde20e --- /dev/null +++ b/src/test/java/org/mapstruct/intellij/inspection/ThisUsedAsSourcePropertyInspectionTest.java @@ -0,0 +1,45 @@ +/* + * 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.codeInsight.intention.IntentionAction; +import com.intellij.codeInspection.LocalInspectionTool; +import org.jetbrains.annotations.NotNull; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * @author hduelme + */ +public class ThisUsedAsSourcePropertyInspectionTest extends BaseInspectionTest { + @Override + protected @NotNull Class getInspection() { + return ThisUsedAsSourcePropertyInspection.class; + } + + public void testThisUsedAsSourcePropertyInspection() { + doTest(); + List allQuickFixes = myFixture.getAllQuickFixes(); + + assertThat( allQuickFixes ) + .extracting( IntentionAction::getText ) + .as( "Intent Text" ) + .containsExactlyInAnyOrder( + "Replace source '.' with 'source'", + "Replace source '.' with 'source'", + "Replace source '.' with 'source'", + "Replace source '.' with 'age'" + ); + + myFixture.launchAction( allQuickFixes.get( 0 ) ); + myFixture.launchAction( allQuickFixes.get( 1 ) ); + myFixture.launchAction( allQuickFixes.get( 2 ) ); + String testName = getTestName( false ); + myFixture.checkResultByFile( testName + "_after.java" ); + } +} diff --git a/testData/inspection/ThisUsedAsSourcePropertyInspection.java b/testData/inspection/ThisUsedAsSourcePropertyInspection.java new file mode 100644 index 0000000..65cff1a --- /dev/null +++ b/testData/inspection/ThisUsedAsSourcePropertyInspection.java @@ -0,0 +1,73 @@ +/* + * 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 org.mapstruct.Context; + +class Source { + + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} + +class Target { + + private Source source; + + public void setSource(Source source) { + this.source = source; + } + + public Source getSource() { + return source; + } +} + +@Mapper +interface SingleMappingMapper { + + @Mapping(target = "source", source = ".") + Target map(Source source); +} + +@Mapper +interface SingleMappingsMapper { + + @Mappings({ + @Mapping(target = "source", source = ".") + }) + Target map(Source source); +} + +@Mapper +interface MultiSourceMappingMapper { + + @Mapping(target = "source", source = ".") + Target map(Source source, Long age, @Context String thisMustNotBeSuggested); +} + +@Mapper +interface NoSourcePropertyMappingMapper { + + @Mapping(target = "source", ignore = true) + Target map(Source source); +} + +@Mapper +interface CorrectSourcePropertyMappingMapper { + + @Mapping(target = "source", source = "source") + Target map(Source source); +} diff --git a/testData/inspection/ThisUsedAsSourcePropertyInspection_after.java b/testData/inspection/ThisUsedAsSourcePropertyInspection_after.java new file mode 100644 index 0000000..e13dee1 --- /dev/null +++ b/testData/inspection/ThisUsedAsSourcePropertyInspection_after.java @@ -0,0 +1,73 @@ +/* + * 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 org.mapstruct.Context; + +class Source { + + private String name; + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } +} + +class Target { + + private Source source; + + public void setSource(Source source) { + this.source = source; + } + + public Source getSource() { + return source; + } +} + +@Mapper +interface SingleMappingMapper { + + @Mapping(target = "source", source = "source") + Target map(Source source); +} + +@Mapper +interface SingleMappingsMapper { + + @Mappings({ + @Mapping(target = "source", source = "source") + }) + Target map(Source source); +} + +@Mapper +interface MultiSourceMappingMapper { + + @Mapping(target = "source", source = "source") + Target map(Source source, Long age, @Context String thisMustNotBeSuggested); +} + +@Mapper +interface NoSourcePropertyMappingMapper { + + @Mapping(target = "source", ignore = true) + Target map(Source source); +} + +@Mapper +interface CorrectSourcePropertyMappingMapper { + + @Mapping(target = "source", source = "source") + Target map(Source source); +}