RFR: 8267329: Modernize Javadoc code to use instanceof with pattern matching [v3]

[Jonathan Gibbons]

On Wed, 19 May 2021 16:57:04 GMT, Ian Graves <igraves at openjdk.org> wrote:

>> 8267329: Modernize Javadoc code to use instanceof with pattern matching
>
> Ian Graves has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Name shortening. Copyright updates.

Thanks ... I think ... with a side of grumble.
It's bad enough people using IDE driven refactoring; it's even worse when there appears to be a manual component of trying to match (often unsuccessfully) the existing style, such as it is.
These module-wide refactorings can and will cause merge conflicts with other ongoing work in progress. In my opinion, this sort of change is better done when the code is being modified for other more legitimate reasons.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/Contents.java line 415:

> 413:                 c.add(con);
> 414:             }
> 415: 

I guess at some point this becomes "pattern switch" or whatever the feature is called ... but not yet

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 1127:

> 1125:                 // inherits it automatically.
> 1126:                 if (this instanceof ClassWriterImpl cwrtr) {
> 1127:                     containing = cwrtr.getTypeElement();

suggest the name `writer` not `cwrtr`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/HtmlDocletWriter.java line 2132:

> 2130:         List<DocPath> stylesheets = new ArrayList<>();
> 2131:         DocPath basePath = null;
> 2132:         if (element instanceof PackageElement pkgElem) {

Suggest `pkg`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/LinkOutputImpl.java line 54:

> 52:     @Override
> 53:     public void append(Object o) {
> 54:         output.append(o.toString());

There may be a change in behavior here, for "invalid" arguments, which will now use `.toString()` instead of throwing CCE.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/SourceToHTMLConverter.java line 178:

> 176:         }
> 177:         for (Element elem : mdl.getEnclosedElements()) {
> 178:             if (elem instanceof PackageElement pkgElem && configuration.docEnv.isIncluded(elem)

Use `pkg`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/TagletWriterImpl.java line 323:

> 321:         }
> 322:         if (utils.isVariableElement(holder) && ((VariableElement)holder).getConstantValue() != null &&
> 323:                 htmlWriter instanceof ClassWriterImpl clsHtml) {

Use `writer`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/ContentBuilder.java line 55:

> 53:         Objects.requireNonNull(content);
> 54:         ensureMutableContents();
> 55:         if (content instanceof ContentBuilder conBldr) {

use `b` or `cb`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/ContentBuilder.java line 72:

> 70:             } else {
> 71:                 contents.add(tb = new TextBuilder());
> 72:             }

Restructure this

contents.add((c instanceof TextBuilder tb) ? tb : new TextBuilder());

or something similar

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/markup/HtmlTree.java line 177:

> 175:     @Override
> 176:     public HtmlTree add(Content content) {
> 177:         if (content instanceof ContentBuilder conBldr) {

Use `cb`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/builders/MemberSummaryBuilder.java line 319:

> 317:             }
> 318:             List<? extends DocTree> propertyTags = utils.getBlockTags(property,
> 319:                     t -> (t instanceof UnknownBlockTagTree ukbt)

Use `tree`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/builders/MemberSummaryBuilder.java line 334:

> 332: 
> 333:         List<? extends DocTree> bTags = utils.getBlockTags(property,
> 334:                 t -> (t instanceof UnknownBlockTagTree ukbt)

Use `tree`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java line 399:

> 397:     private Item findElementItem(Element element) {
> 398:         Item item = null;
> 399:         if (element instanceof ModuleElement modElem) {

Use `mdle` or `me`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Extern.java line 402:

> 400:             item = moduleItems.get(utils.getModuleName(modElem));
> 401:         }
> 402:         else if (element instanceof PackageElement pkgElem) {

Use `pkg` or `pe`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 2335:

> 2333:             if (val == null)
> 2334:                 return null;
> 2335:             else if (val instanceof String str)

Use `s`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java line 2637:

> 2635:                 return bt.accepts(t);
> 2636:             } else if (t instanceof UnknownBlockTagTree ukbt) {
> 2637:                 return ukbt.getTagName().equals(taglet.getName());

Use `tree` 

`ukbt` is not even an acronym, unless you spell it Un Known Blocktag Tree!

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclint/Checker.java line 594:

> 592: 
> 593:     void warnIfEmpty(TagStackItem tsi, DocTree endTree) {
> 594:         if (tsi.tag != null && tsi.tree instanceof StartElementTree sElemTree) {

Use `tree`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Messager.java line 69:

> 67:     public static Messager instance0(Context context) {
> 68:         Log instance = context.get(logKey);
> 69:         if (!(instance instanceof Messager msgr))

Use `m`

src/jdk.javadoc/share/classes/jdk/javadoc/internal/tool/Start.java line 143:

> 141: 
> 142:         Log log = context.get(Log.logKey);
> 143:         if (log instanceof Messager mLog){

Use `m`

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

Changes requested by jjg (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4105