Adjusting tests and javadocs, cleanup

mplushnikov · Jun 10, 2016 · 1afb51c · 1afb51c
1 parent c139cde
commit 1afb51c
Show file tree

Hide file tree

Showing 12 changed files with 64 additions and 74 deletions.
diff --git a/...java/de/plushnikov/intellij/plugin/processor/modifier/FieldDefaultsModifierProcessor.java b/...java/de/plushnikov/intellij/plugin/processor/modifier/FieldDefaultsModifierProcessor.java
@@ -39,17 +39,16 @@ public boolean isSupported(@NotNull PsiModifierList modifierList) {
   }
 
   @Override
-  @NotNull
-  public Set<String> transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers) {
+  public void transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers) {
 
     PsiClass searchableClass = PsiTreeUtil.getParentOfType(modifierList, PsiClass.class, true);
     if (searchableClass == null) {
-      return modifiers; // Should not get here, but safer to check
+      return; // Should not get here, but safer to check
     }
 
     PsiAnnotation fieldDefaultsAnnotation = PsiAnnotationSearchUtil.findAnnotation(searchableClass, lombok.experimental.FieldDefaults.class);
     if (fieldDefaultsAnnotation == null) {
-      return modifiers; // Should not get here, but safer to check
+      return; // Should not get here, but safer to check
     }
 
     PsiField parentElement = (PsiField) modifierList.getParent();
@@ -68,7 +67,7 @@ public Set<String> transformModifiers(@NotNull PsiModifierList modifierList, @No
         !hasPackagePrivateModifier(modifierList) ||
             // If @PackagePrivate is requested, leave the field as is
             PsiAnnotationSearchUtil.isAnnotatedWith(parentElement, lombok.experimental.PackagePrivate.class)) {
-      return modifiers;
+      return;
     }
 
     switch (defaultAccessLevel) {
@@ -88,12 +87,8 @@ public Set<String> transformModifiers(@NotNull PsiModifierList modifierList, @No
         // no-op
         break;
     }
-
-    return modifiers;
   }
 
-  ;
-
   private boolean hasPackagePrivateModifier(@NotNull PsiModifierList modifierList) {
     return !(modifierList.hasExplicitModifier(PsiModifier.PUBLIC) || modifierList.hasExplicitModifier(PsiModifier.PRIVATE) ||
         modifierList.hasExplicitModifier(PsiModifier.PROTECTED));

diff --git a/src/main/java/de/plushnikov/intellij/plugin/processor/modifier/ModifierProcessor.java b/src/main/java/de/plushnikov/intellij/plugin/processor/modifier/ModifierProcessor.java
@@ -10,29 +10,23 @@
  * To support augmentation of {@link PsiModifierList} properties, processors should implement this interface.
  *
  * @author Alexej Kubarev
- * @see com.intellij.psi.augment.PsiAugmentProvider#hasModifierProperty(PsiModifierList, String)
+ * @see com.intellij.psi.augment.PsiAugmentProvider#transformModifiers(PsiModifierList, Set<String>)
  */
 public interface ModifierProcessor {
 
   /**
    * Validates if this {@link ModifierProcessor} implementation supports provided property on a {@link PsiModifierList}.
-   * This method <strong>should not</strong> do heavy computations and defer them to {@link #hasModifierProperty(PsiModifierList, String)} instead.
+   * This method <strong>should not</strong> do heavy computations and defer them to {@link #transformModifiers(PsiModifierList, Set<String>)} instead.
    *
-   * @param modifierList List the property is queried on
-   * @param name Name of the property
-   * @return true if supported and therefore may be passed to {@link #hasModifierProperty(PsiModifierList, String)}, false otherwise
+   * @param modifierList Modifier List that will have mosifiers augmented
+   * @return true if supported and therefore may be passed to {@link #transformModifiers(PsiModifierList, Set<String>)}, false otherwise
    */
   boolean isSupported(@NotNull PsiModifierList modifierList);
 
   /**
-   * Compute correct response for {@link com.intellij.psi.augment.PsiAugmentProvider#hasModifierProperty(PsiModifierList, String)}.
-   * Must respond with {@literal null} if property existence cannot be identified.
-   * @param modifierList List the property is queried on
-   * @param name Name of the property
-   * @return Boolean value if property existence could be identified, {@literal null} otherwise.
+   * Compute modification of  response for {@link com.intellij.psi.augment.PsiAugmentProvider#transformModifiers(PsiModifierList, Set<String>)}.
+   * @param modifierList Modifier List that will have mosifiers augmented
+   * @param modifiers Set of modifiers that is currently present for the list
    */
-  //Boolean hasModifierProperty(@NotNull PsiModifierList modifierList, @NotNull String name);
-
-  @NotNull
-  Set<String> transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers);
+  void transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers);
 }
