Skip to content

Commit

Permalink
#203 Add a warning when using source = "." (#204)
Browse files Browse the repository at this point in the history
  • Loading branch information
hduelme authored Jul 23, 2024
1 parent 48f4d6a commit 815011b
Show file tree
Hide file tree
Showing 11 changed files with 384 additions and 47 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions description.html
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
<li>More than one <code>source</code> in <code>@Mapping</code> annotation defined with quick fixes: Remove <code>source</code>. Remove <code>constant</code>. Remove <code>expression</code>. Use <code>constant</code> as <code>defaultValue</code>. Use <code>expression</code> as <code>defaultExpression</code>.</li>
<li>More than one default source in <code>@Mapping</code> annotation defined with quick fixes: Remove <code>defaultValue</code>. Remove <code>defaultExpression</code>.</li>
<li><code>target</code> mapped more than once by <code>@Mapping</code> annotations with quick fixes: Remove annotation and change target property.</li>
<li><code>*</code> used as a source in <code>@Mapping</code> annotations with quick fixes: Replace <code>*</code> with method parameter name.</li>
</ul>
</li>
</ul>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
*
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
@@ -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<LocalQuickFix> 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( "(?<!\")\\s*,?\\s*source\\s*=\\s*\"\\.\"",
"source = \"" + targetMethodeParameterName + "\"" );
parent1.replace( getInstance( project ).createAnnotationFromText( annotationText, parent1 ) );
}
}

@Override
public @IntentionFamilyName @NotNull String getFamilyName() {
return family;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.intellij.psi.PsiNameValuePair;
import com.intellij.psi.PsiReference;
import com.intellij.psi.codeStyle.JavaCodeStyleManager;
import com.intellij.psi.impl.source.tree.java.PsiAnnotationParamListImpl;
import com.intellij.util.IncorrectOperationException;
import com.intellij.util.containers.ContainerUtil;
import org.jetbrains.annotations.NotNull;
Expand Down Expand Up @@ -618,4 +619,49 @@ private static ReportingPolicy getUnmappedTargetPolicyPolicyFromAnnotation(
};
}

@Nullable
public static PsiMethod getAnnotatedMethod(@NotNull PsiAnnotation psiAnnotation) {
PsiElement psiAnnotationParent = psiAnnotation.getParent();
if (psiAnnotationParent == null) {
return null;
}
PsiElement psiAnnotationParentParent = psiAnnotationParent.getParent();
if (psiAnnotationParentParent instanceof PsiMethod annotatedPsiMethod) {
// directly annotated with @Mapping
return annotatedPsiMethod;
}

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 annotatedPsiMethod) {
return annotatedPsiMethod;
}
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 annotatedPsiMethod) {
return annotatedPsiMethod;
}
return null;

}
return null;
}

}
8 changes: 8 additions & 0 deletions src/main/resources/META-INF/plugin.xml
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@
key="inspection.target.property.mapped.more.than.once.title"
shortName="TargetPropertyMappedMoreThanOnceInspection"
implementationClass="org.mapstruct.intellij.inspection.TargetPropertyMappedMoreThanOnceInspection"/>
<localInspection
language="JAVA"
enabledByDefault="true"
level="WARNING"
bundle="org.mapstruct.intellij.messages.MapStructBundle"
key="inspection.source.property.this.used"
shortName="ThisUsedAsSourcePropertyInspection"
implementationClass="org.mapstruct.intellij.inspection.ThisUsedAsSourcePropertyInspection"/>
</extensions>

<actions>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<html>
<body>
<p>
This inspection reports when "." is used as a source in <code>@Mapping</code>
</p>
<p>
<pre><code>
//wrong
@Mapper
public interface EmployeeMapper {
@Mapping(target = "dto", source = ".")
Employee toEmployee(EmployeeDto employeeDto, @Context CycleAvoidingMappingContext context);
}
</code></pre>
</p>
<p>
<pre><code>
//correct
@Mapper
public interface EmployeeMapper {
@Mapping(source = "employeeDto", target = "dto")
Employee toEmployee(EmployeeDto employeeDto, @Context CycleAvoidingMappingContext context);
}
</code></pre>
</p>
<!-- tooltip end -->
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Original file line number Diff line number Diff line change
@@ -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<? extends LocalInspectionTool> getInspection() {
return ThisUsedAsSourcePropertyInspection.class;
}

public void testThisUsedAsSourcePropertyInspection() {
doTest();
List<IntentionAction> 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" );
}
}
Loading

0 comments on commit 815011b

Please sign in to comment.