RFR : JDK-8219632: Remove reference to com.sun.javadoc API in RemoveOldDoclet test

[Jonathan Gibbons]

This note is for the record, since I just spent a bunch more time 
understanding this change.

Here is the net change in Start.java, from changeset 53884 (before the 
initial fix in thie work to remove the old doclet) to the proposed 
changeset.

$ diff Start-before.java 
src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java
411,427c411,412
<             if (apiMode) {
<                 com.sun.tools.javadoc.main.Start ostart
<                         = new com.sun.tools.javadoc.main.Start(context);
<                 return ostart.begin(docletClass, options, fileObjects)
<                         ? OK
<                         : ERROR;
<             }
<             warn("main.legacy_api");
<             String[] array = options.toArray(new String[options.size()]);
<             int rc = com.sun.tools.javadoc.Main.execute(
<                     messager.programName,
<                     messager.getWriter(WriterKind.ERROR),
<                     messager.getWriter(WriterKind.WARNING),
<                     messager.getWriter(WriterKind.NOTICE),
<                     docletClass.getName(),
<                     array);
<             return (rc == 0) ? OK : ERROR;
---
 >             error("main.not_a_doclet", docletClass.getName());
 >             return ERROR;

It's notable, and confusing, that this change is *only* about handling 
the case when a Doclet class is passed in through the API, and the code 
was previously trying to disambiguate between old-style doclets and 
new-style doclets.  In hindsight, it is a regrttable, but understandable 
decision to have allowed a Doclet class to be passed in, instead of a 
Doclet instance -- and the roots of that decision go all the way back to 
original javadoc.

The net result is that instantiating doclets is still a bit of a mess. 
See for example, the almost-duplicated code between 750-759 and 771-781. 
And in both of those places, we just instantiate the class, without 
trying to create an instance, which is done in lines 400-409.

It would be better to refactor the code so that Start.preprocess returns 
a Doclet, not a Class<?>, meaning that the instantiation should be moved 
from Start.begin into Start.preprocess.

-- Jon



On 03/08/2019 01:35 PM, Jonathan Gibbons wrote:
>
> Priya,
>
> In the test, you use a literal string for the test class name. A 
> better style is to use the name of the .class literal, as in
>
> 46 static final String Doclet_CLASS_NAME = TestDoclet.class.getName();
> Fix that, and you're good to go.
>
> -- Jon
>
> On 02/25/2019 03:48 AM, Priya Lakshmi Muthuswamy wrote:
>> Hi,
>>
>> Kindly review the fix for 
>> https://bugs.openjdk.java.net/browse/JDK-8219632
>> webrev : http://cr.openjdk.java.net/~pmuthuswamy/8219632/webrev.00/
>>
>> jdk/javadoc/tool/removeOldDoclet/RemoveOldDoclet.java tries to use 
>> OldDoclet.jasm which hold reference to removed API com.sun.javadoc.
>> As part of the fix for 
>> JDK-8219575(jdk/javadoc/tool/removeOldDoclet/RemoveOldDoclet test 
>> fails in mach5),
>> tried to handle the NoDefClassFoundError.
>>
>> This fix removes the reference to com.sun.javadoc in test class and 
>> the handling of NoClassDefFoundError.
>> If user creates any Doclet which doesn't implement 
>> jdk.javadoc.doclet.Doclet, there will be error stating that its not a 
>> valid Doclet.
>>
>> Thanks,
>> Priya
>>
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/javadoc-dev/attachments/20190308/47ac67c5/attachment.html>