Skip to content

Commit

Permalink
Add inspection for mapping to target = "." without source (#175)
Browse files Browse the repository at this point in the history
  • Loading branch information
hduelme authored Mar 10, 2024
1 parent f602c3a commit c461730
Show file tree
Hide file tree
Showing 13 changed files with 400 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ public void visitAnnotation(@NotNull PsiAnnotation annotation ) {
&& annotationAttribute.getAttributeValue() != null) {
PsiNameValuePair nameValuePair = (PsiNameValuePair) annotationAttribute;
switch (nameValuePair.getAttributeName()) {
case "target" :
mappingAnnotation.setTargetProperty( nameValuePair );
break;
case "source":
mappingAnnotation.setSourceProperty( nameValuePair );
break;
Expand Down Expand Up @@ -91,6 +94,7 @@ abstract void visitMappingAnnotation( @NotNull ProblemsHolder problemsHolder, @N
@NotNull MappingAnnotation mappingAnnotation );

protected static class MappingAnnotation {
private PsiNameValuePair targetProperty;
private PsiNameValuePair sourceProperty;
private PsiNameValuePair constantProperty;
private PsiNameValuePair defaultValueProperty;
Expand All @@ -101,6 +105,18 @@ protected static class MappingAnnotation {
private PsiNameValuePair qualifiedByNameProperty;
private PsiNameValuePair conditionExpression;

public PsiNameValuePair getTargetProperty() {
return targetProperty;
}

public void setTargetProperty( PsiNameValuePair targetProperty ) {
this.targetProperty = targetProperty;
}

public boolean isNotThisTarget() {
return targetProperty == null || !".".equals( targetProperty.getLiteralValue() );
}

public PsiNameValuePair getSourceProperty() {
return sourceProperty;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,16 @@ void visitMappingAnnotation( @NotNull ProblemsHolder problemsHolder, @NotNull Ps
|| (mappingAnnotation.getSourceProperty() != null && mappingAnnotation.getExpressionProperty() != null)) {
ArrayList<LocalQuickFix> quickFixes = new ArrayList<>( 5 );
String family = MapStructBundle.message( "intention.more.than.one.source.property" );
if (mappingAnnotation.getSourceProperty() != null) {
if (mappingAnnotation.getSourceProperty() != null && mappingAnnotation.isNotThisTarget()) {
quickFixes.add( createRemoveAnnotationAttributeQuickFix( mappingAnnotation.getSourceProperty(),
"Remove source value", family ) );
}
if (mappingAnnotation.getConstantProperty() != null) {
quickFixes.add( createRemoveAnnotationAttributeQuickFix( mappingAnnotation.getConstantProperty(),
"Remove constant value", family ) );

if (mappingAnnotation.hasNoDefaultProperties() && mappingAnnotation.getSourceProperty() != null) {
if (mappingAnnotation.hasNoDefaultProperties() && mappingAnnotation.getSourceProperty() != null
&& mappingAnnotation.isNotThisTarget() ) {
quickFixes.add( createReplaceAsDefaultValueQuickFix(
mappingAnnotation.getConstantProperty(), "constant", "defaultValue",
"Use constant value as default value", family ) );
Expand All @@ -45,7 +46,8 @@ void visitMappingAnnotation( @NotNull ProblemsHolder problemsHolder, @NotNull Ps
if (mappingAnnotation.getExpressionProperty() != null) {
quickFixes.add( createRemoveAnnotationAttributeQuickFix( mappingAnnotation.getExpressionProperty(),
"Remove expression", family ) );
if (mappingAnnotation.hasNoDefaultProperties() && mappingAnnotation.getSourceProperty() != null) {
if (mappingAnnotation.hasNoDefaultProperties() && mappingAnnotation.getSourceProperty() != null
&& mappingAnnotation.isNotThisTarget() ) {
quickFixes.add( createReplaceAsDefaultValueQuickFix(
mappingAnnotation.getExpressionProperty(), "expression", "defaultExpression",
"Use expression as default expression", family ) );
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
/*
* 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.PsiAnnotation;
import org.jetbrains.annotations.NotNull;
import org.mapstruct.intellij.MapStructBundle;

/**
* @author hduelme
*/
public class TargetThisMappingNoSourcePropertyInspection extends MappingAnnotationInspectionBase {

@Override
void visitMappingAnnotation(@NotNull ProblemsHolder problemsHolder, @NotNull PsiAnnotation psiAnnotation,
@NotNull MappingAnnotation mappingAnnotation) {
if ( mappingAnnotation.isNotThisTarget() || mappingAnnotation.getIgnoreProperty() != null) {
return;
}
if (mappingAnnotation.getSourceProperty() == null ) {
problemsHolder.registerProblem( psiAnnotation,
MapStructBundle.message( "inspection.this.target.mapping.no.source.property" ) );
}
}
}
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 @@ -120,6 +120,14 @@
key="inspection.wrong.map.mapping.map.type"
shortName="FromMapMappingInspection"
implementationClass="org.mapstruct.intellij.inspection.FromMapMappingMapTypeInspection"/>
<localInspection
language="JAVA"
enabledByDefault="true"
level="ERROR"
bundle="org.mapstruct.intellij.messages.MapStructBundle"
key="inspection.this.target.mapping.no.source.property"
shortName="TargetThisMappingNoSourcePropertyInspection"
implementationClass="org.mapstruct.intellij.inspection.TargetThisMappingNoSourcePropertyInspection"/>
</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 <code> @Mapping( target = ".") </code> is used without a source property.
</p>
<p>
<pre><code>
//wrong
@Mapper
public interface EmployeeMapper {
@Mapping(target = ".", expression = "java(company.getEmployee())"
Employee toEmployee(Company company, @Context CycleAvoidingMappingContext context);
}
</code></pre>
</p>
<p>
<pre><code>
//correct
@Mapper
public interface EmployeeMapper {
@Mapping(target = ".", source = "employee")
Employee toEmployee(Company company, @Context CycleAvoidingMappingContext context);
}
</code></pre>
</p>
<!-- tooltip end -->
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ inspection.not.null.checkable.property.source.used.with.default.property.title=C
inspection.java.expression.unnecessary.whitespace=Unnecessary whitespaces {0} {1}
inspection.java.expression.remove.unnecessary.whitespace=Remove unnecessary whitespaces {0} {1}
inspection.java.expression.unnecessary.whitespace.title=Unnecessary whitespaces before or after Java expression
inspection.this.target.mapping.no.source.property=Using @Mapping( target = ".") requires a source property. Expression or constant cannot be used as a source
inspection.wrong.map.mapping.map.type=Map type is raw or key type is not string for mapping Map to Bean
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}<String, String>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,21 @@ public void testMoreThanOneSourceConstantAndSource() {
myFixture.checkResultByFile( testName + "_after.java" );
}

public void testMoreThanOneSourceConstantAndSourceThisMapping() {
doTest();
List<IntentionAction> allQuickFixes = myFixture.getAllQuickFixes();
assertThat( allQuickFixes )
.extracting( IntentionAction::getText )
.as( "Intent Text" )
.containsExactly(
"Remove constant value",
"Remove constant value"
);
allQuickFixes.forEach( myFixture::launchAction );
String testName = getTestName( false );
myFixture.checkResultByFile( testName + "_after.java" );
}

public void testMoreThanOneSourceConstantAndExpression() {
doTest();
List<IntentionAction> allQuickFixes = myFixture.getAllQuickFixes();
Expand Down Expand Up @@ -74,4 +89,19 @@ public void testMoreThanOneSourceExpressionAndSource() {
String testName = getTestName( false );
myFixture.checkResultByFile( testName + "_after.java" );
}

public void testMoreThanOneSourceExpressionAndSourceThisMapping() {
doTest();
List<IntentionAction> allQuickFixes = myFixture.getAllQuickFixes();
assertThat( allQuickFixes )
.extracting( IntentionAction::getText )
.as( "Intent Text" )
.containsExactly(
"Remove expression",
"Remove expression"
);
allQuickFixes.forEach( myFixture::launchAction );
String testName = getTestName( false );
myFixture.checkResultByFile( testName + "_after.java" );
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
/*
* 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 TargetThisMappingNoSourcePropertyInspectionTest extends BaseInspectionTest {
@Override
protected @NotNull Class<? extends LocalInspectionTool> getInspection() {
return TargetThisMappingNoSourcePropertyInspection.class;
}

public void testTargetThisMappingNoSourceProperty() {
doTest();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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;

class Source {

private Target innerTarget;

public Target getInnerTarget() {
return innerTarget;
}

public void setInnerTarget(Target innerTarget) {
this.innerTarget = innerTarget;
}
}

class Target {

private String testName;

public String getTestName() {
return testName;
}

public void setTestName(String testName) {
this.testName = testName;
}
}

@Mapper
interface SingleMappingMapper {

<error descr="More than one source property defined">@Mapping(target = ".", source = "innerTarget", constant = "My name")</error>
Target map(Source source);
}

@Mapper
interface SingleMappingsMapper {

@Mappings({
<error descr="More than one source property defined">@Mapping(target = ".", source = "innerTarget", constant = "My name")</error>
})
Target map(Source source);
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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;

class Source {

private Target innerTarget;

public Target getInnerTarget() {
return innerTarget;
}

public void setInnerTarget(Target innerTarget) {
this.innerTarget = innerTarget;
}
}

class Target {

private String testName;

public String getTestName() {
return testName;
}

public void setTestName(String testName) {
this.testName = testName;
}
}

@Mapper
interface SingleMappingMapper {

@Mapping(target = ".", source = "innerTarget")
Target map(Source source);
}

@Mapper
interface SingleMappingsMapper {

@Mappings({
@Mapping(target = ".", source = "innerTarget")
})
Target map(Source source);
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* 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;

class Source {

private Target innerTarget;

public Target getInnerTarget() {
return innerTarget;
}

public void setInnerTarget(Target innerTarget) {
this.innerTarget = innerTarget;
}
}

class Target {

private String testName;

public String getTestName() {
return testName;
}

public void setTestName(String testName) {
this.testName = testName;
}
}

@Mapper
interface SingleMappingMapper {

<error descr="More than one source property defined">@Mapping(target = ".", source = "innerTarget", expression = "java()")</error>
Target map(Source source);
}

@Mapper
interface SingleMappingsMapper {

@Mappings({
<error descr="More than one source property defined">@Mapping(target = ".", source = "innerTarget", expression = "java()")</error>
})
Target map(Source source);
}

Loading

0 comments on commit c461730

Please sign in to comment.