Quickfixes for removing unnecessary whitespace before and after Java expressions (#164)

hduelme · web-flow · commit 9cb04f31beb0 · 2023-12-10T21:19:16.000+01:00
diff --git a/src/main/java/org/mapstruct/intellij/expression/JavaExpressionInjector.java b/src/main/java/org/mapstruct/intellij/expression/JavaExpressionInjector.java
@@ -49,7 +49,7 @@
  */
 public class JavaExpressionInjector implements MultiHostInjector {
 
-    private static final Pattern JAVA_EXPRESSION = Pattern.compile( "\"java\\(.*\\)\"" );
+    public static final Pattern JAVA_EXPRESSION = Pattern.compile( "\" *java\\(.*\\) *\"" );
 
     private static final ElementPattern<PsiElement> PATTERN =
         StandardPatterns.or(
@@ -280,12 +280,23 @@ else if ( importValue instanceof PsiClassObjectAccessExpression ) {
                         + prefixBuilder,
                     ";\n    }\n}",
                     (PsiLanguageInjectionHost) context,
-                    new TextRange( "\"java(".length(), context.getTextRange().getLength() - ")\"".length() )
+                    getTextRange( context )
                 )
                 .doneInjecting();
         }
     }
 
+    @NotNull
+    private static TextRange getTextRange(@NotNull PsiElement context) {
+        String text = context.getText();
+        return new TextRange(text.indexOf( "java(" ) + 5,
+                context.getTextRange().getLength() - getEndOfExpression( text ) );
+    }
+
+    private static int getEndOfExpression(@NotNull String text) {
+        return text.length() - text.lastIndexOf( ')' );
+    }
+
     @NotNull
     @Override
     public List<? extends Class<? extends PsiElement>> elementsToInjectIn() {
diff --git a/src/main/java/org/mapstruct/intellij/inspection/JavaExpressionUnnecessaryWhitespacesInspector.java b/src/main/java/org/mapstruct/intellij/inspection/JavaExpressionUnnecessaryWhitespacesInspector.java
@@ -0,0 +1,153 @@
+/*
+ * 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.LocalQuickFixOnPsiElement;
+import com.intellij.codeInspection.ProblemHighlightType;
+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.PsiAnnotationMemberValue;
+import com.intellij.psi.PsiElement;
+import com.intellij.psi.PsiFile;
+import com.intellij.psi.PsiNameValuePair;
+import org.jetbrains.annotations.NotNull;
+import org.mapstruct.intellij.MapStructBundle;
+
+import static com.intellij.psi.PsiElementFactory.getInstance;
+import static org.mapstruct.intellij.expression.JavaExpressionInjector.JAVA_EXPRESSION;
+
+public class JavaExpressionUnnecessaryWhitespacesInspector extends MappingAnnotationInspectionBase {
+
+    @Override
+    void visitMappingAnnotation(@NotNull ProblemsHolder problemsHolder, @NotNull PsiAnnotation psiAnnotation,
+                                @NotNull MappingAnnotation mappingAnnotation) {
+       inspectUnnecessaryWhitespaces( problemsHolder, mappingAnnotation.getExpressionProperty() );
+       inspectUnnecessaryWhitespaces( problemsHolder, mappingAnnotation.getDefaultExpressionProperty() );
+       inspectUnnecessaryWhitespaces( problemsHolder, mappingAnnotation.getConditionExpression() );
+    }
+
+    private void inspectUnnecessaryWhitespaces(@NotNull ProblemsHolder problemsHolder, PsiNameValuePair property) {
+        if ( property == null ) {
+            return;
+        }
+        PsiAnnotationMemberValue value = property.getValue();
+        if ( value == null ) {
+            return;
+        }
+        String text = value.getText();
+        if ( !JAVA_EXPRESSION.matcher( text ).matches() ) {
+            return;
+        }
+        if ( text.indexOf( "java(" ) > 1 ) {
+            problemsHolder.registerProblem( property,
+                    MapStructBundle.message( "inspection.java.expression.unnecessary.whitespace",
+                            "before", property.getAttributeName() ),
+                    ProblemHighlightType.WEAK_WARNING, new RemoveWhitespacesBefore(property) );
+        }
+        if ( text.lastIndexOf( ')' ) < text.length() - 2) {
+            problemsHolder.registerProblem( property,
+                    MapStructBundle.message( "inspection.java.expression.unnecessary.whitespace",
+                    "after", property.getAttributeName() ),
+                    ProblemHighlightType.WEAK_WARNING, new RemoveWhitespacesAfter(property) );
+        }
+    }
+
+    private static class RemoveWhitespacesBefore extends LocalQuickFixOnPsiElement {
+
+        private final String name;
+
+        private RemoveWhitespacesBefore(@NotNull PsiNameValuePair element) {
+            super( element );
+            this.name = element.getName();
+        }
+
+        @Override
+        public @IntentionName @NotNull String getText() {
+            return MapStructBundle.message( "inspection.java.expression.remove.unnecessary.whitespace",
+                    "before", name );
+        }
+
+        @Override
+        public void invoke(@NotNull Project project, @NotNull PsiFile psiFile, @NotNull PsiElement psiElement,
+                           @NotNull PsiElement psiElement1) {
+            if (psiElement instanceof PsiNameValuePair) {
+                PsiNameValuePair psiNameValuePair = (PsiNameValuePair) psiElement;
+                PsiAnnotationMemberValue value = psiNameValuePair.getValue();
+                if (value != null) {
+                    String text = value.getText();
+                    psiNameValuePair.setValue( getInstance( project )
+                            .createExpressionFromText( "\"" + text.substring( text.indexOf( "java(" ) ), value ) );
+                }
+            }
+        }
+
+        @Override
+        public @IntentionFamilyName @NotNull String getFamilyName() {
+            return MapStructBundle.message( "intention.java.expression.remove.unnecessary.whitespace" );
+        }
+
+        @Override
+        public boolean isAvailable(@NotNull Project project, @NotNull PsiFile file, @NotNull PsiElement startElement,
+                                   @NotNull PsiElement endElement) {
+            if ( !super.isAvailable( project, file, startElement, endElement ) ) {
+                return false;
+            }
+            if ( !(startElement instanceof PsiNameValuePair ) ) {
+                return false;
+            }
+            return ((PsiNameValuePair) startElement).getValue() != null;
+        }
+    }
+
+    private static class RemoveWhitespacesAfter extends LocalQuickFixOnPsiElement {
+
+        private final String name;
+
+        private RemoveWhitespacesAfter(@NotNull PsiNameValuePair element) {
+            super( element );
+            this.name = element.getName();
+        }
+
+        @Override
+        public @IntentionName @NotNull String getText() {
+            return MapStructBundle.message( "inspection.java.expression.remove.unnecessary.whitespace", "after", name );
+        }
+
+        @Override
+        public void invoke(@NotNull Project project, @NotNull PsiFile psiFile, @NotNull PsiElement psiElement,
+                           @NotNull PsiElement psiElement1) {
+            if (psiElement instanceof PsiNameValuePair) {
+                PsiNameValuePair psiNameValuePair = (PsiNameValuePair) psiElement;
+                PsiAnnotationMemberValue value = psiNameValuePair.getValue();
+                if (value != null) {
+                    String text = value.getText();
+                    psiNameValuePair.setValue( getInstance( project ).createExpressionFromText(
+                            text.substring( 0, text.lastIndexOf( ')' ) + 1 ) + "\"", value ) );
+                }
+            }
+        }
+
+        @Override
+        public @IntentionFamilyName @NotNull String getFamilyName() {
+            return MapStructBundle.message( "intention.java.expression.remove.unnecessary.whitespace" );
+        }
+
+        @Override
+        public boolean isAvailable(@NotNull Project project, @NotNull PsiFile file, @NotNull PsiElement startElement,
+                                   @NotNull PsiElement endElement) {
+            if ( !super.isAvailable( project, file, startElement, endElement ) ) {
+                return false;
+            }
+            if ( !(startElement instanceof PsiNameValuePair ) ) {
+                return false;
+            }
+            return ((PsiNameValuePair) startElement).getValue() != null;
+        }
+    }
+}
diff --git a/src/main/java/org/mapstruct/intellij/inspection/MappingAnnotationInspectionBase.java b/src/main/java/org/mapstruct/intellij/inspection/MappingAnnotationInspectionBase.java
@@ -38,7 +38,7 @@ private MyJavaElementVisitor( ProblemsHolder problemsHolder ) {
         }
 
         @Override
-        public void visitAnnotation( PsiAnnotation annotation ) {
+        public void visitAnnotation(@NotNull PsiAnnotation annotation ) {
             super.visitAnnotation( annotation );
             if (annotation.hasQualifiedName( MapstructUtil.MAPPING_ANNOTATION_FQN )) {
                 MappingAnnotation mappingAnnotation = new MappingAnnotation();
@@ -72,6 +72,9 @@ public void visitAnnotation( PsiAnnotation annotation ) {
                             case "qualifiedByName":
                                 mappingAnnotation.setQualifiedByNameProperty( nameValuePair );
                                 break;
+                            case "conditionExpression":
+                                mappingAnnotation.setConditionExpression( nameValuePair );
+                                break;
                             default:
                                 break;
                         }
@@ -96,6 +99,7 @@ protected static class MappingAnnotation {
         private PsiNameValuePair ignoreProperty;
         private PsiNameValuePair dependsOnProperty;
         private PsiNameValuePair qualifiedByNameProperty;
+        private PsiNameValuePair conditionExpression;
 
         public PsiNameValuePair getSourceProperty() {
             return sourceProperty;
@@ -170,6 +174,14 @@ public PsiNameValuePair getQualifiedByNameProperty() {
         public void setQualifiedByNameProperty(PsiNameValuePair qualifiedByNameProperty) {
             this.qualifiedByNameProperty = qualifiedByNameProperty;
         }
+
+        public PsiNameValuePair getConditionExpression() {
+            return conditionExpression;
+        }
+
+        public void setConditionExpression(PsiNameValuePair conditionExpression) {
+            this.conditionExpression = conditionExpression;
+        }
     }
 
     protected static RemoveAnnotationAttributeQuickFix createRemoveAnnotationAttributeQuickFix(
diff --git a/src/main/resources/META-INF/plugin.xml b/src/main/resources/META-INF/plugin.xml
@@ -101,9 +101,17 @@
             enabledByDefault="true"
             level="ERROR"
             bundle="org.mapstruct.intellij.messages.MapStructBundle"
-            key="inspection.not.null.checkable.property.source.used.with.default.property"
+            key="inspection.not.null.checkable.property.source.used.with.default.property.title"
             shortName="NotNullCheckableSourcePropertyUsedWithDefaultValue"
             implementationClass="org.mapstruct.intellij.inspection.NotNullCheckableSourcePropertyUsedWithDefaultValueInspection"/>
+    <localInspection
+            language="JAVA"
+            enabledByDefault="true"
+            level="WARNING"
+            bundle="org.mapstruct.intellij.messages.MapStructBundle"
+            key="inspection.java.expression.unnecessary.whitespace.title"
+            shortName="JavaExpressionUnnecessaryWhitespaces"
+            implementationClass="org.mapstruct.intellij.inspection.JavaExpressionUnnecessaryWhitespacesInspector"/>
   </extensions>
 
   <actions>
diff --git a/src/main/resources/inspectionDescriptions/JavaExpressionUnnecessaryWhitespaces.html b/src/main/resources/inspectionDescriptions/JavaExpressionUnnecessaryWhitespaces.html
@@ -0,0 +1,27 @@
+<html>
+<body>
+<p>
+    This inspection reports when a java expression has whitespaces before or after the <code>java()</code> block.
+</p>
+<pre><code>
+//wrong
+@Mapper
+public interface EmployeeMapper {
+    @Mapping(source = "employeeName", expression = "  java(\"Name\")")
+    Employee toEmployee(EmployeeDto employeeDto, @Context CycleAvoidingMappingContext context);
+}
+</code></pre>
+</p>
+<p>
+<pre><code>
+//correct
+@Mapper
+public interface EmployeeMapper {
+    @Mapping(source = "employeeName", expression = "java(\"Name\")")
+    Employee toEmployee(EmployeeDto employeeDto, @Context CycleAvoidingMappingContext context);
+}
+</code></pre>
+</p>
+<!-- tooltip end -->
+</body>
+</html>
diff --git a/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties b/src/main/resources/org/mapstruct/intellij/messages/MapStructBundle.properties
@@ -15,13 +15,18 @@ inspection.no.source.property=No source property defined
 inspection.more.than.one.source.property=More than one source property defined
 inspection.more.than.one.default.source.property=More than one default source property defined
 inspection.not.null.checkable.property.source.used.with.default.property={0} property used with {1}
+inspection.not.null.checkable.property.source.used.with.default.property.title=Constant or expression source property used with a default source property
+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
 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
 intention.no.source.property=Add one source property
 intention.more.than.one.source.property=Only use one source property
 intention.more.than.one.default.source.property=Only use one default source property
 intention.not.null.checkable.property.source.used.with.default.property=Remove default properties
+intention.java.expression.remove.unnecessary.whitespace=Remove unnecessary whitespaces
 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/expression/JavaExpressionInjectionTest.java b/src/test/java/org/mapstruct/intellij/expression/JavaExpressionInjectionTest.java
@@ -268,6 +268,42 @@ protected void withTargetDefinedMapper(String attribute) {
         assertThat( elementAt.getText() ).isEqualTo( ";" );
     }
 
+    public void testExpressionWithTargetDefinedAndWhitespacesMapper() {
+        withTargetDefinedAndWhitespaceMapper( "expression" );
+        withTargetDefinedAndWhitespaceMapper( "defaultExpression" );
+        withTargetDefinedAndWhitespaceMapper( "conditionExpression" );
+    }
+
+    protected void withTargetDefinedAndWhitespaceMapper(String attribute) {
+        String mapping = "@Mapping(target = \"manufacturingYear\", " + attribute + " = \" java(car.<caret>)  \")\n";
+        @Language("java")
+        String mapper = formatMapper( CAR_MAPPER, mapping );
+        PsiFile file = configureMapperByText( mapper );
+
+        assertThat( myFixture.completeBasic() )
+                .extracting( LookupElementPresentation::renderElement )
+                .extracting( LookupElementPresentation::getItemText )
+                .contains(
+                        "getMake",
+                        "setMake",
+                        "getManufacturingDate",
+                        "setManufacturingDate",
+                        "getNumberOfSeats",
+                        "setNumberOfSeats"
+                );
+
+        assertThat( myFixture.complete( CompletionType.SMART ) )
+                .extracting( LookupElementPresentation::renderElement )
+                .extracting( LookupElementPresentation::getItemText )
+                .containsExactlyInAnyOrder( "getMake", "toString" );
+
+        PsiElement elementAt = file.findElementAt( myFixture.getCaretOffset() );
+        assertThat( elementAt )
+                .isNotNull()
+                .isInstanceOf( PsiJavaToken.class );
+        assertThat( elementAt.getText() ).isEqualTo( ";" );
+    }
+
     public void testExpressionWithTargetDefinedMapperInMappings() {
         withTargetDefinedMapperInMappings( "expression" );
         withTargetDefinedMapperInMappings( "defaultExpression" );
diff --git a/src/test/java/org/mapstruct/intellij/inspection/JavaExpressionUnnecessaryWhitespacesInspectorTest.java b/src/test/java/org/mapstruct/intellij/inspection/JavaExpressionUnnecessaryWhitespacesInspectorTest.java
@@ -0,0 +1,57 @@
+/*
+ * 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;
+
+public class JavaExpressionUnnecessaryWhitespacesInspectorTest extends BaseInspectionTest {
+    @Override
+    protected @NotNull Class<? extends LocalInspectionTool> getInspection() {
+        return JavaExpressionUnnecessaryWhitespacesInspector.class;
+    }
+
+    public void testJavaExpressionUnnecessaryWhitespacesInspectorWhitespaceBefore() {
+        doTest();
+        String testName = getTestName( false );
+        List<IntentionAction> allQuickFixes = myFixture.getAllQuickFixes();
+
+        assertThat( allQuickFixes )
+                .extracting( IntentionAction::getText )
+                .as( "Intent Text" )
+                .containsExactly(
+                        "Remove unnecessary whitespaces before conditionExpression",
+                        "Remove unnecessary whitespaces before defaultExpression",
+                        "Remove unnecessary whitespaces before expression"
+                );
+
+        allQuickFixes.forEach( myFixture::launchAction );
+        myFixture.checkResultByFile( testName + "_after.java" );
+    }
+
+    public void testJavaExpressionUnnecessaryWhitespacesInspectorWhitespaceAfter() {
+        doTest();
+        String testName = getTestName( false );
+        List<IntentionAction> allQuickFixes = myFixture.getAllQuickFixes();
+
+        assertThat( allQuickFixes )
+                .extracting( IntentionAction::getText )
+                .as( "Intent Text" )
+                .containsExactly(
+                        "Remove unnecessary whitespaces after conditionExpression",
+                        "Remove unnecessary whitespaces after defaultExpression",
+                        "Remove unnecessary whitespaces after expression"
+                );
+
+        allQuickFixes.forEach( myFixture::launchAction );
+        myFixture.checkResultByFile( testName + "_after.java" );
+    }
+}
diff --git a/testData/inspection/JavaExpressionUnnecessaryWhitespacesInspectorWhitespaceAfter.java b/testData/inspection/JavaExpressionUnnecessaryWhitespacesInspectorWhitespaceAfter.java
diff --git a/testData/inspection/JavaExpressionUnnecessaryWhitespacesInspectorWhitespaceAfter_after.java b/testData/inspection/JavaExpressionUnnecessaryWhitespacesInspectorWhitespaceAfter_after.java
diff --git a/testData/inspection/JavaExpressionUnnecessaryWhitespacesInspectorWhitespaceBefore.java b/testData/inspection/JavaExpressionUnnecessaryWhitespacesInspectorWhitespaceBefore.java
diff --git a/testData/inspection/JavaExpressionUnnecessaryWhitespacesInspectorWhitespaceBefore_after.java b/testData/inspection/JavaExpressionUnnecessaryWhitespacesInspectorWhitespaceBefore_after.java
