added method parameter count validation for @DeleGate

added tests for validation cleanup unused methods
mplushnikov · Feb 16, 2014 · 474f6e4 · 474f6e4
1 parent e84b3dc
commit 474f6e4
Show file tree

Hide file tree

Showing 10 changed files with 97 additions and 89 deletions.
diff --git a/...ugin/src/main/java/de/plushnikov/intellij/plugin/action/delombok/BaseDelombokHandler.java b/...ugin/src/main/java/de/plushnikov/intellij/plugin/action/delombok/BaseDelombokHandler.java
@@ -19,12 +19,12 @@
 import com.intellij.psi.PsiNameValuePair;
 import com.intellij.psi.PsiParameter;
 import com.intellij.psi.PsiType;
-import com.intellij.psi.PsiTypeParameter;
 import com.intellij.psi.PsiTypeParameterList;
 import com.intellij.psi.codeStyle.CodeStyleManager;
 import com.intellij.psi.codeStyle.JavaCodeStyleManager;
 import de.plushnikov.intellij.plugin.processor.AbstractProcessor;
 import de.plushnikov.intellij.plugin.settings.ProjectSettings;
+import de.plushnikov.intellij.plugin.util.PsiMethodUtil;
 import org.jetbrains.annotations.NotNull;
 
 import java.util.ArrayList;