diff --git a/.../java/de/plushnikov/intellij/plugin/processor/modifier/UtilityClassModifierProcessor.java b/.../java/de/plushnikov/intellij/plugin/processor/modifier/UtilityClassModifierProcessor.java
@@ -27,14 +27,14 @@ public boolean isSupported(@NotNull PsiModifierList modifierList) {
 
     PsiElement modifierListParent = modifierList.getParent();
 
-    if(modifierListParent instanceof PsiClass) {
+    if (modifierListParent instanceof PsiClass) {
       PsiClass parentClass = (PsiClass) modifierListParent;
-      if(PsiAnnotationSearchUtil.isAnnotatedWith(parentClass, UtilityClass.class)) {
+      if (PsiAnnotationSearchUtil.isAnnotatedWith(parentClass, UtilityClass.class)) {
         return UtilityClassProcessor.validateOnRightType(parentClass, new ProblemNewBuilder());
       }
     }
 
-    if(!isElementFieldMethodOrInnerClass(modifierListParent)) {
+    if (!isElementFieldMethodOrInnerClass(modifierListParent)) {
       return false;
     }
 
@@ -44,24 +44,23 @@ public boolean isSupported(@NotNull PsiModifierList modifierList) {
   }
 
   @Override
-  @NotNull
-  public Set<String> transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers) {
+  public void transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers) {
 
+    final PsiElement parent = modifierList.getParent();
 
     // FINAL
-    PsiElement parent = modifierList.getParent();
-    if(parent instanceof PsiClass) {
+    if (parent instanceof PsiClass) {
       PsiClass psiClass = (PsiClass) parent;
-      if(PsiAnnotationSearchUtil.isAnnotatedWith(psiClass, UtilityClass.class)) {
+      if (PsiAnnotationSearchUtil.isAnnotatedWith(psiClass, UtilityClass.class)) {
         modifiers.add(PsiModifier.FINAL);
       }
     }
 
     // STATIC
-    modifiers.add(PsiModifier.STATIC);
-
-    return modifiers;
-  };
+    if (isElementFieldMethodOrInnerClass(parent)) {
+      modifiers.add(PsiModifier.STATIC);
+    }
+  }
 
   private boolean isElementFieldMethodOrInnerClass(PsiElement element) {
     return element instanceof PsiField || element instanceof PsiMethod || (element instanceof PsiClass && element.getParent() instanceof PsiClass);

diff --git a/src/main/java/de/plushnikov/intellij/plugin/processor/modifier/ValModifierProcessor.java b/src/main/java/de/plushnikov/intellij/plugin/processor/modifier/ValModifierProcessor.java
@@ -25,10 +25,8 @@ public boolean isSupported(@NotNull PsiModifierList modifierList) {
   }
 
   @Override
-  @NotNull
-  public Set<String> transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers) {
-    modifiers.add(PsiModifier.FINAL);
+  public void transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers) {
 
-    return modifiers;
-  };
+    modifiers.add(PsiModifier.FINAL);
+  }
 }
diff --git a/src/main/java/de/plushnikov/intellij/plugin/processor/modifier/ValueModifierProcessor.java b/src/main/java/de/plushnikov/intellij/plugin/processor/modifier/ValueModifierProcessor.java
@@ -23,43 +23,42 @@ public class ValueModifierProcessor implements ModifierProcessor {
   @SuppressWarnings("unchecked")
   public boolean isSupported(@NotNull PsiModifierList modifierList) {
 
-    // @Value only makes fields and class final, methods are to be skipped
     final PsiElement modifierListParent = modifierList.getParent();
+
     if (!(modifierListParent instanceof PsiField || modifierListParent instanceof PsiClass)) {
       return false;
     }
 
     PsiClass searchableClass = PsiTreeUtil.getParentOfType(modifierList, PsiClass.class, true);
 
     return null != searchableClass && PsiAnnotationSearchUtil.isAnnotatedWith(searchableClass, lombok.Value.class, lombok.experimental.Value.class);
+
   }
 
   @Override
-  @NotNull
-  public Set<String> transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers) {
+  public void transformModifiers(@NotNull PsiModifierList modifierList, @NotNull final Set<String> modifiers) {
 
     final PsiModifierListOwner parentElement = PsiTreeUtil.getParentOfType(modifierList, PsiModifierListOwner.class, false);
     if (null != parentElement) {
 
       // FINAL
-
-        if (!PsiAnnotationSearchUtil.isAnnotatedWith(parentElement, lombok.experimental.NonFinal.class)) {
-          modifiers.add(PsiModifier.FINAL);
-        }
-
+      if (!PsiAnnotationSearchUtil.isAnnotatedWith(parentElement, lombok.experimental.NonFinal.class)) {
+        modifiers.add(PsiModifier.FINAL);
+      }
 
       // PRIVATE
-        if (modifierList.getParent() instanceof PsiField &&
-            // Visibility is only changed for package private fields
-            hasPackagePrivateModifier(modifierList) &&
-            // except they are annotated with @PackagePrivate
-            !PsiAnnotationSearchUtil.isAnnotatedWith(parentElement, lombok.experimental.PackagePrivate.class)) {
-          modifiers.add(PsiModifier.PRIVATE);
+      if (modifierList.getParent() instanceof PsiField &&
+          // Visibility is only changed for package private fields
+          hasPackagePrivateModifier(modifierList) &&
+          // except they are annotated with @PackagePrivate
+          !PsiAnnotationSearchUtil.isAnnotatedWith(parentElement, lombok.experimental.PackagePrivate.class)) {
+        modifiers.add(PsiModifier.PRIVATE);
+
+        // IDEA _right now_ checks if other modifiers are set, and ignores PACKAGE_LOCAL but may as well clean it up
+        modifiers.remove(PsiModifier.PACKAGE_LOCAL);
       }
     }
-
-    return modifiers;
-  };
+  }
 
   private boolean hasPackagePrivateModifier(@NotNull PsiModifierList modifierList) {
     return !(modifierList.hasExplicitModifier(PsiModifier.PUBLIC) || modifierList.hasExplicitModifier(PsiModifier.PRIVATE) ||

diff --git a/src/main/java/de/plushnikov/intellij/plugin/provider/LombokAugmentProvider.java b/src/main/java/de/plushnikov/intellij/plugin/provider/LombokAugmentProvider.java
@@ -15,6 +15,7 @@
 import com.intellij.psi.util.CachedValueProvider;
 import com.intellij.psi.util.CachedValuesManager;
 import com.intellij.psi.util.PsiModificationTracker;
+import com.intellij.util.containers.ContainerUtil;
 
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -23,7 +24,6 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 
@@ -58,11 +58,13 @@ protected Set<String> transformModifiers(@NotNull PsiModifierList modifierList,
       return modifiers;
     }
 
-    Set<String> result = new HashSet<String>(modifiers);
+    Set<String> result = ContainerUtil.newConcurrentSet();
+    result.addAll(modifiers);
+
     // Loop through all available processors and give all of them a chance to respond
     for (ModifierProcessor processor : modifierProcessors) {
       if (processor.isSupported(modifierList)) {
-        result = processor.transformModifiers(modifierList, result);
+        processor.transformModifiers(modifierList, result);
       }
     }
 

diff --git a/src/main/java/de/plushnikov/intellij/plugin/util/PsiAnnotationSearchUtil.java b/src/main/java/de/plushnikov/intellij/plugin/util/PsiAnnotationSearchUtil.java
@@ -8,6 +8,7 @@
 import com.intellij.psi.PsiModifierList;
 import com.intellij.psi.PsiModifierListOwner;
 import com.intellij.util.ArrayUtil;
+
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 

diff --git a/src/test/java/de/plushnikov/intellij/plugin/processor/modifier/ValueModifierTest.java b/src/test/java/de/plushnikov/intellij/plugin/processor/modifier/ValueModifierTest.java
@@ -4,6 +4,7 @@
 import com.intellij.psi.PsiField;
 import com.intellij.psi.PsiFile;
 import com.intellij.psi.PsiModifier;
+import com.intellij.psi.PsiModifierList;
 import com.intellij.psi.augment.PsiAugmentProvider;
 import com.intellij.psi.util.PsiTreeUtil;
 import com.intellij.testFramework.PlatformTestUtil;
@@ -43,9 +44,12 @@ public void testValueModifiers() {
     PsiClass clazz = PsiTreeUtil.getParentOfType(field, PsiClass.class);
 
     assertNotNull(clazz);
-    assertNotNull(clazz.getModifierList());
-    assertTrue("@Value should make class final", clazz.getModifierList().hasModifierProperty(PsiModifier.FINAL));
-    assertFalse("@Value should not make class private", clazz.getModifierList().hasModifierProperty(PsiModifier.PRIVATE));
-    assertFalse("@Value should not make class static", clazz.getModifierList().hasModifierProperty(PsiModifier.STATIC));
+
+    PsiModifierList list = clazz.getModifierList();
+
+    assertNotNull(list);
+    assertTrue("@Value should make class final", list.hasModifierProperty(PsiModifier.FINAL));
+    assertFalse("@Value should not make class private", list.hasModifierProperty(PsiModifier.PRIVATE));
+    assertFalse("@Value should not make class static", list.hasModifierProperty(PsiModifier.STATIC));
   }
 }
diff --git a/testData/augment/modifier/UtilityClassModifiersField.java b/testData/augment/modifier/UtilityClassModifiersField.java
@@ -1,7 +1,7 @@
 import lombok.experimental.UtilityClass;
 
-@UtilityClass
+@lombok.experimental.UtilityClass
 public class UtilityClassModifiers {
 
   String field<caret>;
-}
+}
diff --git a/testData/augment/modifier/UtilityClassModifiersInnerClass.java b/testData/augment/modifier/UtilityClassModifiersInnerClass.java
@@ -1,7 +1,7 @@
 import lombok.experimental.UtilityClass;
 
-@UtilityClass
+@lombok.experimental.UtilityClass
 public class UtilityClassModifiers {
 
   public class InnerClass<caret>{}
-}
+}
diff --git a/testData/augment/modifier/UtilityClassModifiersMethod.java b/testData/augment/modifier/UtilityClassModifiersMethod.java
@@ -1,7 +1,7 @@
 import lombok.experimental.UtilityClass;
 
-@UtilityClass
+@lombok.experimental.UtilityClass
 public class UtilityClassModifiers {
 
   public void method<caret>();
-}
+}
diff --git a/testData/augment/modifier/ValueModifiers.java b/testData/augment/modifier/ValueModifiers.java
@@ -1,7 +1,5 @@
-import lombok.Value;
-
-@Value
+@lombok.Value
 public class ValueExample {
 
   String name<caret>;
-}
+}