On Wed, 19 May 2021 16:57:04 GMT, Ian Graves <igra...@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
