Merge pull request #2413 from Rawi01/extensionmethod

Improve ExtensionMethod support in Eclipse

3 files changed, 184 insertions, 37 deletions

diff --git a/src/eclipseAgent/lombok/eclipse/agent/ExtensionMethodCompletionProposal.java b/src/eclipseAgent/lombok/eclipse/agent/ExtensionMethodCompletionProposal.java
index 46ce63f9..833a8226 100644
--- a/src/eclipseAgent/lombok/eclipse/agent/ExtensionMethodCompletionProposal.java
+++ b/src/eclipseAgent/lombok/eclipse/agent/ExtensionMethodCompletionProposal.java
@@ -22,10 +22,15 @@
 package lombok.eclipse.agent;
 
 import static org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants.AccStatic;
+import static org.eclipse.jdt.internal.compiler.classfmt.ClassFileConstants.AccVarargs;
 
 import java.util.Arrays;
 
+import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.jdt.core.CompletionProposal;
+import org.eclipse.jdt.core.IMethod;
+import org.eclipse.jdt.core.JavaModelException;
+import org.eclipse.jdt.core.Signature;
 import org.eclipse.jdt.core.compiler.CharOperation;
 import org.eclipse.jdt.internal.codeassist.CompletionEngine;
 import org.eclipse.jdt.internal.codeassist.InternalCompletionProposal;
@@ -35,16 +40,29 @@ import org.eclipse.jdt.internal.codeassist.complete.CompletionOnSingleNameRefere
 import org.eclipse.jdt.internal.compiler.ast.ASTNode;
 import org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
 import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
