4397513: (reflect) InvocationHandler.invoke javadoc slightly misleading for "method" parameter #27943
Conversation
…ng for "method" parameter
|
👋 Welcome back liach! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
openjdk
bot
added
csr
Webrevs
|
| * methods declared in {@link Object}, or any public non-static method | ||
| * declared in one of the proxy interfaces or a superinterface of them. | ||
| * See also the {@linkplain Proxy##duplicate-methods "Methods Duplicated in | ||
| * Multiple Proxy Interfaces"} section in {@code Proxy}. |
There was a problem hiding this comment.
The proposed wording looks okay for when a method is invoked on a proxy instance.
InvocationHandler is not strictly tied to Proxy. I'm just wondering if the we should qualify this so that the text is in the context invoking a method on the proxy object. I'm also wondering if it should be moved to the interface description to avoid multi sentence parameter description, e.g. "When invoked on a proxy object, the method ...". Up to you but I suspect there are wider usages of this interface.
There was a problem hiding this comment.
Indeed, I think cglib uses InvocationHandler for their subclass proxies. Since you suggested to move this to the interface, I have started reviewing Proxy, and found some other intriguing properties that I don't know if it best belongs to this effort here. Will describe in another post.
|
I have decided to be a bit more ambitious and rephrased large chunks of specs in Proxy. In particular, I condensed and inlined information about duplicate methods, and reevaluated the specification about instance properties and invocation handler dispatching. I also added API notes for two interesting scenarios I discovered during my writeup, which may be of user caution. interface Baz { int clone(); }
Baz baz = (Baz) Proxy.newProxyInstance(Baz.class.getClassLoader(),
new Class<?>[] { Baz.class },
(_, _, _) -> 42);
baz.clone(); // Returns 42, not a duplicate method with Object::clone
interface Foo extends java.util.concurrent.Callable<String> {
String call(); // covariant override, descriptor changed to ()Ljava/lang/String;
}
Object foo = Proxy.newProxyInstance(Foo.class.getClassLoader(),
new Class<?>[] { Foo.class },
(_, _, _) -> { throw new Exception(); });
((Foo) foo).call(); // Throws UndeclaredThrowableException
((Callable<?>) foo).call(); // Throws Exception - allowed by the bridge method |
| * <li>A proxy class implements exactly the interfaces specified at its | ||
| * creation, in the same order. Invoking {@link Class#getInterfaces() getInterfaces} | ||
| * on its {@code Class} object will return an array containing the same | ||
| * list of interfaces (in the order specified at its creation), invoking |
There was a problem hiding this comment.
Class.getInterfaces is already specified to be ordered, this is redundant.
| * and the following cast operation will succeed (rather than throwing | ||
| * a {@code ClassCastException}): | ||
| * <pre> | ||
| * {@code (Foo) proxy} |
There was a problem hiding this comment.
Duplicate with proxy class properties "A proxy class extends java.lang.reflect.Proxy"
| * {@code java.lang.Object} as its declaring class. In other words, | ||
| * the public, non-final methods of {@code java.lang.Object} | ||
| * logically precede all of the proxy interfaces for the determination of | ||
| * which {@code Method} object to pass to the invocation handler. |
There was a problem hiding this comment.
These 2 paragraphs are inlined into:
One
Methodobject is shared for all duplicate methods of each
name and parameter types pair. It represents the method from the first
class or interface that contains one of these duplicate methods, in the
sequence of theObjectclass followed by the proxy interfaces in
their specified order.
| * all of the exception types returned by invoking | ||
| * {@code getExceptionTypes} on the {@code Method} object | ||
| * passed to the {@code invoke} method can necessarily be thrown | ||
| * successfully by the {@code invoke} method. |
There was a problem hiding this comment.
Inlined, and also added different descriptor trick notes:
If the invoked method is a duplicate method, and the checked
exception is allowed by thethrowsclause of those duplicate
methods with the same method descriptor, it is thrown by the invoked
method. This may be more restrictive than or be unrelated to the
declared exceptions on the passedMethodobject, which may
represent a duplicate method with a different method descriptor.
| * <li>Invocations to default methods in proxy interfaces are also dispatched | ||
| * to the invocation handler. An invocation handler can invoke a default | ||
| * method of a proxy interface by calling {@link | ||
| * InvocationHandler#invokeDefault InvocationHandler::invokeDefault}. |
There was a problem hiding this comment.
These two items might be moved to API notes. Thoughts?
2 participants
The
methodparameter documentation forInvocationHandler::invokedoes not indicate that it may be one of the 3 Object methodshashCode,equals, ortoString. This doc-only improvement clarifies that, links to the Proxy section about declaring class selection, and updates the "the interface method" occurrences to be "the invoked method" to reflect the method may be fromObject.Progress
Issues
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/27943/head:pull/27943$ git checkout pull/27943Update a local copy of the PR:
$ git checkout pull/27943$ git pull https://git.openjdk.org/jdk.git pull/27943/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 27943View PR using the GUI difftool:
$ git pr show -t 27943Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/27943.diff
Using Webrev
Link to Webrev Comment