Do not resolve public static fields / methods and do not use as unmapped properties

filiphr · filiphr · commit 5b87efbf6715 · 2020-06-13T09:38:27.000+02:00
Closes #47
diff --git a/change-notes.html b/change-notes.html
@@ -2,6 +2,7 @@
 <h2>1.2.1</h2>
 <ul>
     <li>Support code completion in Mapping#expression and Mapping#defaultExpression</li>
+    <li>Bug fix: public static fields / methods should not be considered as unmapped properties or resolved in auto completion</li>
 </ul>
 <h2>1.2.0</h2>
 <ul>
diff --git a/src/main/java/org/mapstruct/intellij/codeinsight/references/MapstructSourceReference.java b/src/main/java/org/mapstruct/intellij/codeinsight/references/MapstructSourceReference.java
@@ -25,7 +25,7 @@
 import org.mapstruct.intellij.util.MapstructUtil;
 
 import static org.mapstruct.intellij.util.MapstructUtil.asLookup;
-import static org.mapstruct.intellij.util.MapstructUtil.isPublic;
+import static org.mapstruct.intellij.util.MapstructUtil.isPublicNonStatic;
 import static org.mapstruct.intellij.util.SourceUtils.getParameterType;
 import static org.mapstruct.intellij.util.SourceUtils.publicReadAccessors;
 