@@ -118,13 +118,9 @@ private PsiMethod rebuildMethod(@NotNull Project project, @NotNull PsiMethod fro
 
     final PsiTypeParameterList fromMethodTypeParameterList = fromMethod.getTypeParameterList();
     if (null != fromMethodTypeParameterList) {
-      PsiTypeParameter[] typeParameters = fromMethodTypeParameterList.getTypeParameters();
-      if (typeParameters.length > 0) {
-        PsiTypeParameterList parameterList = (PsiTypeParameterList) resultMethod.addAfter(
-            elementFactory.createTypeParameterList(), resultMethod.getModifierList());
-        for (PsiTypeParameter typeParameter : typeParameters) {
-          parameterList.add(elementFactory.createTypeParameter(typeParameter.getName(), typeParameter.getExtendsList().getReferencedTypes()));
-        }
+      PsiTypeParameterList typeParameterList = PsiMethodUtil.createTypeParameterList(fromMethodTypeParameterList);
+      if (null != typeParameterList) {
+        resultMethod.addAfter(typeParameterList, resultMethod.getModifierList());
       }
     }
 

diff --git a/...plugin/src/main/java/de/plushnikov/intellij/plugin/processor/handler/DelegateHandler.java b/...plugin/src/main/java/de/plushnikov/intellij/plugin/processor/handler/DelegateHandler.java
@@ -36,8 +36,11 @@
 import java.util.HashSet;
 import java.util.List;
 
+/**
+ * Handler for Delegate annotation processing, for fields and for methods
+ */
 public class DelegateHandler {
-  //<T extends PsiVariable & PsiMember>
+
   public boolean validate(@NotNull PsiType psiType, @NotNull PsiAnnotation psiAnnotation, @NotNull ProblemBuilder builder) {
     boolean result;
 
@@ -47,8 +50,6 @@ public boolean validate(@NotNull PsiType psiType, @NotNull PsiAnnotation psiAnno
     final Collection<PsiType> excludes = collectExcludeTypes(psiAnnotation);
     result &= validateTypes(excludes, builder);
 
-    //TODO Error: delegation of methods that doesn't exists on type
-
     return result;
   }
 

diff --git a/...src/main/java/de/plushnikov/intellij/plugin/processor/method/DelegateMethodProcessor.java b/...src/main/java/de/plushnikov/intellij/plugin/processor/method/DelegateMethodProcessor.java
@@ -22,8 +22,16 @@ public DelegateMethodProcessor() {
 
   @Override
   protected boolean validate(@NotNull PsiAnnotation psiAnnotation, @NotNull PsiMethod psiMethod, @NotNull ProblemBuilder builder) {
+    boolean result = true;
+    if (psiMethod.getParameterList().getParametersCount() > 0) {
+      builder.addError("@Delegate is legal only on no-argument methods.");
+      result = false;
+    }
+
     final PsiType returnType = psiMethod.getReturnType();
-    return null != returnType && handler.validate(returnType, psiAnnotation, builder);
+    result &= null != returnType && handler.validate(returnType, psiAnnotation, builder);
+
+    return result;
   }
 
   @Override

diff --git a/lombok-plugin/src/main/java/de/plushnikov/intellij/plugin/util/IntelliJVersionRangeUtil.java b/lombok-plugin/src/main/java/de/plushnikov/intellij/plugin/util/IntelliJVersionRangeUtil.java
@@ -30,7 +30,6 @@ public static IntelliJVersion getIntelliJVersion(@NotNull final BuildNumber buil
     IntelliJVersion result = IntelliJVersion.UNKNOWN;
 
     final int baselineVersion = buildNumber.getBaselineVersion();
-    final int build = buildNumber.getBuildNumber();
 
     if (baselineVersion < 93) {
       result = IntelliJVersion.INTELLIJ_8;

diff --git a/lombok-plugin/src/main/java/de/plushnikov/intellij/plugin/util/PsiClassUtil.java b/lombok-plugin/src/main/java/de/plushnikov/intellij/plugin/util/PsiClassUtil.java
@@ -13,7 +13,6 @@
 import com.intellij.psi.PsiField;
 import com.intellij.psi.PsiMethod;
 import com.intellij.psi.PsiModifier;
-import com.intellij.psi.PsiModifierList;
 import com.intellij.psi.PsiType;
 import com.intellij.psi.PsiTypeParameter;
 import com.intellij.psi.impl.source.PsiExtensibleClass;
@@ -169,21 +168,6 @@ public static PsiType getTypeWithGenerics(@NotNull PsiClass psiClass) {
     }
     return result;
   }
-
-  /**
-   * Return true if class is final.
-   *
-   * @param psiClass
-   * @return
-   */
-  public static boolean isFinalClass(@NotNull PsiClass psiClass) {
-    boolean result = false;
-    final PsiModifierList modifierList = psiClass.getModifierList();
-    if (null != modifierList) {
-      result = modifierList.hasModifierProperty(PsiModifier.FINAL);
-    }
-    return result;
-  }
 
   /**
    * Workaround to get inner class of the psiClass, without calling PsiAugmentProvider infinitely
@@ -195,8 +179,9 @@ public static boolean isFinalClass(@NotNull PsiClass psiClass) {
   public static PsiClass getInnerClassInternByName(@NotNull PsiClass psiClass, @NotNull String className) {
     Collection<PsiClass> innerClasses = collectInnerClassesIntern(psiClass);
     for (PsiClass innerClass : innerClasses) {
-      if (className.equals(innerClass.getName()))
+      if (className.equals(innerClass.getName())) {
         return innerClass;
+      }
     }
     return null;
   }
@@ -205,8 +190,9 @@ public static PsiClass getInnerClassInternByName(@NotNull PsiClass psiClass, @No
   public static PsiClass getInnerClassByName(@NotNull PsiClass psiClass, @NotNull String className) {
     PsiClass[] innerClasses = psiClass.getInnerClasses();
     for (PsiClass innerClass : innerClasses) {
-      if (className.equals(innerClass.getName()))
+      if (className.equals(innerClass.getName())) {
         return innerClass;
+      }
     }
     return null;
   }

diff --git a/lombok-plugin/src/main/java/de/plushnikov/intellij/plugin/util/PsiElementUtil.java b/lombok-plugin/src/main/java/de/plushnikov/intellij/plugin/util/PsiElementUtil.java
@@ -76,53 +76,6 @@ public static boolean methodMatches(
     return true;
   }
 
-  /**
-   * @param method        the method to compare to.
-   * @param returnType    the return type, specify null if any type matches
-   * @param methodName    the name the method should have
-   * @param parameterList the list of the parameters of the method, specify
-   *                      null if any number and type of parameters match
-   * @return true, if the specified method matches the specified constraints,
-   *         false otherwise
-   */
-  public static boolean methodMatches(
-      @NotNull PsiMethod method,
-      @Nullable PsiType returnType,
-      @NonNls @Nullable String methodName,
-      @Nullable PsiParameterList parameterList) {
-    final String name = method.getName();
-    if (methodName != null && !methodName.equals(name)) {
-      return false;
-    }
-    if (parameterList != null) {
-      final PsiParameterList methodParameterList = method.getParameterList();
-      if (methodParameterList.getParametersCount() != parameterList.getParametersCount()) {
-        return false;
-      }
-      final PsiParameter[] methodParameters = methodParameterList.getParameters();
-      final PsiParameter[] otherParameters = parameterList.getParameters();
-      for (int i = 0; i < methodParameters.length; i++) {
-        final PsiType type = methodParameters[i].getType();
-        final PsiType parameterType = otherParameters[i].getType();
-        if (PsiType.NULL.equals(parameterType)) {
-          continue;
-        }
-        if (!typesAreEquivalent(type,
-            parameterType)) {
-          return false;
-        }
-      }
-    }
-    if (returnType != null) {
-      final PsiType methodReturnType = method.getReturnType();
-      if (!typesAreEquivalent(returnType,
-          methodReturnType)) {
-        return false;
-      }
-    }
-    return true;
-  }
-
   public static boolean typesAreEquivalent(
       @Nullable PsiType type1, @Nullable PsiType type2) {
     if (type1 == null) {

diff --git a/lombok-plugin/src/main/java/de/plushnikov/intellij/plugin/util/PsiMethodUtil.java b/lombok-plugin/src/main/java/de/plushnikov/intellij/plugin/util/PsiMethodUtil.java
@@ -1,14 +1,19 @@
 package de.plushnikov.intellij.plugin.util;
 
+import com.intellij.psi.CommonClassNames;
 import com.intellij.psi.JavaPsiFacade;
 import com.intellij.psi.PsiClass;
+import com.intellij.psi.PsiClassType;
 import com.intellij.psi.PsiCodeBlock;
 import com.intellij.psi.PsiElement;
 import com.intellij.psi.PsiElementFactory;
 import com.intellij.psi.PsiManager;
 import com.intellij.psi.PsiMethod;
+import com.intellij.psi.PsiTypeParameter;
+import com.intellij.psi.PsiTypeParameterList;
 import de.plushnikov.intellij.plugin.psi.LombokLightMethod;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
 
 import java.util.Collection;
 
@@ -34,29 +39,54 @@ public static PsiCodeBlock createCodeBlockFromText(@NotNull String blockText, @N
     return elementFactory.createCodeBlockFromText("{" + blockText + "}", psiClass);
   }
 
-  public static boolean hasMethodByName(@NotNull Collection<PsiMethod> classMethods, @NotNull String methodName) {
-    boolean hasMethod = false;
-    for (PsiMethod classMethod : classMethods) {
-      if (classMethod.getName().equals(methodName)) {
-        hasMethod = true;
-        break;
+  @Nullable
+  public static PsiTypeParameterList createTypeParameterList(@NotNull PsiTypeParameterList psiTypeParameterList) {
+    PsiTypeParameter[] psiTypeParameters = psiTypeParameterList.getTypeParameters();
+    if (psiTypeParameters.length > 0) {
+
+      final PsiElementFactory elementFactory = JavaPsiFacade.getInstance(psiTypeParameterList.getProject()).getElementFactory();
+
+      final StringBuilder builder = new StringBuilder("public <");
+
+      for (PsiTypeParameter psiTypeParameter : psiTypeParameters) {
+        builder.append(psiTypeParameter.getName());
+
+        PsiClassType[] superTypes = psiTypeParameter.getExtendsListTypes();
+        if (superTypes.length > 1 || superTypes.length == 1 && !superTypes[0].equalsToText(CommonClassNames.JAVA_LANG_OBJECT)) {
+          builder.append(" extends ");
+          for (PsiClassType type : superTypes) {
+            if (type.equalsToText(CommonClassNames.JAVA_LANG_OBJECT)) {
+              continue;
+            }
+            builder.append(type.getCanonicalText()).append('&');
+          }
+          builder.deleteCharAt(builder.length() - 1);
+        }
+        builder.append(',');
       }
+      builder.deleteCharAt(builder.length() - 1);
+
+      builder.append("> void foo(){}");
+
+      PsiMethod methodFromText = elementFactory.createMethodFromText(builder.toString(), null);
+      return methodFromText.getTypeParameterList();
     }
-    return hasMethod;
+    return null;
   }
 
-  public static boolean hasMethodByName(@NotNull Collection<PsiMethod> classMethods, String... methodNames) {
+
+  public static boolean hasMethodByName(@NotNull Collection<PsiMethod> classMethods, @NotNull String methodName) {
     boolean hasMethod = false;
-    for (String methodName : methodNames) {
-      if (hasMethodByName(classMethods, methodName)) {
+    for (PsiMethod classMethod : classMethods) {
+      if (classMethod.getName().equals(methodName)) {
         hasMethod = true;
         break;
       }
     }
     return hasMethod;
   }
 
-  public static boolean hasMethodByName(@NotNull Collection<PsiMethod> classMethods, @NotNull Collection<String> methodNames) {
+  public static boolean hasMethodByName(@NotNull Collection<PsiMethod> classMethods, String... methodNames) {
     boolean hasMethod = false;
     for (String methodName : methodNames) {
       if (hasMethodByName(classMethods, methodName)) {

diff --git a/lombok-plugin/src/test/data/inspection/delegateOnMethodWithParameter/expected.xml b/lombok-plugin/src/test/data/inspection/delegateOnMethodWithParameter/expected.xml
@@ -0,0 +1,13 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<problems>
+  <problem>
+    <file>Test.java</file>
+    <line>7</line>
+    <description>@Delegate is legal only on no-argument methods.</description>
+  </problem>
+  <problem>
+    <file>Test.java</file>
+    <line>12</line>
+    <description>@Delegate is legal only on no-argument methods.</description>
+  </problem>
+</problems>
diff --git a/lombok-plugin/src/test/data/inspection/delegateOnMethodWithParameter/src/Test.java b/lombok-plugin/src/test/data/inspection/delegateOnMethodWithParameter/src/Test.java
@@ -0,0 +1,18 @@
+import java.lang.Integer;
+
+public class Example<T> {
+  @lombok.Delegate
+  private Integer calcDelegatorInteger() {
+    return 1;
+  }
+
+  @lombok.Delegate
+  private Integer calcDelegatorWithParam(int param1) {
+    return 1;
+  }
+
+  @lombok.Delegate
+  private Integer calcDelegatorWithParams(int param1, String param2) {
+    return 1;
+  }
+}
diff --git a/lombok-plugin/src/test/java/de/plushnikov/lombok/InspectionTest.java b/lombok-plugin/src/test/java/de/plushnikov/lombok/InspectionTest.java
@@ -28,4 +28,8 @@ public void testBuilderRightType() throws Exception {
   public void testDelegateConcreteType() throws Exception {
     doTest();
   }
+
+  public void testDelegateOnMethodWithParameter() throws Exception {
+    doTest();
+  }
 }