Conversation
maximthomas
left a comment
There was a problem hiding this comment.
praise: The check renders each page the way the site does and stays quiet on braces that were never attributes.
processreplays the body's attribute entries withplayback_attributesand restores every document afterwards (literal-attribute-references.rb:46-60), and the description shows the HTML is byte-identical with and without the extension.names_incounts a{name}only when some:name:entry in the sources sets it (literal-attribute-references.rb:39-43), so{SSHA}values and MakeLDIF{cn}tokens stay quiet, while a page that neither defines nor substitutes{opendj-version}is still caught.- The three mutants in the description, and the fact that the check finds both defects #1140 fixed again, show that both halves of the check fire on real pages.
issue (non-blocking): A literal-style table cell (l|, or a cols spec with l) is never checked.
opendj-doc-generated-ref/src/main/resources/asciidoc/extensions/literal-attribute-references.rb:55-56
Asciidoctor::Block === block is false for Asciidoctor::Table::Cell, whose ancestors are AbstractBlock and AbstractNode. In gem 2.0.20 a literal cell gets content_model :verbatim with BASIC_SUBS (table.rb:312-314). A page that defines :opendj-version: and has l|literal cell {opendj-version} therefore publishes the braces. attribute-missing cannot fire there, and the extension never visits the cell. A probe with the jar's gem warned on the listing and the indented literal of the same page, but not on the cell. No such cell exists at HEAD. A cell also cannot take a subs attribute, so it needs its own advice.
if Asciidoctor::Table::Cell === block
check block, block.text, names if block.content_model == :verbatim
elsif Asciidoctor::Block === block &&
(block.content_model == :verbatim || block.content_model == :raw) && !(block.subs.include? :attributes)
check block, (block.lines.join Asciidoctor::LF), names
endcheck then scans its text argument instead of block.lines. A literal cell's text only applies specialcharacters, so its braces stay intact.
issue (non-blocking): Dir.glob treats the literal-attribute-sources path as a pattern, so a checkout path that contains [ or { silently empties the cross-page name set.
opendj-doc-generated-ref/src/main/resources/asciidoc/extensions/literal-attribute-references.rb:40
File.join dir, '**', '*.adoc' with dir = ${project.build.directory}/asciidoc/source is not escaped. A probe on MRI 3.2.3 matched 0 files under ws[1]/src and ws{a}/src, and 1 under plain/src. The empty set is memoized with no message. After that, a listing that uses {opendj-version}, on a page that neither defines nor substitutes it, is no longer reported. CI paths contain no metacharacter, so only local checkouts are hit.
def self.names_in dir
@names_by_dir[dir] ||= Dir.glob('**/*.adoc', base: dir).each_with_object(Set.new) do |path, names|
File.foreach((File.join dir, path), encoding: 'UTF-8') {|line| names << $1.downcase if EntryRx =~ line }
end
endsuggestion (non-blocking): Nothing pins the check itself. A mutant that disables the extension or drops attribute-missing=warn keeps the build green.
opendj-doc-generated-ref/src/main/resources/asciidoc/extensions/literal-attribute-references.rb:66, opendj-doc-generated-ref/pom.xml:639-648
The execution reads only the real pre-processed pages, and at HEAD they are clean, so its only observable is a green verify. Both placeholders the PR escapes are paragraph text, which attribute-missing caught, so the tree processor has never failed a build in the repo. These mutants all leave HEAD green, measured on scratch copies: next if true at line 66, the subs.include? :attributes guard inverted, a typo in <containsText>, <attribute-missing>warn removed. A scratch pom with this execution's configuration exits 1 on a planted listing and on {undefinedthing}, so the code works today. The module has no src/test, so nothing keeps it working.
public class LiteralAttributeReferencesTest {
private static List<String> warnings(String page) {
Asciidoctor asciidoctor = Asciidoctor.Factory.create();
asciidoctor.requireLibrary(
new File("src/main/resources/asciidoc/extensions/literal-attribute-references.rb").getAbsolutePath());
List<String> warnings = new ArrayList<>();
asciidoctor.registerLogHandler(r -> {
if (r.getSeverity() == Severity.WARN) {
warnings.add(r.getMessage());
}
});
asciidoctor.convert(page, Options.builder().safe(SafeMode.UNSAFE)
.attributes(Attributes.builder().attribute("attribute-missing", "warn").build()).build());
return warnings;
}
@Test
public void listingWithoutSubsIsReported() {
assertTrue(warnings(":v: 1\n\n----\nunzip x-{v}.zip\n----\n").stream().anyMatch(m -> m.contains("attribute {v}")));
}
@Test
public void undefinedAttributeIsReported() {
assertTrue(warnings("tool {undefinedthing}\n").stream().anyMatch(m -> m.contains("missing attribute: undefinedthing")));
}
}Pin: with test-scoped org.asciidoctor:asciidoctorj:2.5.11 and org.testng:testng, the first case fails under the line-66 and line-56 mutants. The pom-side mutants (containsText, attribute-missing) need a maven-invoker IT over the same two pages with invoker.buildResult = failure.
suggestion (non-blocking): Every Maven project property is a document attribute in this render. A page that uses one passes the check and still shows the braces on the site.
opendj-doc-generated-ref/pom.xml:640
asciidoctor-maven-plugin 2.2.6 always calls AsciidoctorHelper.addMavenProperties in AsciidoctorMojo.createAttributesBuilder. It adds each project property with dots turned into dashes. attribute-missing and the extension's attributes.key? therefore treat {product-name}, {commons-version} or {docTargetVersion} as defined. The Antora site defines none of them: its playbook sets only page-toclevels/page-pagination, and opendj/antora.yml sets only pdfs. Nothing at HEAD references such a name, and {opendj-version} is not masked because no pom defines opendj.version. The plugin has no switch for this, so the comment can at least name the blind spot.
Maven project properties are attributes here (product.name is
{product-name}) but not on the site, so a page that uses one passes
this check and still shows the braces there.suggestion (non-blocking): The warning's advice, add subs="attributes", replaces a verbatim block's default subs instead of adding to them.
opendj-doc-generated-ref/src/main/resources/asciidoc/extensions/literal-attribute-references.rb:67, :18
On a listing or literal block, subs="attributes" drops specialcharacters and callouts. A probe of [subs="attributes"] over <b>1</b> & emits the markup raw. The 16 existing uses in the guides are all subs="attributes", so the advice matches the repo today. It only breaks on a block that holds <, >, & or callouts. +attributes is the additive form.
logger.warn message_with_context %(attribute {#{name}} is published as literal text: the #{block.context} block does not substitute attributes, add subs="+attributes"), source_location: block.source_locationsuggestion (non-blocking): Inside a verbatim block the extension does not report \{name}, nor the intrinsic attributes ({nbsp}, {empty}, {lt}).
opendj-doc-generated-ref/src/main/resources/asciidoc/extensions/literal-attribute-references.rb:66
A listing without attribute subs does not consume the backslash, so \{v} is published as \{v}. The intrinsic names live in Asciidoctor::INTRINSIC_ATTRIBUTES, not in document.attributes, so {nbsp} is published as {nbsp}. A probe showed neither is reported. Both gaps are latent. The pom comment's \{name} advice is correctly scoped to text.
key = name.downcase
next unless (attributes.key? key) || (names.include? key) || (Asciidoctor::INTRINSIC_ATTRIBUTES.key? key)
logger.warn message_with_context(escaped ?
%(\\{#{name}} is published with its backslash: the #{block.context} block does not substitute attributes) :
%(attribute {#{name}} is published as literal text: the #{block.context} block does not substitute attributes, add subs="attributes")),
source_location: block.source_locationOr: grep HEAD's verbatim blocks for \{ first. A hit there turns the build red.
nitpick (non-blocking): The rerun command in the pom comment fails anywhere but Linux.
opendj-doc-generated-ref/pom.xml:615-618
The module is in the reactor only through the Linux-activated distribution-unix profile of opendj-packages/pom.xml, and the execution sits in the Linux-activated man-pages profile. On macOS, Maven 3.9.16 runs the command against HEAD's poms and stops with Could not find the selected project in the reactor: opendj-doc-generated-ref. With -Pdistribution-unix,man-pages, Maven selects the module and the execution resolves.
The plugin stops at the first page that fails, so fix it and run again to see
the next one: mvn -Pdistribution-unix,man-pages -pl opendj-doc-generated-ref
asciidoctor:process-asciidoc@check-attribute-references, after a build of this
module (both profiles activate by themselves only on Linux).…AsciiDoc attribute unresolved
Render every page of the pre-processed doc sources on its own, the way Antora
publishes each chapter, with attribute-missing=warn and an extension that reports
an attribute reference in a verbatim block without subs="attributes". Either
warning fails the build in the new check-attribute-references execution.
Escape the two literal placeholders the check found on master: {options} in the
windows-service synopsis and {path} in the password policy chapter.
…intrinsic references, test the check with its pom configuration, and keep the special characters of the attribute listings
- literal-attribute-references.rb also checks a literal table cell (l|, cols="1l"),
which takes no subs and gets its own advice; reports \{name} and intrinsic names
such as {nbsp} in a block without attribute subs; advises subs="+attributes",
which keeps the block's default subs; and globs the sources with the directory as
base, so braces in its path no longer empty the set of names.
- LiteralAttributeReferencesTest renders pages with the configuration of the
check-attribute-references execution, read from the pom, and checks which fail
the build. The pom module binds testCompile and test for it; the plugin and the
test share asciidoctorj ${asciidoctorj.version}.
- The 16 blocks of the guides with subs="attributes" take subs="+attributes": the
replacing form had dropped specialcharacters, so the plugin guide published
"(build <unknown>, revision <unknown>)" as raw tags, which a browser hides.
- The pom comment gives the rerun command with the profiles it needs off Linux and
names the Maven project properties as a blind spot of the check.
|
Thanks for the review. All seven points are taken in the new commit; the branch is rebased on master with #1132 first. Literal table cells: taken. A cell with
Nothing pins the check: taken, with one change of approach. Instead of a maven-invoker IT, Maven project properties: taken as a comment in the pom and a line under Limits in the description, close to your wording. I confirmed
Rerun command off Linux: taken, with your text: The description is updated to match: the Change, Limits and Verification sections. |
Fixes #1147
Problem
A page that uses an attribute it does not define, or a verbatim block that uses an attribute without attribute subs, publishes
{name}literally, and the build stays green. The only render in the build is the PDF book, where an attribute defined in an earlier chapter also resolves in the later ones; the site renders every chapter as a page of its own, so a page can be broken on the site while the PDF is fine (chap-uninstall.adocbefore #1140). Theantoragoal ofdoc-maven-pluginonly rewrites the.adocfiles, it renders nothing.Change
check-attribute-referencesexecution ofasciidoctor-maven-pluginin theman-pagesprofile (phaseverify, so it runs on the ubuntu legs of the PR build). It renders every.adocoftarget/asciidoc/sourceon its own to throwaway HTML, with each file's own directory as the base, and fails on any WARN that mentions an attribute:attribute-missing=warn;l|,cols="1l"): the newliteral-attribute-references.rbtree processor. A reference counts when its name is an attribute at that point of the page, an intrinsic one such as{nbsp}, or one that a:name:entry anywhere in the doc sources sets - the last catches a page that neither defines nor substitutes the attribute. Braces around any other name ({SSHA}values,{cn}in MakeLDIF templates) are left alone. Without attribute subs a backslash does not escape, so\{name}is reported too, as published with its backslash.The advice in the warning is
subs="+attributes", which keeps the block's default subs; a literal cell takes no subs, so it is told to become ana|cell with such a listing.LiteralAttributeReferencesTestreads the execution's configuration from the pom - the required extensions, the attributes, thefailIf- renders pages with it, and checks which of them would fail the build: 11 that must fail, 7 that must pass. The pom module bindstestCompileandtestfor it, and the plugin and the test shareasciidoctorj${asciidoctorj.version}(2.5.11, the plugin's own).subs="attributes"takesubs="+attributes". The replacing form had droppedspecialcharacters, sochap-writing-pluginspublished(build <unknown>, revision <unknown>)as raw tags, which a browser hides; the>,>>>>and&&of the other blocks were raw as well. The rendered HTML of the six pages differs only in that escaping.{options}in thewindows-servicesynopsis and{path}in the password policy chapter (both were shown with braces, but by accident).Asciidoctor resets the document attributes to the header before tree processors run, so the extension replays the attribute entries of the body in document order, as the converter does, and resets them again afterwards.
Limits
asciidoctor-maven-plugin2.2.6 evaluatesfailIfafter each file, so the build stops at the first page that fails; fix it and runmvn -Pdistribution-unix,man-pages -pl opendj-doc-generated-ref asciidoctor:process-asciidoc@check-attribute-referencesagain for the next one (both profiles activate by themselves only on Linux). This is noted in the pom.Convertedline that follows the warning.product.nameis{product-name}), and the site defines none of them, so a page that uses one passes the check and still shows the braces on the site. The plugin has no switch for it; no page uses one today. This is noted in the pom.Verification
BUILD SUCCESS; the check itself takes about 35 s. The hand-written pages were pre-processed again from this branch; the generated reference pages come from my last module build.{opendj-version}inchap-uninstall.adocand the listing without subs inchap-monitoring.adoc.:opendj-version:removed fromchap-uninstall;subs="attributes"removed from a listing inchap-monitoring; both the subs and the definition removed fromchap-monitoring(caught only through the:name:entries of the other pages).LiteralAttributeReferencesTest: 18 green. Each of 13 mutants turns it red - in the extension: no report at all, the subs guard inverted, no literal-cell branch, no intrinsic names, no replay of body entries, the sources path back in the glob pattern,\{name}skipped, the oldsubs="attributes"advice; in the pom: a typo incontainsText,failIfonERROR, noattribute-missing, noliteral-attribute-sources, the extension not required.{SSHA}/{givenName}braces, a sources path with{and[.