On Fri, 17 Dec 2021 17:51:17 GMT, Pavel Rappo <pra...@openjdk.org> wrote:

> Using a custom taglet may result in exceptions, not all of which are javadoc 
> bugs. Some of these exceptions are taglet bugs.
> 
> It was noticed that an exception in a static initializer of a custom taglet 
> looks like a javadoc bug, which it is not. The fix was to trivially catch 
> java.lang.ExceptionInInitializerError and translate it into an error like it 
> is already done for some other exceptions.
> 
> While working on this fix, I noticed that there were no tests that cover 
> various exceptions that could arise while registering a taglet:
> 
>   * ClassNotFoundException
>   * NoSuchMethodException
>   * InstantiationException
>   * InvocationTargetException
> 
> I added a test to cover all of those as well as ExceptionInInitializerError 
> and ClassCastException, which I proactively handled similarly to 
> ExceptionInInitializerError.
> 
> I note that a typical taglet-registration error message is quite poor and 
> should be improved in the future. For example, it is unclear that the method 
> that javadoc cannot find is the nullary (i.e. parameterless or no-arg) 
> MyTaglet constructor:
> 
>     Error - Exception java.lang.NoSuchMethodException thrown while trying to 
> register Taglet MyTaglet...

LGTM, within the spirit of the existing code.

Please file a followup bug for a more comprehensive cleanup to protect against 
errors in user code, like (untrusted) user-provided tablets and doclets.   
While this could be loosely modeled on `javac` `ClientCodeWrapper` and 
`ClientCodeException`, this time round it might be reasonable to use a 
`Proxy`-based wrapper, to catch exceptions in user-code and then throw them in 
a wrapped exception for better handling at a higher level.  That suggests that 
going forward, failure to init a taglet should be a fatal error.  _Note:_ one 
potential complication for `javadoc` not found in `javac` is the hard 
tool-to-doclet abstraction barrier.

test/langtools/jdk/javadoc/doclet/testCustomTagletRegistration/ExceptionInInitializerErrorTaglet.java
 line 36:

> 34: 
> 35:     static {
> 36:         if (true) {

Does the `if (true)` have any significance? 

javac will optimize it, per JLS.

-------------

Marked as reviewed by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk18/pull/44

Reply via email to