Skip to content
Permalink
Browse files
8273544: Increase test coverage for snippets
Reviewed-by: jjg
  • Loading branch information
pavelrappo committed Nov 19, 2021
1 parent 2d4af22 commit 2ab43ec2428edaebfe9a7fb0e716ff7141a28de0
Show file tree
Hide file tree
Showing 12 changed files with 1,233 additions and 289 deletions.
@@ -429,7 +429,7 @@ protected Content snippetTagOutput(Element element, SnippetTree tag, StyledText
String strippedLine = line.strip();
int idx = line.indexOf(strippedLine);
assert idx >= 0; // because the stripped line is a substring of the line being stripped
Text whitespace = Text.of(line.substring(0, idx));
Text whitespace = Text.of(utils.normalizeNewlines(line.substring(0, idx)));
// If the leading whitespace is not excluded from the link,
// browsers might exhibit unwanted behavior. For example, a
// browser might display hand-click cursor while user hovers
@@ -438,7 +438,7 @@ protected Content snippetTagOutput(Element element, SnippetTree tag, StyledText
c = new ContentBuilder(whitespace, htmlWriter.linkToContent(element, e, t, strippedLine));
// We don't care about trailing whitespace.
} else {
c = HtmlTree.SPAN(Text.of(sequence));
c = HtmlTree.SPAN(Text.of(utils.normalizeNewlines(sequence)));
classes.forEach(((HtmlTree) c)::addStyle);
}
code.add(c);
@@ -361,6 +361,9 @@ doclet.snippet.region.not_found=\
doclet.tag.attribute.value.illegal=\
illegal value for attribute "{0}": "{1}"

doclet.tag.attribute.value.missing=\
missing value for attribute "{0}"

doclet.tag.attribute.repeated=\
repeated attribute: "{0}"

@@ -28,7 +28,6 @@
import java.io.IOException;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
@@ -84,6 +83,45 @@ public SnippetTaglet() {
*/
@Override
public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writer) {
try {
return generateContent(holder, tag, writer);
} catch (BadSnippetException e) {
error(writer, holder, e.tag(), e.key(), e.args());
return badSnippet(writer);
}
}

private static final class BadSnippetException extends Exception {

@java.io.Serial
private static final long serialVersionUID = 1;

private final transient DocTree tag;
private final String key;
private final transient Object[] args;

BadSnippetException(DocTree tag, String key, Object... args) {
this.tag = tag;
this.key = key;
this.args = args;
}

DocTree tag() {
return tag;
}

String key() {
return key;
}

Object[] args() {
return args;
}
}

private Content generateContent(Element holder, DocTree tag, TagletWriter writer)
throws BadSnippetException
{
SnippetTree snippetTag = (SnippetTree) tag;

// organize snippet attributes in a map, performing basic checks along the way
@@ -98,9 +136,8 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ
// two like-named attributes found; although we report on the most
// recently encountered of the two, the iteration order might differ
// from the source order (see JDK-8266826)
error(writer, holder, a, "doclet.tag.attribute.repeated",
a.getName().toString());
return badSnippet(writer);
throw new BadSnippetException(a, "doclet.tag.attribute.repeated",
a.getName().toString());
}

final String CLASS = "class";
@@ -111,22 +148,19 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ
final boolean containsBody = snippetTag.getBody() != null;

if (containsClass && containsFile) {
error(writer, holder, attributes.get(CLASS),
"doclet.snippet.contents.ambiguity.external");
return badSnippet(writer);
throw new BadSnippetException(attributes.get(CLASS),
"doclet.snippet.contents.ambiguity.external");
} else if (!containsClass && !containsFile && !containsBody) {
error(writer, holder, tag, "doclet.snippet.contents.none");
return badSnippet(writer);
throw new BadSnippetException(tag, "doclet.snippet.contents.none");
}

String regionName = null;
AttributeTree region = attributes.get("region");
if (region != null) {
regionName = stringOf(region.getValue());
regionName = stringValueOf(region);
if (regionName.isBlank()) {
error(writer, holder, region, "doclet.tag.attribute.value.illegal",
"region", region.getValue());
return badSnippet(writer);
throw new BadSnippetException(region, "doclet.tag.attribute.value.illegal",
"region", region.getValue());
}
}

@@ -141,12 +175,12 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ
if (containsFile || containsClass) {
AttributeTree a;
String v = containsFile
? stringOf((a = attributes.get(FILE)).getValue())
: stringOf((a = attributes.get(CLASS)).getValue()).replace(".", "/") + ".java";
? stringValueOf((a = attributes.get(FILE)))
: stringValueOf((a = attributes.get(CLASS))).replace(".", "/") + ".java";

if (v.isBlank()) {
error(writer, holder, a, "doclet.tag.attribute.value.illegal",
containsFile ? FILE : CLASS, v);
throw new BadSnippetException(a, "doclet.tag.attribute.value.illegal",
containsFile ? FILE : CLASS, v);
}

// we didn't create JavaFileManager, so we won't close it; even if an error occurs
@@ -165,24 +199,21 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ
if (fileObject == null && fileManager.hasLocation(Location.SNIPPET_PATH)) {
fileObject = fileManager.getFileForInput(Location.SNIPPET_PATH, "", v);
}
} catch (IOException | IllegalArgumentException e) {
} catch (IOException | IllegalArgumentException e) { // TODO: test this when JDK-8276892 is integrated
// JavaFileManager.getFileForInput can throw IllegalArgumentException in certain cases
error(writer, holder, a, "doclet.exception.read.file", v, e.getCause());
return badSnippet(writer);
throw new BadSnippetException(a, "doclet.exception.read.file", v, e.getCause());
}

if (fileObject == null) {
// i.e. the file does not exist
error(writer, holder, a, "doclet.File_not_found", v);
return badSnippet(writer);
throw new BadSnippetException(a, "doclet.File_not_found", v);
}

try {
externalContent = fileObject.getCharContent(true).toString();
} catch (IOException e) {
error(writer, holder, a, "doclet.exception.read.file",
fileObject.getName(), e.getCause());
return badSnippet(writer);
} catch (IOException e) { // TODO: test this when JDK-8276892 is integrated
throw new BadSnippetException(a, "doclet.exception.read.file",
fileObject.getName(), e.getCause());
}
}