@@ -59,12 +59,12 @@ PsiElement resolveInternal(@NotNull String value, @NotNull PsiType psiType) {
         if ( methods.length == 0 ) {
             methods = psiClass.findMethodsByName( "is" + MapstructUtil.capitalize( value ), true );
         }
-        if ( methods.length > 0 ) {
+        if ( methods.length > 0 && isPublicNonStatic( methods[0] ) ) {
             return methods[0];
         }
 
         PsiField field = psiClass.findFieldByName( value, true );
-        if ( field != null && isPublic( field ) ) {
+        if ( field != null && isPublicNonStatic( field ) ) {
             return field;
         }
         return null;
diff --git a/src/main/java/org/mapstruct/intellij/codeinsight/references/MapstructTargetReference.java b/src/main/java/org/mapstruct/intellij/codeinsight/references/MapstructTargetReference.java
@@ -29,6 +29,7 @@
 
 import static org.mapstruct.intellij.util.MapstructUtil.asLookup;
 import static org.mapstruct.intellij.util.MapstructUtil.isPublicModifiable;
+import static org.mapstruct.intellij.util.MapstructUtil.isPublicNonStatic;
 import static org.mapstruct.intellij.util.TargetUtils.getRelevantType;
 import static org.mapstruct.intellij.util.TargetUtils.publicWriteAccessors;
 import static org.mapstruct.intellij.util.TargetUtils.resolveBuilderOrSelfClass;
@@ -79,7 +80,7 @@ PsiElement resolveInternal(@NotNull String value, @NotNull PsiType psiType) {
         }
 
         PsiMethod[] methods = psiClass.findMethodsByName( "set" + MapstructUtil.capitalize( value ), true );
-        if ( methods.length != 0 ) {
+        if ( methods.length != 0 && isPublicNonStatic( methods[0] ) ) {
             return methods[0];
         }
 
diff --git a/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java b/src/main/java/org/mapstruct/intellij/util/MapstructUtil.java
@@ -163,14 +163,17 @@ public static boolean isPublicStatic(@NotNull PsiMethod method) {
         return isPublic( method ) && method.hasModifierProperty( PsiModifier.STATIC );
     }
 
-    public static boolean isPublic(@NotNull PsiField field) {
+    public static boolean isPublicNonStatic(@NotNull PsiField field) {
+        return isPublic( field ) && !field.hasModifierProperty( PsiModifier.STATIC );
+    }
+
+    private static boolean isPublic(@NotNull PsiField field) {
         return field.hasModifierProperty( PsiModifier.PUBLIC );
     }
 
     public static boolean isPublicModifiable(@NotNull PsiField field) {
-        return isPublic( field ) &&
-               !field.hasModifierProperty( PsiModifier.FINAL ) &&
-               !field.hasModifierProperty( PsiModifier.STATIC );
+        return isPublicNonStatic( field ) &&
+               !field.hasModifierProperty( PsiModifier.FINAL );
     }
 
     public static boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType) {
@@ -377,7 +380,7 @@ public static Map<String, Pair<PsiField, PsiSubstitutor>> publicFields(PsiClass
 
         for ( Pair<PsiField, PsiSubstitutor> fieldPair : fieldPairs ) {
             PsiField field = fieldPair.getFirst();
-            if ( MapstructUtil.isPublic( field ) ) {
+            if ( MapstructUtil.isPublicNonStatic( field ) ) {
                 publicFields.put( field.getName(), fieldPair );
             }
         }
diff --git a/src/test/java/org/mapstruct/intellij/MapstructCompletionTestCase.java b/src/test/java/org/mapstruct/intellij/MapstructCompletionTestCase.java
@@ -542,6 +542,20 @@ public void testCarMapperReferencePublicSourceProperty() {
             } );
     }
 
+    public void testCarMapperReferencePublicStaticFieldSourceProperty() {
+        myFixture.configureByFile( "CarMapperReferencePublicStaticFieldSourceProperty.java" );
+        PsiReference reference = myFixture.getFile().findReferenceAt( myFixture.getCaretOffset() );
+        assertThat( reference ).isNotNull();
+        assertThat( reference.resolve() ).isNull();
+    }
+
+    public void testCarMapperReferenceProtectedSourceProperty() {
+        myFixture.configureByFile( "CarMapperReferenceProtectedSourceProperty.java" );
+        PsiReference reference = myFixture.getFile().findReferenceAt( myFixture.getCaretOffset() );
+        assertThat( reference ).isNotNull();
+        assertThat( reference.resolve() ).isNull();
+    }
+
     public void testCarMapperReferenceTargetPropertyInCarDtoWithBuilder() {
         myFixture.configureByFile( "CarMapperReferenceBuilderTargetProperty.java" );
         PsiElement reference = myFixture.getElementAtCaret();
@@ -748,6 +762,20 @@ public void testFluentCarMapperReferenceUnknownTargetProperty() {
         assertThat( reference.resolve() ).isNull();
     }
 
+    public void testCarMapperReferencePublicStaticFieldTargetProperty() {
+        myFixture.configureByFile( "CarMapperReferencePublicStaticFieldTargetProperty.java" );
+        PsiReference reference = myFixture.getFile().findReferenceAt( myFixture.getCaretOffset() );
+        assertThat( reference ).isNotNull();
+        assertThat( reference.resolve() ).isNull();
+    }
+
+    public void testCarMapperReferenceProtectedTargetProperty() {
+        myFixture.configureByFile( "CarMapperReferenceProtectedTargetProperty.java" );
+        PsiReference reference = myFixture.getFile().findReferenceAt( myFixture.getCaretOffset() );
+        assertThat( reference ).isNotNull();
+        assertThat( reference.resolve() ).isNull();
+    }
+
     public void testReferenceCarMapperNoSourceClass() {
         myFixture.configureByFile( "CarMapperNoSourceClass.java" );
         PsiReference reference = myFixture.getFile().findReferenceAt( myFixture.getCaretOffset() );
diff --git a/testData/inspection/UnmappedTargetPropertiesData.java b/testData/inspection/UnmappedTargetPropertiesData.java
@@ -48,6 +48,8 @@ public void setOnlyInSource(String onlyInSource) {
 
     public static class Target {
 
+        public static final String EMPTY_STRING = "";
+
         private String testName;
         private String matching;
         private String moreTarget;
diff --git a/testData/mapping/CarMapperReferenceProtectedSourceProperty.java b/testData/mapping/CarMapperReferenceProtectedSourceProperty.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.ap.test.complex;
+
+import java.util.List;
+
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.example.dto.CarDto;
+import org.example.dto.Car;
+
+@Mapper
+public interface CarMapper {
+
+    @Mapping(source = "privateField<caret>", target = "seatCount")
+    CarDto carToCarDto(Car car);
+}
diff --git a/testData/mapping/CarMapperReferenceProtectedTargetProperty.java b/testData/mapping/CarMapperReferenceProtectedTargetProperty.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.ap.test.complex;
+
+import java.util.List;
+
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.example.dto.CarDto;
+import org.example.dto.Car;
+
+@Mapper
+public interface CarMapper {
+
+    @Mapping(source = "numberOfSeats", target = "privateField<caret>")
+    CarDto carToCarDto(Car car);
+}
diff --git a/testData/mapping/CarMapperReferencePublicStaticFieldSourceProperty.java b/testData/mapping/CarMapperReferencePublicStaticFieldSourceProperty.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.ap.test.complex;
+
+import java.util.List;
+
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.example.dto.CarDto;
+import org.example.dto.CarPublic;
+
+@Mapper
+public interface CarMapper {
+
+    @Mapping(source = "EMPTY_STRING<caret>", target = "seatCount")
+    CarDtoPublic carToCarDto(CarPublic car);
+}
diff --git a/testData/mapping/CarMapperReferencePublicStaticFieldTargetProperty.java b/testData/mapping/CarMapperReferencePublicStaticFieldTargetProperty.java
@@ -0,0 +1,20 @@
+/*
+ * Copyright MapStruct Authors.
+ *
+ * Licensed under the Apache License version 2.0, available at http://www.apache.org/licenses/LICENSE-2.0
+ */
+package org.mapstruct.ap.test.complex;
+
+import java.util.List;
+
+import org.mapstruct.Mapper;
+import org.mapstruct.Mapping;
+import org.example.dto.CarDtoPublic;
+import org.example.dto.Car;
+
+@Mapper
+public interface CarMapper {
+
+    @Mapping(source = "numberOfSeats", target = "EMPTY_STRING<caret>")
+    CarDtoPublic carToCarDto(Car car);
+}
diff --git a/testData/mapping/dto/Car.java b/testData/mapping/dto/Car.java
@@ -95,4 +95,20 @@ public boolean isFree() {
     public void setFree(boolean free) {
         this.free = free;
     }
+
+    protected void setPrivateField(String privateField) {
+        // nothing to do
+    }
+
+    protected String getPrivateField() {
+        return null;
+    }
+
+    public static void setStaticField(String staticField) {
+        // nothing to do
+    }
+
+    public static String getStaticField() {
+        return null;
+    }
 }
diff --git a/testData/mapping/dto/CarDto.java b/testData/mapping/dto/CarDto.java
@@ -100,7 +100,7 @@ public void setCategory(String category) {
     }
 
     protected void setPrivateField(String privateField) {
-        // nothing to di
+        // nothing to do
     }
 
     protected String getPrivateField() {
@@ -114,4 +114,12 @@ public boolean isAvailable() {
     public void setAvailable(boolean available) {
         this.available = available;
     }
+
+    public static void setStaticField(String staticField) {
+        // nothing to do
+    }
+
+    public static String getStaticField() {
+        return null;
+    }
 }
diff --git a/testData/mapping/dto/CarDtoPublic.java b/testData/mapping/dto/CarDtoPublic.java
@@ -12,6 +12,8 @@
 
 public class CarDtoPublic {
 
+    public static final String EMPTY_STRING = "";
+
     public String make;
     public int seatCount;
     public String manufacturingYear;
diff --git a/testData/mapping/dto/CarPublic.java b/testData/mapping/dto/CarPublic.java
@@ -12,6 +12,8 @@
 
 public class CarPublic {
 
+    public static final String EMPTY_STRING = "";
+
     public private String make;
     public private int numberOfSeats;
     public private Date manufacturingDate;

Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,7 @@`
`29`	`29`
`30`	`30`	`import static org.mapstruct.intellij.util.MapstructUtil.asLookup;`
`31`	`31`	`import static org.mapstruct.intellij.util.MapstructUtil.isPublicModifiable;`
	`32`	`+import static org.mapstruct.intellij.util.MapstructUtil.isPublicNonStatic;`
`32`	`33`	`import static org.mapstruct.intellij.util.TargetUtils.getRelevantType;`
`33`	`34`	`import static org.mapstruct.intellij.util.TargetUtils.publicWriteAccessors;`
`34`	`35`	`import static org.mapstruct.intellij.util.TargetUtils.resolveBuilderOrSelfClass;`
`@@ -79,7 +80,7 @@ PsiElement resolveInternal(@NotNull String value, @NotNull PsiType psiType) {`
`79`	`80`	`}`
`80`	`81`
`81`	`82`	`PsiMethod[] methods = psiClass.findMethodsByName( "set" + MapstructUtil.capitalize( value ), true );`
`82`		`- if ( methods.length != 0 ) {`
	`83`	`+ if ( methods.length != 0 && isPublicNonStatic( methods[0] ) ) {`
`83`	`84`	`return methods[0];`
`84`	`85`	`}`
`85`	`86`
Original file line number	Diff line number	Diff line change
`@@ -163,14 +163,17 @@ public static boolean isPublicStatic(@NotNull PsiMethod method) {`
`163`	`163`	`return isPublic( method ) && method.hasModifierProperty( PsiModifier.STATIC );`
`164`	`164`	`}`
`165`	`165`
`166`		`- public static boolean isPublic(@NotNull PsiField field) {`
	`166`	`+ public static boolean isPublicNonStatic(@NotNull PsiField field) {`
	`167`	`+ return isPublic( field ) && !field.hasModifierProperty( PsiModifier.STATIC );`
	`168`	`+ }`
	`169`	`+`
	`170`	`+ private static boolean isPublic(@NotNull PsiField field) {`
`167`	`171`	`return field.hasModifierProperty( PsiModifier.PUBLIC );`
`168`	`172`	`}`
`169`	`173`
`170`	`174`	`public static boolean isPublicModifiable(@NotNull PsiField field) {`
`171`		`- return isPublic( field ) &&`
`172`		`- !field.hasModifierProperty( PsiModifier.FINAL ) &&`
`173`		`- !field.hasModifierProperty( PsiModifier.STATIC );`
	`175`	`+ return isPublicNonStatic( field ) &&`
	`176`	`+ !field.hasModifierProperty( PsiModifier.FINAL );`
`174`	`177`	`}`
`175`	`178`
`176`	`179`	`public static boolean isFluentSetter(@NotNull PsiMethod method, PsiType psiType) {`
`@@ -377,7 +380,7 @@ public static Map<String, Pair<PsiField, PsiSubstitutor>> publicFields(PsiClass`
`377`	`380`
`378`	`381`	`for ( Pair<PsiField, PsiSubstitutor> fieldPair : fieldPairs ) {`
`379`	`382`	`PsiField field = fieldPair.getFirst();`
`380`		`- if ( MapstructUtil.isPublic( field ) ) {`
	`383`	`+ if ( MapstructUtil.isPublicNonStatic( field ) ) {`
`381`	`384`	`publicFields.put( field.getName(), fieldPair );`
`382`	`385`	`}`
`383`	`386`	`}`