8276964: Better indicate a snippet that could not be processed

Reviewed-by: jjg
This commit is contained in:
Hannes Wallnöfer 2021-12-08 07:07:57 +00:00
parent 30f0c64753
commit b334d9680b
13 changed files with 329 additions and 151 deletions

View File

@ -36,6 +36,7 @@ import java.util.ListIterator;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@ -1063,7 +1064,8 @@ public class HtmlDocletWriter {
"doclet.see.class_or_package_not_found",
"@" + tagName,
seeText);
return (labelContent.isEmpty() ? text: labelContent);
return invalidTagOutput(resources.getText("doclet.tag.invalid", tagName),
Optional.of(labelContent.isEmpty() ? text: labelContent));
}
}
} else if (refMemName == null) {
@ -1614,13 +1616,18 @@ public class HtmlDocletWriter {
DocTreePath dtp = ch.getDocTreePath(node);
if (dtp != null) {
String body = node.getBody();
if (body.matches("(?i)\\{@[a-z]+.*")) {
messages.warning(dtp,"doclet.tag.invalid_usage", body);
} else {
Matcher m = Pattern.compile("(?i)\\{@([a-z]+).*").matcher(body);
String tagName = m.matches() ? m.group(1) : null;
if (tagName == null) {
messages.warning(dtp, "doclet.tag.invalid_input", body);
result.add(invalidTagOutput(resources.getText("doclet.tag.invalid_input", body),
Optional.empty()));
} else {
messages.warning(dtp, "doclet.tag.invalid_usage", body);
result.add(invalidTagOutput(resources.getText("doclet.tag.invalid", tagName),
Optional.of(Text.of(body))));
}
}
result.add(Text.of(node.toString()));
return false;
}
@ -1774,6 +1781,24 @@ public class HtmlDocletWriter {
&& currentPageElement != utils.getEnclosingTypeElement(element));
}
/**
* Returns the output for an invalid tag. The returned content uses special styling to
* highlight the problem. Depending on the presence of the {@code detail} string the method
* returns a plain text span or an expandable component.
*
* @param summary the single-line summary message
* @param detail the optional detail message which may contain preformatted text
* @return the output
*/
protected Content invalidTagOutput(String summary, Optional<Content> detail) {
if (detail.isEmpty() || detail.get().isEmpty()) {
return HtmlTree.SPAN(HtmlStyle.invalidTag, Text.of(summary));
}
return new HtmlTree(TagName.DETAILS).addStyle(HtmlStyle.invalidTag)
.add(new HtmlTree(TagName.SUMMARY).add(Text.of(summary)))
.add(new HtmlTree(TagName.PRE).add(detail.get()));
}
/**
* Returns true if element lives in the same package as the type or package
* element of this writer.

View File

@ -29,6 +29,7 @@ import java.util.ArrayList;
import java.util.EnumSet;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import javax.lang.model.element.Element;
@ -536,6 +537,14 @@ public class TagletWriterImpl extends TagletWriter {
: Text.of(constantVal);
}
@Override
protected Content invalidTagOutput(String summary, Optional<String> detail) {
return htmlWriter.invalidTagOutput(summary,
detail.isEmpty() || detail.get().isEmpty()
? Optional.empty()
: Optional.of(Text.of(utils.normalizeNewlines(detail.get()))));
}
@Override
public Content commentTagsToOutput(DocTree holder, List<? extends DocTree> tags) {
return commentTagsToOutput(null, holder, tags, false);

View File

@ -907,6 +907,11 @@ public enum HtmlStyle {
*/
inheritedList,
/**
* The class of an element that acts as a notification for an invalid tag.
*/
invalidTag,
/**
* The class of a {@code p} element containing legal copy in the page footer.
*/

View File

@ -47,6 +47,7 @@ public enum TagName {
CAPTION,
CODE,
DD,
DETAILS,
DIV,
DL,
DT,
@ -83,6 +84,7 @@ public enum TagName {
SPAN,
STRONG,
SUB,
SUMMARY,
SUP,
TABLE,
TBODY,

View File

@ -106,6 +106,7 @@ doclet.see.class_or_package_not_accessible=Tag {0}: reference not accessible: {1
doclet.see.nested_link=Tag {0}: nested link
doclet.tag.invalid_usage=invalid usage of tag {0}
doclet.tag.invalid_input=invalid input: ''{0}''
doclet.tag.invalid=invalid @{0}
doclet.Deprecated_API=Deprecated API
doclet.Deprecated_Elements=Deprecated {0}
doclet.Deprecated_In_Release=Deprecated in {0}

View File

@ -555,6 +555,18 @@ div.block {
div.block div.deprecation-comment {
font-style:normal;
}
details.invalid-tag, span.invalid-tag {
font-size:14px;
font-family:'DejaVu Serif', Georgia, "Times New Roman", Times, serif;
background: #ffe6e6;
border: thin solid #000000;
border-radius:2px;
padding: 2px 4px;
display:inline-block;
}
details.invalid-tag summary {
cursor: pointer;
}
/*
* Styles specific to HTML5 elements.
*/
@ -991,11 +1003,9 @@ button.snippet-copy:active {
pre.snippet .italic {
font-style: italic;
}
pre.snippet .bold {
font-weight: bold;
}
pre.snippet .highlighted {
background-color: #f7c590;
border-radius: 10%;

View File

@ -120,7 +120,8 @@ public class SnippetTaglet extends BaseTaglet {
return generateContent(holder, tag, writer);
} catch (BadSnippetException e) {
error(writer, holder, e.tag(), e.key(), e.args());
return badSnippet(writer);
String details = writer.configuration().getDocResources().getText(e.key(), e.args());
return badSnippet(writer, Optional.of(details));
}
}
@ -286,7 +287,7 @@ public class SnippetTaglet extends BaseTaglet {
.getText("doclet.snippet.markup", e.getMessage());
writer.configuration().getReporter().print(Diagnostic.Kind.ERROR,
path, e.getPosition(), e.getPosition(), e.getPosition(), msg);
return badSnippet(writer);
return badSnippet(writer, Optional.of(e.getMessage()));
}
try {
@ -299,7 +300,7 @@ public class SnippetTaglet extends BaseTaglet {
assert fileObject != null;
writer.configuration().getMessages().error(fileObject, e.getPosition(),
e.getPosition(), e.getPosition(), "doclet.snippet.markup", e.getMessage());
return badSnippet(writer);
return badSnippet(writer, Optional.of(e.getMessage()));
}
// the region must be matched at least in one content: it can be matched
@ -408,8 +409,9 @@ public class SnippetTaglet extends BaseTaglet {
writer.configuration().utils.getCommentHelper(holder).getDocTreePath(tag), key, args);
}
private Content badSnippet(TagletWriter writer) {
return writer.getOutputInstance().add("bad snippet");
private Content badSnippet(TagletWriter writer, Optional<String> details) {
Resources resources = writer.configuration().getDocResources();
return writer.invalidTagOutput(resources.getText("doclet.tag.invalid", "snippet"), details);
}
private String packageName(PackageElement pkg, Utils utils) {

View File

@ -27,6 +27,7 @@ package jdk.javadoc.internal.doclets.toolkit.taglets;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.lang.model.element.Element;
import javax.lang.model.element.ElementKind;
import javax.lang.model.element.TypeElement;
@ -239,6 +240,17 @@ public abstract class TagletWriter {
protected abstract Content valueTagOutput(VariableElement field,
String constantVal, boolean includeLink);
/**
* Returns the output for an invalid tag. The returned content uses special styling to
* highlight the problem. Depending on the presence of the {@code detail} string the method
* returns a plain text span or an expandable component.
*
* @param summary the single-line summary message
* @param detail the optional detail message which may contain preformatted text
* @return the output
*/
protected abstract Content invalidTagOutput(String summary, Optional<String> detail);
/**
* Returns the main type element of the current page or null for pages that don't have one.
*

View File

@ -166,18 +166,48 @@ public class TestGenericTypeLink extends JavadocTester {
checkExit(Exit.ERROR);
checkOutput("pkg2/B.html", true,
"""
<div class="block"><code>java.util.Foo&lt;String&gt;</code>
Baz&lt;Object&gt;
<code>#b(List&lt;Integer&gt;)</code></div>""",
<div class="block">
<details class="invalid-tag">
<summary>invalid @link</summary>
<pre><code>java.util.Foo&lt;String&gt;</code></pre>
</details>
\s
<details class="invalid-tag">
<summary>invalid @linkplain</summary>
<pre>Baz&lt;Object&gt;</pre>
</details>
\s
<details class="invalid-tag">
<summary>invalid @link</summary>
<pre><code>#b(List&lt;Integer&gt;)</code></pre>
</details>
</div>""",
"""
<dl class="notes">
<dt>See Also:</dt>
<dd>
<ul class="see-list-long">
<li><code>java.util.List&lt;Bar&gt;</code></li>
<li><code>Baz&lt;Object, String&gt;</code></li>
<li><code>B#b(List&lt;Baz&gt;)</code></li>
<li>
<details class="invalid-tag">
<summary>invalid @see</summary>
<pre><code>java.util.List&lt;Bar&gt;</code></pre>
</details>
</li>
<li>
<details class="invalid-tag">
<summary>invalid @see</summary>
<pre><code>Baz&lt;Object, String&gt;</code></pre>
</details>
</li>
<li>
<details class="invalid-tag">
<summary>invalid @see</summary>
<pre><code>B#b(List&lt;Baz&gt;)</code></pre>
</details>
</li>
</ul>
</dd>
</dl>""");

View File

@ -69,11 +69,11 @@ public class TestInherited extends JavadocTester {
checkExit(Exit.OK);
checkOutput("BadParam.Base.html", true, """
<dt>Parameters:</dt>
<dd><code>i</code> - a &lt; b</dd>
<dd><code>i</code> - a <span class="invalid-tag">invalid input: '&lt;'</span> b</dd>
""");
checkOutput("BadParam.Sub.html", true, """
<dt>Parameters:</dt>
<dd><code>i</code> - a &lt; b</dd>
<dd><code>i</code> - a <span class="invalid-tag">invalid input: '&lt;'</span> b</dd>
""");
}
@ -101,11 +101,11 @@ public class TestInherited extends JavadocTester {
checkExit(Exit.OK);
checkOutput("BadReturn.Base.html", true, """
<dt>Returns:</dt>
<dd>a &lt; b</dd>
<dd>a <span class="invalid-tag">invalid input: '&lt;'</span> b</dd>
""");
checkOutput("BadReturn.Sub.html", true, """
<dt>Returns:</dt>
<dd>a &lt; b</dd>
<dd>a <span class="invalid-tag">invalid input: '&lt;'</span> b</dd>
""");
}
@ -147,16 +147,36 @@ public class TestInherited extends JavadocTester {
src.resolve("BadReference.java").toString());
checkExit(Exit.OK);
checkOutput("BadReference.Intf.html", true, """
<div class="block"><code>NonExistingClass</code></div>
<div class="block">
<details class="invalid-tag">
<summary>invalid @link</summary>
<pre><code>NonExistingClass</code></pre>
</details>
</div>
""");
checkOutput("BadReference.Impl1.html", true, """
<div class="block"><code>NonExistingClass</code></div>
<div class="block">
<details class="invalid-tag">
<summary>invalid @link</summary>
<pre><code>NonExistingClass</code></pre>
</details>
</div>
""");
checkOutput("BadReference.Impl2.html", true, """
<div class="block"><code>NonExistingClass</code></div>
<div class="block">
<details class="invalid-tag">
<summary>invalid @link</summary>
<pre><code>NonExistingClass</code></pre>
</details>
</div>
""");
checkOutput("BadReference.Impl3.html", true, """
<div class="block"><code>NonExistingClass</code></div>
<div class="block">
<details class="invalid-tag">
<summary>invalid @link</summary>
<pre><code>NonExistingClass</code></pre>
</details>
</div>
""");
}
}

View File

@ -85,6 +85,6 @@ public class TestNonInlineHtmlTagRemoval extends JavadocTester {
checkOutput("Negative.html", true,
"""
<div class="block">case1: A hanging &lt; : xx&lt;</div>""");
<div class="block">case1: A hanging &lt; : xx<span class="invalid-tag">invalid input: '&lt;'</span></div>""");
}
}

View File

@ -100,7 +100,12 @@ public class TestSeeTag extends JavadocTester {
<dd>
<ul class="see-list">
<li><code>Object</code></li>
<li><code>Foo&lt;String&gt;</code></li>
<li>
<details class="invalid-tag">
<summary>invalid @see</summary>
<pre><code>Foo&lt;String&gt;</code></pre>
</details>
</li>
</ul>
</dd>
</dl>""");

View File

@ -23,7 +23,7 @@
/*
* @test
* @bug 8266666 8275788
* @bug 8266666 8275788 8276964
* @summary Implementation for snippets
* @library /tools/lib ../../lib
* @modules jdk.compiler/com.sun.tools.javac.api
@ -1080,6 +1080,12 @@ public class TestSnippetTag extends SnippetTester {
checkOutput(Output.OUT, true,
"""
A.java:4: error: File not found: %s""".formatted(fileName));
checkOutput("pkg/A.html", true, """
<details class="invalid-tag">
<summary>invalid @snippet</summary>
<pre>File not found: text.txt</pre>
</details>
""");
checkNoCrashes();
}
@ -1155,6 +1161,12 @@ public class TestSnippetTag extends SnippetTester {
checkOutput(Output.OUT, true,
"""
A.java:3: error: @snippet does not specify contents""");
checkOutput("pkg/A.html", true, """
<details class="invalid-tag">
<summary>invalid @snippet</summary>
<pre>@snippet does not specify contents</pre>
</details>
""");
checkNoCrashes();
}
@ -1211,6 +1223,12 @@ public class TestSnippetTag extends SnippetTester {
checkOutput(Output.OUT, true,
"""
A.java:3: error: @snippet specifies multiple external contents, which is ambiguous""");
checkOutput("pkg/A.html", true, """
<details class="invalid-tag">
<summary>invalid @snippet</summary>
<pre>@snippet specifies multiple external contents, which is ambiguous</pre>
</details>
""");
checkNoCrashes();
}
@ -1863,15 +1881,15 @@ public class TestSnippetTag extends SnippetTester {
for (String attrName : List.of("class", "file", "id", "lang", "region")) {
// special case: valueless region attribute
TestCase t = new TestCase("""
{@snippet %s:
First line
Second line
}
""".formatted(attrName),
"""
: error: missing value for attribute "%s"
{@snippet %s:
^""".formatted(attrName, attrName));
{@snippet %s:
First line
Second line
}
""".formatted(attrName),
"""
: error: missing value for attribute "%s"
{@snippet %s:
^""".formatted(attrName, attrName));
testCases.add(t);
}
@ -1915,15 +1933,18 @@ public class TestSnippetTag extends SnippetTester {
for (String quote : List.of("", "'", "\""))
for (String value : List.of("", " ")) {
var t = new TestCase("""
{@snippet region=%s%s%s:
First line
Second line
}
""".formatted(quote, value, quote),
"""
: error: illegal value for attribute "region": "%s"
{@snippet region=%s%s%s:
^""".formatted(quote.isEmpty() ? "" : value, quote, value, quote)); // unquoted whitespace translates to empty string
{@snippet region=%s%s%s:
First line
Second line
}
""".formatted(quote, value, quote),
"""
: error: illegal value for attribute "region": "%s"
{@snippet region=%s%s%s:
^
""".formatted(
quote.isEmpty() ? "" : value, // unquoted whitespace translates to empty string
quote, value, quote));
testCases.add(t);
}
@ -2015,6 +2036,17 @@ public class TestSnippetTag extends SnippetTester {
checkOutput(Output.OUT, true,
"""
A.java:4: error: contents mismatch""");
checkOutput("pkg/A.html", true, """
<details class="invalid-tag">
<summary>invalid @snippet</summary>
<pre>contents mismatch:
----------------- inline -------------------
Hello, Snippet!
----------------- external -----------------
Hello, Snippet!...more
</pre>
</details>
""");
checkNoCrashes();
}
@ -2055,6 +2087,19 @@ public class TestSnippetTag extends SnippetTester {
checkOutput(Output.OUT, true,
"""
A.java:4: error: contents mismatch""");
checkOutput("pkg/A.html", true, """
<details class="invalid-tag">
<summary>invalid @snippet</summary>
<pre>contents mismatch:
----------------- inline -------------------
Hello, Snippet! ...more
----------------- external -----------------
Hello, Snippet!
</pre>
</details>
""");
checkNoCrashes();
}
@ -2278,41 +2323,41 @@ public class TestSnippetTag extends SnippetTester {
final var testCases = List.of(
new TestCase("""
{@snippet :
hello there // @highlight regex ="\t**"
}""",
{@snippet :
hello there // @highlight regex ="\t**"
}""",
"""
error: snippet markup: invalid regex
hello there // @highlight regex ="\t**"
\t ^
"""),
error: snippet markup: invalid regex
hello there // @highlight regex ="\t**"
\t ^
"""),
new TestCase("""
{@snippet :
hello there // @highlight regex ="\\t**"
}""",
{@snippet :
hello there // @highlight regex ="\\t**"
}""",
"""
error: snippet markup: invalid regex
hello there // @highlight regex ="\\t**"
^
"""),
error: snippet markup: invalid regex
hello there // @highlight regex ="\\t**"
^
"""),
new TestCase("""
{@snippet :
hello there // @highlight regex="\\.\\*\\+\\E"
}""",
{@snippet :
hello there // @highlight regex="\\.\\*\\+\\E"
}""",
"""
error: snippet markup: invalid regex
hello there // @highlight regex="\\.\\*\\+\\E"
\s\s\s\s ^
"""), // use \s to counteract shift introduced by \\ so as to visually align ^ right below E
error: snippet markup: invalid regex
hello there // @highlight regex="\\.\\*\\+\\E"
\s\s\s\s ^
"""), // use \s to counteract shift introduced by \\ so as to visually align ^ right below E
new TestCase("""
{@snippet :
hello there // @highlight type="italics" regex =" ["
}""",
{@snippet :
hello there // @highlight type="italics" regex =" ["
}""",
"""
error: snippet markup: invalid regex
hello there // @highlight type="italics" regex =" ["
^
""")
error: snippet markup: invalid regex
hello there // @highlight type="italics" regex =" ["
^
""")
);
List<String> inputs = testCases.stream().map(s -> s.input).toList();
@ -2342,6 +2387,12 @@ hello there // @highlight type="italics" regex =" ["
src.resolve("A.java").toString());
checkExit(Exit.ERROR);
checkOrder(Output.OUT, testCases.stream().map(TestCase::expectedError).toArray(String[]::new));
checkOutput("A.html", true, """
<details class="invalid-tag">
<summary>invalid @snippet</summary>
<pre>invalid regex</pre>
</details>
""");
checkNoCrashes();
}
@ -2352,118 +2403,118 @@ hello there // @highlight type="italics" regex =" ["
final var testCases = List.of(
new TestCase("""
{@snippet :
hello // @link
}""",
{@snippet :
hello // @link
}""",
"""
error: snippet markup: missing attribute "target"
hello // @link
^
error: snippet markup: missing attribute "target"
hello // @link
^
"""),
new TestCase("""
{@snippet :
hello // @start
}""",
{@snippet :
hello // @start
}""",
"""
error: snippet markup: missing attribute "region"
hello // @start
^
error: snippet markup: missing attribute "region"
hello // @start
^
"""),
new TestCase("""
{@snippet :
hello // @replace
}""",
{@snippet :
hello // @replace
}""",
"""
error: snippet markup: missing attribute "replacement"
hello // @replace
^
error: snippet markup: missing attribute "replacement"
hello // @replace
^
"""),
/* ---------------------- */
new TestCase("""
{@snippet :
hello // @highlight regex=\\w+ substring=hello
}""",
{@snippet :
hello // @highlight regex=\\w+ substring=hello
}""",
"""
error: snippet markup: attributes "substring" and "regex" used simultaneously
hello // @highlight regex=\\w+ substring=hello
^
error: snippet markup: attributes "substring" and "regex" used simultaneously
hello // @highlight regex=\\w+ substring=hello
^
"""),
new TestCase("""
{@snippet :
hello // @start region="x" name="here"
}""",
{@snippet :
hello // @start region="x" name="here"
}""",
"""
error: snippet markup: unexpected attribute
hello // @start region="x" name="here"
^
error: snippet markup: unexpected attribute
hello // @start region="x" name="here"
^
"""),
new TestCase("""
{@snippet :
hello // @start region=""
}""",
{@snippet :
hello // @start region=""
}""",
"""
error: snippet markup: invalid attribute value
hello // @start region=""
^
error: snippet markup: invalid attribute value
hello // @start region=""
^
"""),
new TestCase("""
{@snippet :
hello // @link target="Object#equals()" type=fluffy
}""",
{@snippet :
hello // @link target="Object#equals()" type=fluffy
}""",
"""
error: snippet markup: invalid attribute value
hello // @link target="Object#equals()" type=fluffy
^
error: snippet markup: invalid attribute value
hello // @link target="Object#equals()" type=fluffy
^
"""),
/* ---------------------- */
new TestCase("""
{@snippet :
hello
there // @highlight substring="
}""",
{@snippet :
hello
there // @highlight substring="
}""",
"""
error: snippet markup: unterminated attribute value
there // @highlight substring="
^
error: snippet markup: unterminated attribute value
there // @highlight substring="
^
"""),
new TestCase("""
{@snippet :
hello // @start region="this"
world // @start region="this"
! // @end
}""",
{@snippet :
hello // @start region="this"
world // @start region="this"
! // @end
}""",
"""
error: snippet markup: duplicated region
world // @start region="this"
^
error: snippet markup: duplicated region
world // @start region="this"
^
"""),
new TestCase("""
{@snippet :
hello // @end
}""",
{@snippet :
hello // @end
}""",
"""
error: snippet markup: no region to end
hello // @end
^
error: snippet markup: no region to end
hello // @end
^
"""),
new TestCase("""
{@snippet :
hello // @start region=this
}""",
{@snippet :
hello // @start region=this
}""",
"""
error: snippet markup: unpaired region
hello // @start region=this
^
error: snippet markup: unpaired region
hello // @start region=this
^
"""),
new TestCase("""
{@snippet :
hello // @highlight substring="hello" :
}""",
{@snippet :
hello // @highlight substring="hello" :
}""",
"""
error: snippet markup: tag refers to non-existent lines
hello // @highlight substring="hello" :
^
""")
error: snippet markup: tag refers to non-existent lines
hello // @highlight substring="hello" :
^
""")
);
List<String> inputs = testCases.stream().map(s -> s.input).toList();
StringBuilder methods = new StringBuilder();
@ -2493,6 +2544,12 @@ error: snippet markup: tag refers to non-existent lines
checkExit(Exit.ERROR);
// use the facility from JDK-8273154 when it becomes available
checkOutput(Output.OUT, true, testCases.stream().map(TestCase::expectedError).toArray(String[]::new));
checkOutput("A.html", true, """
<details class="invalid-tag">
<summary>invalid @snippet</summary>
<pre>missing attribute "target"</pre>
</details>
""");
checkNoCrashes();
}
}