@@ -197,12 +228,12 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ
}
} catch (ParseException e) {
var path = writer.configuration().utils.getCommentHelper(holder)
.getDocTreePath(snippetTag.getBody());
.getDocTreePath(snippetTag.getBody());
// TODO: there should be a method in Messages; that method should mirror Reporter's; use that method instead accessing Reporter.
String msg = writer.configuration().getDocResources()
.getText("doclet.snippet.markup", e.getMessage());
.getText("doclet.snippet.markup", e.getMessage());
writer.configuration().getReporter().print(Diagnostic.Kind.ERROR,
path, e.getPosition(), e.getPosition(), e.getPosition(), msg);
path, e.getPosition(), e.getPosition(), e.getPosition(), msg);
return badSnippet(writer);
}

@@ -213,7 +244,7 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ
} catch (ParseException e) {
assert fileObject != null;
writer.configuration().getMessages().error(fileObject, e.getPosition(),
e.getPosition(), e.getPosition(), "doclet.snippet.markup", e.getMessage());
e.getPosition(), e.getPosition(), "doclet.snippet.markup", e.getMessage());
return badSnippet(writer);
}

@@ -235,8 +266,7 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ
}
}
if (r1 == null && r2 == null) {
error(writer, holder, tag, "doclet.snippet.region.not_found", regionName);
return badSnippet(writer);
throw new BadSnippetException(tag, "doclet.snippet.region.not_found", regionName);
}
}

@@ -252,9 +282,7 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ
String inlineStr = inlineSnippet.asCharSequence().toString();
String externalStr = externalSnippet.asCharSequence().toString();
if (!Objects.equals(inlineStr, externalStr)) {
error(writer, holder, tag, "doclet.snippet.contents.mismatch", diff(inlineStr, externalStr));
// output one above the other
return badSnippet(writer);
throw new BadSnippetException(tag, "doclet.snippet.contents.mismatch", diff(inlineStr, externalStr));
}
}

