Fix: try to set methods accessible before invoking them reflectively

2 files changed, 45 insertions, 56 deletions

diff --git a/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/JavaReflectiveMethodInvoker.java b/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/JavaReflectiveMethodInvoker.java
index 21e3485..c82b7b7 100644
--- a/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/JavaReflectiveMethodInvoker.java
+++ b/plugins/org.eclipse.xtext.ui.common/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/JavaReflectiveMethodInvoker.java
@@ -20,17 +20,16 @@ import org.eclipse.xtext.util.Tuples;
 
 /**
  * Calls methods on a target object reflectively. Caches resolved methods in a map.
- * 
+ *
  * @author Jan Köhnlein - Initial contribution and API
  * @author Michael Clay
- * 
+ *
  */
 public class JavaReflectiveMethodInvoker {
 
-	@SuppressWarnings("unchecked")
-	private final Map<Pair<String, Class>, Method> methodLookupMap = new HashMap<Pair<String, Class>, Method>();
+	private final Map<Pair<String, ? extends Class<?>>, Method> methodLookupMap = new HashMap<Pair<String, ? extends Class<?>>, Method>();
 
-	private Object target;
+	private final Object target;
 
 	public JavaReflectiveMethodInvoker(Object target) {
 		this.target = target;
@@ -44,11 +43,10 @@ public class JavaReflectiveMethodInvoker {
 		return invokeMethod(method, target, parameterValues.toArray(new Object[] {}));
 	}
 
-	@SuppressWarnings("unchecked")
-	private final Method findMethod(Class<?> clazz, String name, Class... paramTypes) {
+	private final Method findMethod(Class<?> clazz, String name, Class<?>... paramTypes) {
 		Assert.isNotNull(clazz, "Class must not be null");
 		Assert.isNotNull(name, "Method name must not be null");
-		Pair<String, Class> methodKey = Tuples.pair(name, (paramTypes == null || paramTypes.length == 0) ? null
+		Pair<String, ? extends Class<?>>  methodKey = Tuples.pair(name, (paramTypes == null || paramTypes.length == 0) ? null
 				: paramTypes[0]);
 		Method result = methodLookupMap.get(methodKey);
 		if (result != null)
@@ -73,45 +71,28 @@ public class JavaReflectiveMethodInvoker {
 
 	@SuppressWarnings("unchecked")
 	private final List<ICompletionProposal> invokeMethod(Method method, Object target, Object... args) {
+		boolean wasAccessible = method.isAccessible();
 		try {
+			method.setAccessible(true);
 			return (List<ICompletionProposal>) method.invoke(target, args);
 		}
-		catch (Exception ex) {
-			handleReflectionException(ex);
-		}
-		throw new IllegalStateException("huh?");
-	}
-
-	private void handleReflectionException(Exception ex) {
-		if (ex instanceof NoSuchMethodException) {
-			throw new IllegalStateException("Method not found: " + ex.getMessage());
-		}
-		if (ex instanceof IllegalAccessException) {
-			throw new IllegalStateException("Could not access method: " + ex.getMessage());
+		catch (IllegalArgumentException e) {
+			throw new IllegalStateException("Illegal argument exception: " + e.getMessage(), e);
 		}
-		if (ex instanceof InvocationTargetException) {
-			rethrowRuntimeException(((InvocationTargetException) ex).getTargetException());
+		catch (IllegalAccessException e) {
+			throw new IllegalStateException("Could not access method: " + e.getMessage(), e);
 		}
-		if (ex instanceof RuntimeException) {
-			throw (RuntimeException) ex;
-		}
-		handleUnexpectedException(ex);
-	}
-
-	private void handleUnexpectedException(Throwable ex) {
-		IllegalStateException isex = new IllegalStateException("Unexpected exception thrown");
-		isex.initCause(ex);
-		throw isex;
-	}
-
-	private final void rethrowRuntimeException(Throwable ex) {
-		if (ex instanceof RuntimeException) {
-			throw (RuntimeException) ex;
+		catch (InvocationTargetException e) {
+			Throwable targetEx = e.getTargetException();
+			if (targetEx instanceof RuntimeException)
+				throw (RuntimeException) targetEx;
+			if (targetEx instanceof Error)
+				throw (Error)targetEx;
+			throw new IllegalStateException("Unexpected exception thrown", targetEx);
 		}
-		if (ex instanceof Error) {
-			throw (Error) ex;
+		finally {
+			method.setAccessible(wasAccessible);
 		}
-		handleUnexpectedException(ex);
 	}
 
 	private boolean equalOrAssignableTypes(Class<?>[] a, Class<?>[] a2) {
diff --git a/tests/org.eclipse.xtext.ui.common.tests/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DynamicDispatchTest.java b/tests/org.eclipse.xtext.ui.common.tests/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DynamicDispatchTest.java
index c94ab33..fa01203 100644
--- a/tests/org.eclipse.xtext.ui.common.tests/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DynamicDispatchTest.java
+++ b/tests/org.eclipse.xtext.ui.common.tests/src/org/eclipse/xtext/ui/common/editor/contentassist/impl/DynamicDispatchTest.java
@@ -1,3 +1,10 @@
+/*******************************************************************************
+ * Copyright (c) 2008 itemis AG (http://www.itemis.eu) and others.
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *******************************************************************************/
 package org.eclipse.xtext.ui.common.editor.contentassist.impl;
 
 import java.util.Arrays;
@@ -14,15 +21,15 @@ import org.eclipse.swt.graphics.Point;
 public class DynamicDispatchTest extends TestCase{
 
 	private static final String FOO = "foo";
-	private static final List<ICompletionProposal> STRING = Arrays.asList(new ICompletionProposal[] {new CompletionProposal()});
-	private static final List<ICompletionProposal> OBJECT = Arrays.asList(new ICompletionProposal[] {new CompletionProposal()});
+	private static final List<ICompletionProposal> STRING = Arrays.<ICompletionProposal>asList(new CompletionProposal());
+	private static final List<ICompletionProposal> OBJECT = Arrays.<ICompletionProposal>asList(new CompletionProposal());
 
-	private static final List<Class<?>> objectType = Arrays.asList(new Class<?>[] { Object.class });
-	private static final List<Class<?>> stringType = Arrays.asList(new Class<?>[] { String.class });
+	private static final List<Class<?>> objectType = Arrays.<Class<?>>asList(Object.class);
+	private static final List<Class<?>> stringType = Arrays.<Class<?>>asList(String.class);
+
+	private static final List<Object> object = Arrays.asList(new Object());
+	private static final List<String> string = Arrays.asList("dummdidumm");
 
-	private static final List<Object> object = Arrays.asList(new Object[] { new Object() });
-	private static final List<String> string = Arrays.asList(new String[] { "dummdidumm" });
-	
 	public void testDynamicDispatch() throws Exception {
 		JavaReflectiveMethodInvoker invoker = new JavaReflectiveMethodInvoker(new Example1());
 		assertEquals(OBJECT, invoker.invoke(FOO, objectType, object));
@@ -31,19 +38,20 @@ public class DynamicDispatchTest extends TestCase{
 		assertEquals(OBJECT, invoker1.invoke(FOO, objectType, object));
 		assertEquals(STRING, invoker1.invoke(FOO, stringType, string));
 	}
-	
-	class Example1 {
-		List<ICompletionProposal> foo(Object o) {
+
+	private class Example1 {
+		@SuppressWarnings("unused")
+		private List<ICompletionProposal> foo(Object o) {
 			return OBJECT;
 		}
-		
-		List<ICompletionProposal> foo(String s) {
+
+		protected List<ICompletionProposal> foo(String s) {
 			return STRING;
 		}
 	}
-	
+
 	class Example2 {
-		List<ICompletionProposal> foo(String s) {
+		public List<ICompletionProposal> foo(String s) {
 			return STRING;
 		}
 
@@ -51,7 +59,7 @@ public class DynamicDispatchTest extends TestCase{
 			return OBJECT;
 		}
 	}
-	
+
 	static class CompletionProposal implements ICompletionProposal {
 
 		public void apply(IDocument document) {
@@ -76,6 +84,6 @@ public class DynamicDispatchTest extends TestCase{
 		public Point getSelection(IDocument document) {
 			return null;
 		}
-		
+
 	}
 }