Refactor construction of Asciidoctor options to make them independent (…

…#134) Refactor construction of Asciidoctor options to make them independent for each conversion. Now we can apply the original intended code where we respect DocType if present. Only template initialization is shared to reduce IO operations (create temp dirs). * Refactor options creation into AsciidoctorOptionsFactory * Remove shared attribute options from AsciidoctorConverter. * Remove unnecessary cleanup logic from AsciidoctorFilteredEnvironment. * Minor refactors and formatting fixes. Closes #125
asciidoctor · Apr 19, 2024 · bdf3646 · bdf3646
1 parent 144704b
commit bdf3646
Show file tree

Hide file tree

Showing 11 changed files with 267 additions and 137 deletions.
diff --git a/pom.xml b/pom.xml
@@ -61,6 +61,12 @@
             <version>3.25.3</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>org.mockito</groupId>
+            <artifactId>mockito-core</artifactId>
+            <version>5.11.0</version>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
 
     <dependencyManagement>

diff --git a/src/main/java/org/asciidoctor/asciidoclet/AsciiDocTrees.java b/src/main/java/org/asciidoctor/asciidoclet/AsciiDocTrees.java
@@ -98,12 +98,11 @@ private Tokens.Comment convertToAsciidoctor(Tokens.Comment comment) {
         AsciidocComment result = new AsciidocComment(asciidoc, comment);
         return result;
     }
-    
+
     private String convertJavadocStringToAsciidoctorString(String javadocString) {
-      return converter.convert(javadocString);
+        return converter.convert(javadocString);
     }
-
-
+
     @Override
     public DocCommentTree getDocCommentTree(Element e) {
         TreePath path = getPath(e);
@@ -154,19 +153,19 @@ public DocTreePath getDocTreePath(FileObject fileObject, PackageElement packageE
     public Element getElement(DocTreePath path) {
         return docTrees.getElement(path);
     }
-    
+
     // Not giving @Override in order to make this class compilable under all of JDK 11, 17, 21.
     public TypeMirror getType(DocTreePath path) {
         // In order to make this method compilable with JDK11, which doesn't define DocTrees#getType method,
         // and make this method work with JDK 17 and later, invoke the DocTrees#getType(DocTreePath) method reflectively.
         // Once we decide to stop supporting JDK 11, just call getType directly.
-      try {
-        return (TypeMirror) DocTrees.class.getMethod("getType", DocTreePath.class).invoke(docTrees, path);
-      } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
-        throw new RuntimeException(e);
-      }
+        try {
+            return (TypeMirror) DocTrees.class.getMethod("getType", DocTreePath.class).invoke(docTrees, path);
+        } catch (NoSuchMethodException | IllegalAccessException | InvocationTargetException e) {
+            throw new RuntimeException(e);
+        }
     }
-    
+
     @Override
     public List<DocTree> getFirstSentence(List<? extends DocTree> list) {
         return docTrees.getFirstSentence(list);

diff --git a/src/main/java/org/asciidoctor/asciidoclet/Asciidoclet.java b/src/main/java/org/asciidoctor/asciidoclet/Asciidoclet.java
@@ -21,8 +21,6 @@
 import jdk.javadoc.doclet.StandardDoclet;
 
 import javax.lang.model.SourceVersion;
-import java.io.IOException;
-import java.io.UncheckedIOException;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.Locale;
@@ -213,13 +211,11 @@ public SourceVersion getSupportedSourceVersion() {
 
     @Override
     public boolean run(DocletEnvironment environment) {
-        docletOptions.validateOptions();
+        docletOptions.validate();
         AsciidoctorConverter converter = new AsciidoctorConverter(docletOptions, reporter);
         boolean result;
         try (AsciidoctorFilteredEnvironment env = new AsciidoctorFilteredEnvironment(environment, converter)) {
             result = standardDoclet.run(env);
-        } catch (IOException e) {
-            throw new UncheckedIOException(e);
         }
         return result && postProcess(environment);
     }

diff --git a/src/main/java/org/asciidoctor/asciidoclet/AsciidoctorConverter.java b/src/main/java/org/asciidoctor/asciidoclet/AsciidoctorConverter.java
@@ -17,17 +17,9 @@
 
 import jdk.javadoc.doclet.Reporter;
 import org.asciidoctor.Asciidoctor;
-import org.asciidoctor.Attributes;
-import org.asciidoctor.AttributesBuilder;
 import org.asciidoctor.Options;
-import org.asciidoctor.OptionsBuilder;
-import org.asciidoctor.SafeMode;
-import org.asciidoctor.extension.RubyExtensionRegistry;
 import org.asciidoctor.jruby.AsciidoctorJRuby;
 
-import java.io.IOException;
-import java.util.List;
-import java.util.Optional;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -40,36 +32,20 @@ class AsciidoctorConverter {
 
     static final String MARKER = " \t \t";
 
-    private static AttributesBuilder defaultAttributes() {
-        return Attributes.builder()
-                .attribute("at", "&#64;")
-                .attribute("slash", "/")
-                .attribute("icons", null)
-                .attribute("idprefix", "")
-                .attribute("idseparator", "-")
-                .attribute("javadoc", "")
-                .attribute("showtitle", true)
-                .attribute("source-highlighter", "coderay")
-                .attribute("coderay-css", "class")
-                .attribute("env-asciidoclet")
-                .attribute("env", "asciidoclet");
-    }
-
-    private static OptionsBuilder defaultOptions() {
-        return Options.builder()
-                .safe(SafeMode.SAFE)
-                .backend("html5");
-    }
-
     private static final Pattern TYPE_PARAM = Pattern.compile("\\s*<(\\w+)>(.*)");
     private static final String INLINE_DOCTYPE = "inline";
 
+    private final DocletOptions docletOptions;
+    private final Reporter reporter;
+
     private final Asciidoctor asciidoctor;
-    private final Optional<OutputTemplates> templates;
-    private final Options options;
+    private final OutputTemplates templates;
 
     AsciidoctorConverter(DocletOptions docletOptions, Reporter reporter) {
-        this(docletOptions, reporter, OutputTemplates.create(reporter), createAsciidoctorInstance(docletOptions.gemPath()));
+        this.asciidoctor = createAsciidoctorInstance(docletOptions.gemPath());
+        this.reporter = reporter;
+        this.templates = OutputTemplates.create(reporter);
+        this.docletOptions = docletOptions;
     }
 
     private static Asciidoctor createAsciidoctorInstance(String gemPath) {
@@ -79,37 +55,6 @@ private static Asciidoctor createAsciidoctorInstance(String gemPath) {
         return Asciidoctor.Factory.create();
     }
 
-    /**
-     * Constructor used directly for testing purposes only.
-     */
-    AsciidoctorConverter(DocletOptions docletOptions, Reporter errorReporter, Optional<OutputTemplates> templates, Asciidoctor asciidoctor) {
-        this.asciidoctor = asciidoctor;
-        this.templates = templates;
-        this.options = buildOptions(docletOptions, errorReporter);
-    }
-
-    private Options buildOptions(DocletOptions docletOptions, Reporter errorReporter) {
-        final OptionsBuilder opts = defaultOptions();
-        if (docletOptions.baseDir().isPresent()) {
-            opts.baseDir(docletOptions.baseDir().get());
-        }
-        templates.ifPresent(outputTemplates -> opts.templateDir(outputTemplates.templateDir().toFile()));
-        opts.attributes(buildAttributes(docletOptions, errorReporter));
-        if (docletOptions.requires().size() > 0) {
-            RubyExtensionRegistry rubyExtensionRegistry = asciidoctor.rubyExtensionRegistry();
-            for (String require : docletOptions.requires()) {
-                rubyExtensionRegistry.requireLibrary(require);
-            }
-        }
-        return opts.get();
-    }
-
-    private Attributes buildAttributes(DocletOptions docletOptions, Reporter errorReporter) {
-        return defaultAttributes()
-                .attributes(new AttributesLoader(asciidoctor, docletOptions, errorReporter).load())
-                .get();
-    }
-
     /**
      * Converts a generic document (class, field, method, etc.).
      *
@@ -132,12 +77,6 @@ String convert(String doc) {
         return buffer.toString();
     }
 
-    void cleanup() throws IOException {
-        if (templates.isPresent()) {
-            templates.get().delete();
-        }
-    }
-
     /**
      * Renders a document tag in the standard way.
      *
@@ -181,21 +120,17 @@ private String convert(String input, boolean inline) {
         if (input.trim().isEmpty()) {
             return "";
         }
+        Options options = new AsciidoctorOptionsFactory(asciidoctor, reporter)
+                .create(docletOptions, templates);
         // Setting doctype to null results in an NPE from asciidoctor.
         // the default value from the command line is "article".
         // https://docs.asciidoctor.org/asciidoctor/latest/cli/man1/asciidoctor/#options
-        // In general, in order to respect original doctype, we should do the following.
-        // options.setDocType(inline ?
-        //    INLINE_DOCTYPE :
-        //    options.map().containsKey(Options.DOCTYPE) ?
-        //        (String)options.map().get(Options.DOCTYPE) :
-        //        "article");
-        // However, this fix breaks AsciidoctorConverterTest#testParameterWithoutTypeTag.
-        // For now, I simply set it to "article", always.
         options.setDocType(inline ?
-            INLINE_DOCTYPE :
-            "article"); // upstream sets this to "".
-
+                INLINE_DOCTYPE :
+                options.map().containsKey(Options.DOCTYPE) ?
+                        (String) options.map().get(Options.DOCTYPE) :
+                        "article");
+
         return asciidoctor.convert(cleanJavadocInput(input), options);
     }
 

diff --git a/src/main/java/org/asciidoctor/asciidoclet/AsciidoctorFilteredEnvironment.java b/src/main/java/org/asciidoctor/asciidoclet/AsciidoctorFilteredEnvironment.java
@@ -21,7 +21,6 @@
 
 import javax.tools.JavaFileManager;
 import javax.tools.StandardJavaFileManager;
-import java.io.IOException;
 
 /**
  * An operating environment defined for AsciiDoclet.
@@ -30,13 +29,11 @@ public class AsciidoctorFilteredEnvironment
         extends DocEnvImpl
         implements DocletEnvironment, AutoCloseable {
 
-    private final AsciidoctorConverter converter;
     private final StandardJavaFileManager fileManager;
     private final AsciiDocTrees asciiDocTrees;
 
     AsciidoctorFilteredEnvironment(DocletEnvironment environment, AsciidoctorConverter converter) {
         super(((DocEnvImpl) environment).toolEnv, ((DocEnvImpl) environment).etable);
-        this.converter = converter;
         this.fileManager = new AsciidoctorFileManager(converter, (StandardJavaFileManager) environment.getJavaFileManager());
         this.asciiDocTrees = new AsciiDocTrees(converter, fileManager, environment.getDocTrees());
     }
@@ -52,7 +49,6 @@ public DocTrees getDocTrees() {
     }
 
     @Override
-    public void close() throws IOException {
-        converter.cleanup();
+    public void close() {
     }
 }
diff --git a/src/main/java/org/asciidoctor/asciidoclet/AsciidoctorOptionsFactory.java b/src/main/java/org/asciidoctor/asciidoclet/AsciidoctorOptionsFactory.java
@@ -0,0 +1,70 @@
+package org.asciidoctor.asciidoclet;
+
+import jdk.javadoc.doclet.Reporter;
+import org.asciidoctor.Asciidoctor;
+import org.asciidoctor.Attributes;
+import org.asciidoctor.AttributesBuilder;
+import org.asciidoctor.Options;
+import org.asciidoctor.OptionsBuilder;
+import org.asciidoctor.SafeMode;
+import org.asciidoctor.extension.RubyExtensionRegistry;
+
+class AsciidoctorOptionsFactory {
+
+    private static final String DEFAULT_BACKEND = "html5";
+
+    private final Asciidoctor asciidoctor;
+    private final Reporter reporter;
+
+    AsciidoctorOptionsFactory(Asciidoctor asciidoctor, Reporter reporter) {
+        this.asciidoctor = asciidoctor;
+        this.reporter = reporter;
+    }
+
+    Options create(DocletOptions docletOptions, OutputTemplates templates) {
+        final OptionsBuilder opts = defaultOptions();
+        if (docletOptions.baseDir().isPresent()) {
+            opts.baseDir(docletOptions.baseDir().get());
+        }
+        if (templates != null) {
+            opts.templateDir(templates.templateDir().toFile());
+        }
+
+        opts.attributes(buildAttributes(docletOptions));
+        if (docletOptions.requires().size() > 0) {
+            RubyExtensionRegistry rubyExtensionRegistry = asciidoctor.rubyExtensionRegistry();
+            for (String require : docletOptions.requires()) {
+                rubyExtensionRegistry.requireLibrary(require);
+            }
+        }
+        return opts.get();
+    }
+
+    private Attributes buildAttributes(DocletOptions docletOptions) {
+        return defaultAttributes()
+                .attributes(new AttributesLoader(asciidoctor, docletOptions, reporter).load())
+                .get();
+    }
+
+    private static OptionsBuilder defaultOptions() {
+        return Options.builder()
+                .safe(SafeMode.SAFE)
+                .backend(DEFAULT_BACKEND);
+    }
+
+    private static AttributesBuilder defaultAttributes() {
+        return org.asciidoctor.Attributes.builder()
+                .attribute("at", "&#64;")
+                .attribute("slash", "/")
+                .attribute("icons", null)
+                .attribute("idprefix", "")
+                .attribute("idseparator", "-")
+                .attribute("javadoc", "")
+                .attribute("showtitle", true)
+                .attribute("source-highlighter", "coderay")
+                .attribute("coderay-css", "class")
+                .attribute("env-asciidoclet")
+                .attribute("env", "asciidoclet");
+    }
+
+}
diff --git a/src/main/java/org/asciidoctor/asciidoclet/AttributesLoader.java b/src/main/java/org/asciidoctor/asciidoclet/AttributesLoader.java
@@ -21,15 +21,11 @@
 import org.asciidoctor.Options;
 import org.asciidoctor.OptionsBuilder;
 import org.asciidoctor.SafeMode;
-import org.asciidoctor.jruby.internal.IOUtils;
 
 import javax.tools.Diagnostic;
 import java.io.File;
-import java.io.IOException;
 import java.io.Reader;
-import java.io.StringReader;
 import java.nio.file.Files;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
@@ -41,12 +37,12 @@ class AttributesLoader {
 
     private final Asciidoctor asciidoctor;
     private final DocletOptions docletOptions;
-    private final Reporter errorReporter;
+    private final Reporter reporter;
 
-    AttributesLoader(Asciidoctor asciidoctor, DocletOptions docletOptions, Reporter errorReporter) {
+    AttributesLoader(Asciidoctor asciidoctor, DocletOptions docletOptions, Reporter reporter) {
         this.asciidoctor = asciidoctor;
         this.docletOptions = docletOptions;
-        this.errorReporter = errorReporter;
+        this.reporter = reporter;
     }
 
     Map<String, Object> load() {
@@ -80,7 +76,7 @@ private Map<String, Object> parseAttributesFile(Optional<File> attrsFile, Map<St
             try (Reader reader = Files.newBufferedReader(attrsFile.get().toPath(), docletOptions.encoding())) {
                 return parseAttributes(reader, cmdlineAttrs);
             } catch (Exception e) {
-                errorReporter.print(Diagnostic.Kind.WARNING, "Cannot read attributes file: " + e);
+                reporter.print(Diagnostic.Kind.WARNING, "Cannot read attributes file: " + e);
             }
         }
         return cmdlineAttrs;
@@ -95,13 +91,11 @@ private Map<String, Object> parseAttributes(Reader in, Map<String, Object> exist
             options.baseDir(docletOptions.baseDir().get());
         }
 
-        final String content = read(in);
-        final Map<String, Object> parsed = asciidoctor.load(content, options.build()).getAttributes();
-        return parsed;
+        return asciidoctor.load(read(in), options.build()).getAttributes();
     }
 
     public static String read(Reader reader) {
-        try (Scanner scanner = new Scanner(reader).useDelimiter("\\A")){
+        try (Scanner scanner = new Scanner(reader).useDelimiter("\\A")) {
             return scanner.next();
         }
     }

diff --git a/src/main/java/org/asciidoctor/asciidoclet/DocletOptions.java b/src/main/java/org/asciidoctor/asciidoclet/DocletOptions.java
@@ -89,7 +89,7 @@ private Stream<String> splitTrimStream(List<String> list) {
                 .filter(s -> !s.isEmpty());
     }
 
-    void validateOptions() {
+    void validate() {
         if (baseDir().isEmpty()) {
             printWarning(AsciidocletOptions.BASEDIR + " must be present for includes or file reference features to work properly");
         }