+import org.eclipse.jdt.internal.core.NameLookup;
 
 public class ExtensionMethodCompletionProposal extends InternalCompletionProposal {
+	private char[] fullSignature;
+	private char[][] parameterNames;
+	
 	public ExtensionMethodCompletionProposal(final int replacementOffset) {
 		super(CompletionProposal.METHOD_REF, replacementOffset - 1);
 	}
 	
 	public void setMethodBinding(final MethodBinding method, final ASTNode node) {
+		// Add proposal parameter names, sometimes its empty...
+		if (method.parameterNames != null && method.parameterNames.length > 0) {
+			setParameterNames(Arrays.copyOfRange(method.parameterNames, 1, method.parameterNames.length));
+		} else {
+			// Copy signature for parameter name resolution, this is more reliable but slower
+			fullSignature = CompletionEngine.getSignature(method);
+		}
+
 		MethodBinding original = method.original();
 		TypeBinding[] parameters = Arrays.copyOf(method.parameters, method.parameters.length);
 		method.parameters = Arrays.copyOfRange(method.parameters, 1, method.parameters.length);
+
 		TypeBinding[] originalParameters = null;
 		if (original != method) {
 			originalParameters = Arrays.copyOf(method.original().parameters, method.original().parameters.length);
@@ -76,6 +94,10 @@ public class ExtensionMethodCompletionProposal extends InternalCompletionProposa
 		setName(method.selector);
 		setCompletion(completion);
 		setFlags(method.modifiers & (~AccStatic));
+		// Remove varargs flag if it is the only parameter
+		if (method.isVarargs() && length == 0) {
+			setFlags(getFlags() & (~AccVarargs));
+		}
 		int index = node.sourceEnd + 1;
 		if (node instanceof CompletionOnQualifiedNameReference) {
 			index -= ((CompletionOnQualifiedNameReference) node).completionIdentifier.length;
@@ -96,4 +118,58 @@ public class ExtensionMethodCompletionProposal extends InternalCompletionProposa
 			method.original().parameters = originalParameters;
 		}
 	}
+	
+	@Override
+	public char[][] findParameterNames(IProgressMonitor monitor) {
+		if (parameterNames != null) {
+			return parameterNames;
+		}
+		
+		NameLookup.Answer answer = this.nameLookup.findType(
+			new String(this.declarationTypeName),
+			new String(this.declarationPackageName),
+			false,
+			NameLookup.ACCEPT_CLASSES & NameLookup.ACCEPT_INTERFACES,
+			true/* consider secondary types */,
+			false/* do NOT wait for indexes */,
+			false/*don't check restrictions*/,
+			null);
+		
+		if (answer != null && answer.type != null) {
+			char[][] parameterTypes = Signature.getParameterTypes(fullSignature);
+			
+			String[] args = new String[parameterTypes.length];
+			for (int i = 0; i < parameterTypes.length; i++) {
+				args[i] = new String(parameterTypes[i]);
+			}
+			IMethod method = answer.type.getMethod(new String(this.getName()), args);
+			IMethod[] methods = answer.type.findMethods(method);
+			if (methods != null && methods.length > 0) {
+				method = methods[0];
+			}
+			if (method != null) {
+				try {
+					String[] parameterNames = method.getParameterNames();
+					char[][] parameterNamesAsChar = new char[parameterNames.length - 1][];
+					for (int i = 0; i < parameterNamesAsChar.length; i++) {
+						parameterNamesAsChar[i] = parameterNames[i + 1].toCharArray();
+					}
+					setParameterNames(parameterNamesAsChar);
+				} catch (JavaModelException e) {
+					// Nope
+				}
+			}
+		}
+		// Seems like we failed, fallback
+		if (parameterNames == null) {
+			parameterNames = super.findParameterNames(monitor);
+		}
+		return parameterNames;
+	}
+	
+	@Override
+	public void setParameterNames(char[][] parameterNames) {
+		this.parameterNames = parameterNames;
+		super.setParameterNames(parameterNames);
+	}
 }
diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchExtensionMethod.java b/src/eclipseAgent/lombok/eclipse/agent/PatchExtensionMethod.java
index fcc76059..0b33ff0a 100644
--- a/src/eclipseAgent/lombok/eclipse/agent/PatchExtensionMethod.java
+++ b/src/eclipseAgent/lombok/eclipse/agent/PatchExtensionMethod.java
@@ -24,6 +24,7 @@ package lombok.eclipse.agent;
 import static lombok.eclipse.handlers.EclipseHandlerUtil.createAnnotation;
 
 import java.lang.ref.WeakReference;
+import java.lang.reflect.Constructor;
 import java.lang.reflect.Field;
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
@@ -58,6 +59,7 @@ import org.eclipse.jdt.internal.compiler.lookup.Binding;
 import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
 import org.eclipse.jdt.internal.compiler.lookup.CompilationUnitScope;
 import org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
+import org.eclipse.jdt.internal.compiler.lookup.ParameterizedTypeBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ProblemMethodBinding;
 import org.eclipse.jdt.internal.compiler.lookup.ReferenceBinding;
 import org.eclipse.jdt.internal.compiler.lookup.Scope;
@@ -250,6 +252,14 @@ public class PatchExtensionMethod {
 			Binding binding = ((NameReference)methodCall.receiver).binding;
 			if (binding instanceof TypeBinding) skip = true;
 		}
+		// It's impossible to resolve the right method without types
+		if (Reflection.argumentsHaveErrors != null) {
+			try {
+				if ((Boolean) Reflection.argumentsHaveErrors.get(methodCall)) skip = true;
+			} catch (IllegalAccessException ignore) {
+				// ignore
+			}
+		}
 		
 		if (!skip) for (Extension extension : extensions) {
 			if (!extension.suppressBaseMethods && !(methodCall.binding instanceof ProblemMethodBinding)) continue;
@@ -262,13 +272,30 @@ public class PatchExtensionMethod {
 				List<Expression> arguments = new ArrayList<Expression>();
 				arguments.add(methodCall.receiver);
 				if (methodCall.arguments != null) arguments.addAll(Arrays.asList(methodCall.arguments));
+				Expression[] originalArgs = methodCall.arguments;
+				methodCall.arguments = arguments.toArray(new Expression[0]);
+				
 				List<TypeBinding> argumentTypes = new ArrayList<TypeBinding>();
 				for (Expression argument : arguments) {
-					if (argument.resolvedType != null) argumentTypes.add(argument.resolvedType);
-					// TODO: Instead of just skipping nulls entirely, there is probably a 'unresolved type' placeholder. THAT is what we ought to be adding here!
+					TypeBinding argumentType = argument.resolvedType;
+					if (argumentType == null && Reflection.isFunctionalExpression(argument)) {
+						argumentType = Reflection.getPolyTypeBinding(argument);
+					}
+					if (argumentType == null) {
+						argumentType = TypeBinding.NULL;
+					}					
+					argumentTypes.add(argumentType);
 				}
-				Expression[] originalArgs = methodCall.arguments;
-				methodCall.arguments = arguments.toArray(new Expression[0]);
+				
+				// Copy generic information. This one covers a few simple cases, more complex cases are still broken
+				int typeVariables = extensionMethod.typeVariables.length;
+				if (typeVariables > 0 && methodCall.receiver.resolvedType instanceof ParameterizedTypeBinding) {
+					ParameterizedTypeBinding parameterizedTypeBinding = (ParameterizedTypeBinding) methodCall.receiver.resolvedType;
+					if (parameterizedTypeBinding.arguments != null && parameterizedTypeBinding.arguments.length == typeVariables) {
+						methodCall.genericTypeArguments = parameterizedTypeBinding.arguments;
+					}
+				}
+				
 				MethodBinding fixedBinding = scope.getMethod(extensionMethod.declaringClass, methodCall.selector, argumentTypes.toArray(new TypeBinding[0]), methodCall);
 				if (fixedBinding instanceof ProblemMethodBinding) {
 					methodCall.arguments = originalArgs;
@@ -276,18 +303,33 @@ public class PatchExtensionMethod {
 						PostponedInvalidMethodError.invoke(scope.problemReporter(), methodCall, fixedBinding, scope);
 					}
 				} else {
+					// If the extension method uses varargs, the last fixed binding parameter is an array but 
+					// the method arguments are not. Even thought we already know that the method is fine we still
+					// have to compare each parameter with the type of the array to support autoboxing/unboxing.
+					boolean isVarargs = fixedBinding.isVarargs();
 					for (int i = 0, iend = arguments.size(); i < iend; i++) {
 						Expression arg = arguments.get(i);
-						if (fixedBinding.parameters[i].isArrayType() != arg.resolvedType.isArrayType()) break;
-						if (arg instanceof MessageSend) {
-							((MessageSend) arg).valueCast = arg.resolvedType;
+						TypeBinding[] parameters = fixedBinding.parameters;
+						TypeBinding param;
+						if (isVarargs && i >= parameters.length - 1) {
+							// Extract the array element type for all vararg arguments
+							param = parameters[parameters.length - 1].leafComponentType();
+						} else {
+							param = parameters[i];
+						}
+						// Resolve types for lambdas
+						if (Reflection.isFunctionalExpression(arg)) {
+							arg.setExpectedType(param);
+							arg.resolveType(scope);
 						}
-						if (!fixedBinding.parameters[i].isBaseType() && arg.resolvedType.isBaseType()) {
-							int id = arg.resolvedType.id;
-							arg.implicitConversion = TypeIds.BOXING | (id + (id << 4)); // magic see TypeIds
-						} else if (fixedBinding.parameters[i].isBaseType() && !arg.resolvedType.isBaseType()) {
-							int id = fixedBinding.parameters[i].id;
-							arg.implicitConversion = TypeIds.UNBOXING | (id + (id << 4)); // magic see TypeIds
+						if (arg.resolvedType != null) {
+							if (!param.isBaseType() && arg.resolvedType.isBaseType()) {
+								int id = arg.resolvedType.id;
+								arg.implicitConversion = TypeIds.BOXING | (id + (id << 4)); // magic see TypeIds
+							} else if (param.isBaseType() && !arg.resolvedType.isBaseType()) {
+								int id = parameters[i].id;
+								arg.implicitConversion = TypeIds.UNBOXING | (id + (id << 4)); // magic see TypeIds
+							}
 						}
 					}
 					
@@ -295,6 +337,7 @@ public class PatchExtensionMethod {
 					methodCall.actualReceiverType = extensionMethod.declaringClass;
 					methodCall.binding = fixedBinding;
 					methodCall.resolvedType = methodCall.binding.returnType;
+					methodCall.statementEnd = methodCall.sourceEnd;
 					if (Reflection.argumentTypes != null) {
 						try {
 							Reflection.argumentTypes.set(methodCall, argumentTypes.toArray(new TypeBinding[0]));
@@ -340,16 +383,41 @@ public class PatchExtensionMethod {
 	}
 	
 	private static final class Reflection {
-		public static final Field argumentTypes;
+		public static final Field argumentTypes = Permit.permissiveGetField(MessageSend.class, "argumentTypes");
+		public static final Field argumentsHaveErrors = Permit.permissiveGetField(MessageSend.class, "argumentsHaveErrors");
+		private static final Class<?> functionalExpression;
+		private static final Constructor<?> polyTypeBindingConstructor;
 		
 		static {
-			Field a = null;
+			Class<?> a = null;
+			Constructor<?> b = null;
 			try {
-				a = Permit.getField(MessageSend.class, "argumentTypes");
-			}  catch (Throwable t) {
-				//ignore - old eclipse versions don't know this one
+				a = Class.forName("org.eclipse.jdt.internal.compiler.ast.FunctionalExpression");
+			} catch (Exception e) {
+				// Ignore
+			}
+			try {
+				b = Permit.getConstructor(Class.forName("org.eclipse.jdt.internal.compiler.lookup.PolyTypeBinding"), Expression.class);
+			} catch (Exception e) {
+				// Ignore
+			}
+			functionalExpression = a;
+			polyTypeBindingConstructor = b;
+		}
+		
+		public static boolean isFunctionalExpression(Expression expression) {
+			if (functionalExpression == null) return false;
+			return functionalExpression.isInstance(expression);
+		}
+		
+		public static TypeBinding getPolyTypeBinding(Expression expression) {
+			if (polyTypeBindingConstructor == null) return null;
+			try {
+				return (TypeBinding) polyTypeBindingConstructor.newInstance(expression);
+			} catch (Exception e) {
+				// Ignore
 			}
-			argumentTypes = a;
+			return null;
 		}
 	}
 }
diff --git a/src/eclipseAgent/lombok/eclipse/agent/PatchExtensionMethodCompletionProposal.java b/src/eclipseAgent/lombok/eclipse/agent/PatchExtensionMethodCompletionProposal.java
index 085c903f..08a42d1c 100755
--- a/src/eclipseAgent/lombok/eclipse/agent/PatchExtensionMethodCompletionProposal.java
+++ b/src/eclipseAgent/lombok/eclipse/agent/PatchExtensionMethodCompletionProposal.java
@@ -65,13 +65,14 @@ public class PatchExtensionMethodCompletionProposal {
 			CompletionProposalCollector completionProposalCollector) {
 		
 		List<IJavaCompletionProposal> proposals = new ArrayList<IJavaCompletionProposal>(Arrays.asList(javaCompletionProposals));
-		if (canExtendCodeAssist(proposals)) {
-			IJavaCompletionProposal firstProposal = proposals.get(0);
-			int replacementOffset = getReplacementOffset(firstProposal);
+		if (canExtendCodeAssist()) {
 			for (Extension extension : getExtensionMethods(completionProposalCollector)) {
 				for (MethodBinding method : extension.extensionMethods) {
-					ExtensionMethodCompletionProposal newProposal = new ExtensionMethodCompletionProposal(replacementOffset);
-					copyNameLookupAndCompletionEngine(completionProposalCollector, firstProposal, newProposal);
+					if (!isMatchingProposal(method, completionProposalCollector)) {
+						continue;
+					}
+					ExtensionMethodCompletionProposal newProposal = new ExtensionMethodCompletionProposal(0);
+					copyNameLookupAndCompletionEngine(completionProposalCollector, newProposal);
 					ASTNode node = getAssistNode(completionProposalCollector);
 					newProposal.setMethodBinding(method, node);
 					createAndAddJavaCompletionProposal(completionProposalCollector, newProposal, proposals);
@@ -81,7 +82,6 @@ public class PatchExtensionMethodCompletionProposal {
 		return proposals.toArray(new IJavaCompletionProposal[0]);
 	}
 	
-	
 	private static List<Extension> getExtensionMethods(CompletionProposalCollector completionProposalCollector) {
 		List<Extension> extensions = new ArrayList<Extension>();
 		ClassScope classScope = getClassScope(completionProposalCollector);
@@ -96,6 +96,17 @@ public class PatchExtensionMethodCompletionProposal {
 		return extensions;
 	}
 	
+	private static boolean isMatchingProposal(MethodBinding method, CompletionProposalCollector completionProposalCollector) {
+		try {
+			InternalCompletionContext context = (InternalCompletionContext) Reflection.contextField.get(completionProposalCollector);
+			String searchToken = new String(context.getToken());
+			String extensionMethodName = new String(method.selector);
+			return extensionMethodName.contains(searchToken);
+		} catch (IllegalAccessException e) {
+			return true;
+		}
+	}
+	
 	static TypeBinding getFirstParameterType(TypeDeclaration decl, CompletionProposalCollector completionProposalCollector) {
 		TypeBinding firstParameterType = null;
 		ASTNode node = getAssistNode(completionProposalCollector);
@@ -149,8 +160,7 @@ public class PatchExtensionMethodCompletionProposal {
 		return scope;
 	}
 	
-	private static void copyNameLookupAndCompletionEngine(CompletionProposalCollector completionProposalCollector, IJavaCompletionProposal proposal,
-			InternalCompletionProposal newProposal) {
+	private static void copyNameLookupAndCompletionEngine(CompletionProposalCollector completionProposalCollector, InternalCompletionProposal newProposal) {
 		
 		try {
 			InternalCompletionContext context = (InternalCompletionContext) Reflection.contextField.get(completionProposalCollector);
@@ -173,17 +183,10 @@ public class PatchExtensionMethodCompletionProposal {
 		}
 	}
 	
-	private static boolean canExtendCodeAssist(List<IJavaCompletionProposal> proposals) {
-		return !proposals.isEmpty() && Reflection.isComplete();
-	}
-	
-	private static int getReplacementOffset(Object proposal) {
-		try {
-			return Reflection.replacementOffsetField.getInt(proposal);
-		} catch (Exception ignore) {
-			return 0;
-		}
+	private static boolean canExtendCodeAssist() {
+		return Reflection.isComplete();
 	}
+
 	
 	static class Reflection {
 		public static final Field replacementOffsetField;