@@ -263,17 +291,17 @@ public Content getInlineTagOutput(Element holder, DocTree tag, TagletWriter writ

String lang = null;
AttributeTree langAttr = attributes.get("lang");
if (langAttr != null && langAttr.getValueKind() != AttributeTree.ValueKind.EMPTY) {
lang = stringOf(langAttr.getValue());
if (langAttr != null) {
lang = stringValueOf(langAttr);
} else if (containsClass) {
lang = "java";
} else if (containsFile) {
lang = languageFromFileName(fileObject.getName());
}
AttributeTree idAttr = attributes.get("id");
String id = idAttr == null || idAttr.getValueKind() == AttributeTree.ValueKind.EMPTY
? null
: stringOf(idAttr.getValue());
String id = idAttr == null
? null
: stringValueOf(idAttr);

return writer.snippetTagOutput(holder, snippetTag, text, id, lang);
}
@@ -304,8 +332,12 @@ private StyledText parse(Resources resources, String content) throws ParseExcept
return result.text();
}

private static String stringOf(List<? extends DocTree> value) {
return value.stream()
private static String stringValueOf(AttributeTree at) throws BadSnippetException {
if (at.getValueKind() == AttributeTree.ValueKind.EMPTY) {
throw new BadSnippetException(at, "doclet.tag.attribute.value.missing",
at.getName().toString());
}
return at.getValue().stream()
// value consists of TextTree or ErroneousTree nodes;
// ErroneousTree is a subtype of TextTree
.map(t -> ((TextTree) t).getBody())
@@ -58,8 +58,7 @@
* This code and its internal interfaces are subject to change or
* deletion without notice.</b>
*/
// TODO: uncomment /* sealed */ when minimum boot JDK version >= 17
public /* sealed */ abstract class Attribute {
public abstract class Attribute {

private final String name;

@@ -67,12 +67,4 @@ public <T extends Attribute> Optional<T> get(String name, Class<T> type) {
.map(type::cast)
.findAny();
}

public int size() {
return attributes.values().stream().mapToInt(List::size).sum();
}

public boolean isEmpty() {
return attributes.isEmpty();
}
}
@@ -152,15 +152,15 @@ record OffsetAndLine(int offset, String line) { }
line = rawLine + (addLineTerminator ? "\n" : "");
} else {
String maybeMarkup = markedUpLine.group(3);
List<Tag> parsedTags = null;
List<Tag> parsedTags;
try {
parsedTags = markupParser.parse(maybeMarkup);
} catch (ParseException e) {
// adjust index
// translate error position from markup to file line
throw new ParseException(e::getMessage, markedUpLine.start(3) + e.getPosition());
}
for (Tag t : parsedTags) {
t.lineSourceOffset = next.offset;
t.lineSourceOffset = next.offset();
t.markupLineOffset = markedUpLine.start(3);
}
thisLineTags.addAll(parsedTags);
@@ -66,7 +66,7 @@ record Replacement(int start, int end, String value) { }
Matcher matcher = pattern.matcher(textString);
var replacements = new ArrayList<Replacement>();
StringBuilder b = new StringBuilder();
int off = 0; // offset because of the replacements (can be negative)
int off = 0; // cumulative offset caused by replacements (can become negative)
while (matcher.find()) {
int start = matcher.start();
int end = matcher.end();
@@ -79,7 +79,7 @@ record Replacement(int start, int end, String value) { }
// there's no need to call matcher.appendTail(b)
for (int i = replacements.size() - 1; i >= 0; i--) {
Replacement r = replacements.get(i);
text.subText(r.start, r.end).replace(Set.of(), r.value);
text.subText(r.start(), r.end()).replace(Set.of(), r.value());
}
}
}
@@ -33,8 +33,7 @@
* This code and its internal interfaces are subject to change or
* deletion without notice.</b>
*/
// TODO: uncomment /* sealed */ when minimum boot JDK version >= 17
public /* sealed */ interface Style {
public sealed interface Style {

/**
* A style that describes a link. Characters of this style are typically

1 comment on commit 2ab43ec

@openjdk-notifier
Copy link

@openjdk-notifier openjdk-notifier bot commented on 2ab43ec Nov 19, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.