Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8267329: Modernize Javadoc code to use instanceof with pattern matching #4105

Closed
Closed
@@ -113,15 +113,15 @@ public DocumentationTask getTask(

if (out == null)
context.put(Log.errKey, new PrintWriter(System.err, true));
else if (out instanceof PrintWriter)
context.put(Log.errKey, ((PrintWriter) out));
else if (out instanceof PrintWriter printWriter)
context.put(Log.errKey, printWriter);
else
context.put(Log.errKey, new PrintWriter(out, true));

if (fileManager == null) {
fileManager = getStandardFileManager(diagnosticListener, null, null);
if (fileManager instanceof BaseFileManager) {
((BaseFileManager) fileManager).autoClose = true;
if (fileManager instanceof BaseFileManager baseFileManager) {
baseFileManager.autoClose = true;
}
}
fileManager = ccw.wrap(fileManager);
@@ -470,9 +470,9 @@ private Content getClassLinks(HtmlLinkInfo.Kind context, Collection<?> list) {
isFirst = false;
}
// TODO: should we simply split this method up to avoid instanceof ?
if (type instanceof TypeElement) {
if (type instanceof TypeElement typeElement) {
Content link = getLink(
new HtmlLinkInfo(configuration, context, (TypeElement)(type)));
new HtmlLinkInfo(configuration, context, typeElement));
content.add(HtmlTree.CODE(link));
} else {
Content link = getLink(
@@ -407,10 +407,10 @@ public Content getContent(String key, Object o0, Object o1, Object o2) {

if (o == null) {
c.add("{" + m.group(1) + "}");
} else if (o instanceof String) {
c.add((String) o);
} else if (o instanceof Content) {
c.add((Content) o);
} else if (o instanceof String str) {
c.add(str);
} else if (o instanceof Content con) {
c.add(con);
}

Comment on lines 408 to 415

This comment has been minimized.

@jonathan-gibbons

jonathan-gibbons May 19, 2021
Contributor

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

This comment has been minimized.

@pavelrappo

pavelrappo May 24, 2021
Member

The feature you are talking about is called "Pattern Matching for switch". The feature is described as a preview feature by JEP 406, which is currently proposed to target JDK 17.

start = m.end();
@@ -354,8 +354,7 @@ public Locale getLocale() {
@Override
public JavaFileObject getOverviewPath() {
String overviewpath = options.overviewPath();
if (overviewpath != null && getFileManager() instanceof StandardJavaFileManager) {
StandardJavaFileManager fm = (StandardJavaFileManager) getFileManager();
if (overviewpath != null && getFileManager() instanceof StandardJavaFileManager fm) {
return fm.getJavaFileObjects(overviewpath).iterator().next();
}
return null;
@@ -1123,8 +1123,8 @@ public Content seeTagToContent(Element element, DocTree see, TagletWriterImpl.Co
// documented, this must be an inherited link. Redirect it.
// The current class either overrides the referenced member or
// inherits it automatically.
if (this instanceof ClassWriterImpl) {
containing = ((ClassWriterImpl) this).getTypeElement();
if (this instanceof ClassWriterImpl classWriterImpl) {
containing = (classWriterImpl.getTypeElement();
} else if (!utils.isPublic(containing)) {
messages.warning(
ch.getDocTreePath(see), "doclet.see.class_or_package_not_accessible",
@@ -2129,11 +2129,11 @@ Script getMainBodyScript() {
List<DocPath> getLocalStylesheets(Element element) throws DocFileIOException {
List<DocPath> stylesheets = new ArrayList<>();
DocPath basePath = null;
if (element instanceof PackageElement) {
stylesheets.addAll(getModuleStylesheets((PackageElement)element));
basePath = docPaths.forPackage((PackageElement)element);
} else if (element instanceof ModuleElement) {
basePath = DocPaths.forModule((ModuleElement)element);
if (element instanceof PackageElement packageElement) {
stylesheets.addAll(getModuleStylesheets(packageElement));
basePath = docPaths.forPackage(packageElement);
} else if (element instanceof ModuleElement moduleElement) {
basePath = DocPaths.forModule(moduleElement);
}
for (DocPath stylesheet : getStylesheets(element)) {
stylesheets.add(basePath.resolve(stylesheet.getPath()));
@@ -51,8 +51,8 @@ public LinkOutputImpl() {

@Override
public void append(Object o) {
output.append(o instanceof String ?
(String) o : ((LinkOutputImpl)o).toString());
output.append(o instanceof String str ?
str : o.toString());
Comment on lines +54 to +55

This comment has been minimized.

@pavelrappo

pavelrappo May 19, 2021
Member Outdated

A briefer equivalent would look like this:

output.append(o.toString);
}

@Override
@@ -175,9 +175,9 @@ public void convertModule(ModuleElement mdl, DocPath outputdir)
return;
}
for (Element elem : mdl.getEnclosedElements()) {
if (elem instanceof PackageElement && configuration.docEnv.isIncluded(elem)
if (elem instanceof PackageElement packageElement && configuration.docEnv.isIncluded(elem)
&& !(options.noDeprecated() && utils.isDeprecated(elem))) {
convertPackage((PackageElement) elem, outputdir);
convertPackage(packageElement, outputdir);
}
}
}
@@ -155,8 +155,7 @@ private Content createLink(IndexItem i) {
if (element instanceof OverviewElement) {
return links.createLink(pathToRoot.resolve(i.getUrl()),
resources.getText("doclet.Overview"));
} else if (element instanceof DocletElement) {
DocletElement e = (DocletElement) element;
} else if (element instanceof DocletElement e) {
// Implementations of DocletElement do not override equals and
// hashCode; putting instances of DocletElement in a map is not
// incorrect, but might well be inefficient
@@ -320,12 +320,12 @@ public Content seeTagOutput(Element holder, List<? extends SeeTree> seeTags) {
links.add(htmlWriter.seeTagToContent(holder, dt, context.within(dt)));
}
if (utils.isVariableElement(holder) && ((VariableElement)holder).getConstantValue() != null &&
htmlWriter instanceof ClassWriterImpl) {
htmlWriter instanceof ClassWriterImpl classHtmlWriter) {
//Automatically add link to constant values page for constant fields.
DocPath constantsPath =
htmlWriter.pathToRoot.resolve(DocPaths.CONSTANT_VALUES);
String whichConstant =
((ClassWriterImpl) htmlWriter).getTypeElement().getQualifiedName() + "." +
classHtmlWriter.getTypeElement().getQualifiedName() + "." +
utils.getSimpleName(holder);
DocLink link = constantsPath.fragment(whichConstant);
links.add(htmlWriter.links.createLink(link,
@@ -506,8 +506,7 @@ public String visitVariable(VariableElement e, Void p) {

@Override
public String visitUnknown(Element e, Void p) {
if (e instanceof DocletElement) {
DocletElement de = (DocletElement) e;
if (e instanceof DocletElement de) {
return switch (de.getSubKind()) {
case OVERVIEW -> resources.getText("doclet.Overview");
case DOCFILE -> getHolderName(de);
@@ -52,8 +52,8 @@ public ContentBuilder(Content... contents) {
public ContentBuilder add(Content content) {
Objects.requireNonNull(content);
ensureMutableContents();
if (content instanceof ContentBuilder) {
contents.addAll(((ContentBuilder) content).contents);
if (content instanceof ContentBuilder contentBuilder) {
contents.addAll(contentBuilder.contents);
} else
contents.add(content);
return this;
@@ -65,8 +65,8 @@ public ContentBuilder add(CharSequence text) {
ensureMutableContents();
Content c = contents.isEmpty() ? null : contents.get(contents.size() - 1);
TextBuilder tb;
if (c != null && c instanceof TextBuilder) {
tb = (TextBuilder) c;
if (c instanceof TextBuilder textBuilder) {
tb = textBuilder;
} else {
contents.add(tb = new TextBuilder());
}
@@ -174,8 +174,8 @@ public HtmlTree addStyle(String style) {
*/
@Override
public HtmlTree add(Content content) {
if (content instanceof ContentBuilder) {
((ContentBuilder) content).contents.forEach(this::add);
if (content instanceof ContentBuilder contentBuilder) {
(contentBuilder.contents.forEach(this::add);
}
else if (content == HtmlTree.EMPTY || content.isValid()) {
// quietly avoid adding empty or invalid nodes (except EMPTY)
@@ -420,8 +420,7 @@ public DocCommentInfo getHtmlCommentInfo(Element e) {
PackageElement pe = null;
switch (e.getKind()) {
case OTHER:
if (e instanceof DocletElement) {
DocletElement de = (DocletElement)e;
if (e instanceof DocletElement de) {
fo = de.getFileObject();
pe = de.getPackageElement();
}
@@ -316,8 +316,8 @@ private void processProperty(Element member,
fullBody.addAll(cmtutils.makeFirstSentenceTree(text));
}
List<? extends DocTree> propertyTags = utils.getBlockTags(property,
t -> (t instanceof UnknownBlockTagTree)
&& ((UnknownBlockTagTree) t).getTagName().equals("propertyDescription"));
t -> (t instanceof UnknownBlockTagTree unknownBlockTagTree)
&& (unknownBlockTagTree.getTagName().equals("propertyDescription"));
if (propertyTags.isEmpty()) {
List<? extends DocTree> comment = utils.getFullBody(property);
blockTags.addAll(cmtutils.makePropertyDescriptionTree(comment));
@@ -331,8 +331,8 @@ private void processProperty(Element member,
blockTags.addAll(tags);

List<? extends DocTree> bTags = utils.getBlockTags(property,
t -> (t instanceof UnknownBlockTagTree)
&& ((UnknownBlockTagTree) t).getTagName().equals("defaultValue"));
t -> (t instanceof UnknownBlockTagTree unknownBlockTagTree)
&& (unknownBlockTagTree.getTagName().equals("defaultValue"));
blockTags.addAll(bTags);

//add @see tags
@@ -223,8 +223,7 @@ public TagletManager(BaseConfiguration configuration) {
* @throws IOException if an error occurs while setting the location
*/
public void initTagletPath(JavaFileManager fileManager) throws IOException {
if (fileManager instanceof StandardJavaFileManager) {
StandardJavaFileManager sfm = (StandardJavaFileManager)fileManager;
if (fileManager instanceof StandardJavaFileManager sfm) {
if (tagletPath != null) {
List<File> paths = new ArrayList<>();
for (String pathname : tagletPath.split(File.pathSeparator)) {
@@ -553,8 +552,7 @@ private void printTagMisuseWarn(CommentHelper ch, Taglet taglet, DocTree tag, St
case PACKAGE:
return blockTagletsByLocation.get(Location.PACKAGE);
case OTHER:
if (e instanceof DocletElement) {
DocletElement de = (DocletElement) e;
if (e instanceof DocletElement de) {
switch (de.getSubKind()) {
case DOCFILE:
return blockTagletsByLocation.get(Location.PACKAGE);
@@ -753,7 +751,7 @@ private void showTaglets(PrintStream out) {
+ format(t.inMethod(), "method") + " "
+ format(t.inField(), "field") + " "
+ format(t.isInlineTag(), "inline")+ " "
+ format((t instanceof SimpleTaglet) && !((SimpleTaglet) t).enabled, "disabled"));
+ format((t instanceof SimpleTaglet st) && !st.enabled, "disabled"));
});
}

@@ -263,8 +263,7 @@ public Content getBlockTagOutput(TagletManager tagletManager,
continue;
}

if (element.getKind() == ElementKind.MODULE && taglet instanceof BaseTaglet) {
BaseTaglet t = (BaseTaglet) taglet;
if (element.getKind() == ElementKind.MODULE && taglet instanceof BaseTaglet t) {
switch (t.getTagKind()) {
// @uses and @provides are handled separately, so skip here.
// See ModuleWriterImpl.computeModulesData
@@ -74,8 +74,8 @@ public void inherit(DocFinder.Input input, DocFinder.Output output) {
Element exception;
CommentHelper ch = utils.getCommentHelper(input.element);
if (input.tagId == null) {
exception = input.docTreeInfo.docTree instanceof ThrowsTree
? ch.getException((ThrowsTree) input.docTreeInfo.docTree) : null;
exception = input.docTreeInfo.docTree instanceof ThrowsTree throwsTree
? ch.getException(throwsTree) : null;
input.tagId = exception == null
? ch.getExceptionName(input.docTreeInfo.docTree).getSignature()
: utils.getFullyQualifiedName(exception);
@@ -63,7 +63,7 @@ protected DocPath(String p) {

@Override
public boolean equals(Object other) {
return (other instanceof DocPath) && path.equals(((DocPath)other).path);
return (other instanceof DocPath docPath) && path.equals(docPath.path);
}

@Override
@@ -396,11 +396,10 @@ private boolean link(String url, String elemlisturl, Reporter reporter, boolean
*/
private Item findElementItem(Element element) {
Item item = null;
if (element instanceof ModuleElement) {
item = moduleItems.get(utils.getModuleName((ModuleElement)element));
if (element instanceof ModuleElement moduleElement) {
item = moduleItems.get(utils.getModuleName(moduleElement));
}
else if (element instanceof PackageElement) {
PackageElement packageElement = (PackageElement)element;
else if (element instanceof PackageElement packageElement) {
ModuleElement moduleElement = utils.containingModule(packageElement);
Map<String, Item> pkgMap = packageItems.get(utils.getModuleName(moduleElement));
item = (pkgMap != null) ? pkgMap.get(utils.getPackageName(packageElement)) : null;
@@ -624,8 +623,7 @@ private InputStream open(URL url) throws IOException {
in = conn.getInputStream();
redir = false;

if (conn instanceof HttpURLConnection) {
HttpURLConnection http = (HttpURLConnection)conn;
if (conn instanceof HttpURLConnection http) {
int stat = http.getResponseCode();
// See:
// https://developer.mozilla.org/en-US/docs/Web/HTTP/Status
@@ -1736,11 +1736,11 @@ public int compare(String s1, String s2) {

private Collator createCollator(Locale locale) {
Collator baseCollator = Collator.getInstance(locale);
if (baseCollator instanceof RuleBasedCollator) {
if (baseCollator instanceof RuleBasedCollator ruleBaseCollator) {
// Extend collator to sort signatures with additional args and var-args in a well-defined order:
// () < (int) < (int, int) < (int...)
try {
return new RuleBasedCollator(((RuleBasedCollator) baseCollator).getRules()
return new RuleBasedCollator(ruleBaseCollator.getRules()
+ "& ')' < ',' < '.','['");
} catch (ParseException e) {
throw new RuntimeException(e);
@@ -2332,8 +2332,8 @@ public String visitPrimitiveAsLong(PrimitiveType t, Object val) {
protected String defaultAction(TypeMirror e, Object val) {
if (val == null)
return null;
else if (val instanceof String)
return sourceForm((String) val);
else if (val instanceof String str)
This conversation was marked as resolved by igraves

This comment has been minimized.

@jonathan-gibbons

jonathan-gibbons May 19, 2021
Contributor Outdated

Use s

return sourceForm(str);
return val.toString(); // covers int, short
}

@@ -2631,10 +2631,10 @@ public void removeCommentHelper(Element element) {

public List<? extends DocTree> getBlockTags(Element element, Taglet taglet) {
return getBlockTags(element, t -> {
if (taglet instanceof BaseTaglet) {
return ((BaseTaglet) taglet).accepts(t);
} else if (t instanceof UnknownBlockTagTree) {
return ((UnknownBlockTagTree) t).getTagName().equals(taglet.getName());
if (taglet instanceof BaseTaglet baseTaglet) {
return baseTaglet.accepts(t);
} else if (t instanceof UnknownBlockTagTree unknownBlockTagTree) {
return unknownBlockTagTree.getTagName().equals(taglet.getName());
} else {
return false;
}
@@ -591,14 +591,14 @@ public Void visitEndElement(EndElementTree tree, Void ignore) {
}

void warnIfEmpty(TagStackItem tsi, DocTree endTree) {
if (tsi.tag != null && tsi.tree instanceof StartElementTree) {
if (tsi.tag != null && tsi.tree instanceof StartElementTree startElementTree) {
if (tsi.tag.flags.contains(HtmlTag.Flag.EXPECT_CONTENT)
&& !tsi.flags.contains(Flag.HAS_TEXT)
&& !tsi.flags.contains(Flag.HAS_ELEMENT)
&& !tsi.flags.contains(Flag.HAS_INLINE_TAG)
&& !(tsi.tag.elemKind == ElemKind.HTML4)) {
DocTree tree = (endTree != null) ? endTree : tsi.tree;
Name treeName = ((StartElementTree) tsi.tree).getName();
DocTree tree = (endTree != null) ? endTree : startElementTree;
Name treeName = startElementTree.getName();
env.messages.warning(HTML, tree, "dc.tag.empty", treeName);
}
}
@@ -1162,8 +1162,7 @@ boolean hasModule() {

@Override
public boolean equals(Object obj) {
if (obj instanceof ModulePackage) {
ModulePackage that = (ModulePackage)obj;
if (obj instanceof ModulePackage that) {
return this.toString().equals(that.toString());
}
return false;
@@ -158,8 +158,8 @@ public DocletEnvironment getEnvironment(ToolOptions toolOptions,
ListBuffer<JCCompilationUnit> classTrees = new ListBuffer<>();

try {
StandardJavaFileManager fm = toolEnv.fileManager instanceof StandardJavaFileManager
? (StandardJavaFileManager) toolEnv.fileManager
StandardJavaFileManager fm = toolEnv.fileManager instanceof StandardJavaFileManager standardJavaFileManager
? standardJavaFileManager
: null;
Set<String> packageNames = new LinkedHashSet<>();
// Normally, the args should be a series of package names or file names.
@@ -66,9 +66,9 @@
/** Get the current messager, which is also the compiler log. */
public static Messager instance0(Context context) {
Log instance = context.get(logKey);
if (instance == null || !(instance instanceof Messager))
if (!(instance instanceof Messager messenger))
throw new InternalError("no messager instance!");
return (Messager)instance;
return messenger;
Comment on lines -69 to +71

This comment has been minimized.

@pavelrappo

pavelrappo May 19, 2021
Member Outdated

Every project has quirks. Out of many quirks of JavaDoc, we love this one the most. Perhaps you misread that type's name: it's Messager [sic!]. Please rename the variable either to messager or to a shorter one.

This comment has been minimized.

@jonathan-gibbons

jonathan-gibbons May 19, 2021
Contributor Outdated

@pavelrappo let's consider changing this name sometime (not now)

}

public static void preRegister(Context context,