gnodet commented on code in PR #1965: URL: https://github.com/apache/maven-resolver/pull/1965#discussion_r3594685409
########## maven-resolver-tools/src/test/java/org/eclipse/aether/tools/ConfigurationCollectorDocletTest.java: ########## @@ -0,0 +1,142 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.eclipse.aether.tools; + +import javax.tools.DocumentationTool; +import javax.tools.JavaFileObject; +import javax.tools.StandardJavaFileManager; +import javax.tools.ToolProvider; + +import java.io.InputStream; +import java.io.Reader; +import java.io.StringWriter; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.Properties; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class ConfigurationCollectorDocletTest { + + /** + * Classpath location of the fixture source declaring configuration keys of type {@link Boolean}, {@link String} + * and a custom enum, using the same Javadoc block tags that the doclet extracts. + */ + private static final String FIXTURE = "/org/eclipse/aether/sample/SampleConfigurationKeys.java"; + + @Test + void extractsBooleanStringAndEnumConfigurations(@TempDir Path tempDir) throws Exception { + Path sourceDir = Files.createDirectories(tempDir.resolve("org/eclipse/aether/sample")); + Path sourceFile = sourceDir.resolve("SampleConfigurationKeys.java"); + try (InputStream in = ConfigurationCollectorDocletTest.class.getResourceAsStream(FIXTURE)) { + assertNotNull(in, "fixture source not found on classpath: " + FIXTURE); + Files.copy(in, sourceFile); + } + Path output = tempDir.resolve("configuration-keys.properties"); + + runDoclet(sourceFile, output); + + Map<String, Map<String, String>> keys = readKeys(output); + assertEquals(4, keys.size(), "expected three configuration keys"); Review Comment: Nit: the assertion expects 4 keys but the failure message still says "three": ```suggestion assertEquals(4, keys.size(), "expected four configuration keys"); ``` ########## maven-resolver-tools/src/main/java/org/eclipse/aether/tools/ConfigurationCollectorDoclet.java: ########## @@ -0,0 +1,478 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.eclipse.aether.tools; + +import javax.lang.model.SourceVersion; +import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; +import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; +import javax.lang.model.util.ElementFilter; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.io.Writer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Locale; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import com.sun.source.doctree.DocCommentTree; +import com.sun.source.doctree.DocTree; +import com.sun.source.doctree.LinkTree; +import com.sun.source.doctree.LiteralTree; +import com.sun.source.doctree.SinceTree; +import com.sun.source.doctree.TextTree; +import com.sun.source.doctree.UnknownBlockTagTree; +import com.sun.source.tree.ExpressionTree; +import com.sun.source.tree.IdentifierTree; +import com.sun.source.tree.MemberSelectTree; +import com.sun.source.tree.VariableTree; +import com.sun.source.util.DocTreePath; +import com.sun.source.util.DocTrees; +import jdk.javadoc.doclet.Doclet; +import jdk.javadoc.doclet.DocletEnvironment; +import jdk.javadoc.doclet.Reporter; + +/** + * A custom Javadoc {@link Doclet} that scans constant fields for configuration metadata declared via custom Javadoc + * block tags (e.g. {@code @configurationSource}) and writes the discovered keys into an intermediate + * {@link Properties} file. That file is subsequently consumed by {@link CollectConfiguration} to render the + * documentation via Velocity templates. + * <p> + * The intermediate file uses an indexed layout: + * <pre> + * keys.count=N + * keys.0.key=... + * keys.0.description=... + * ... + * </pre> + */ +public class ConfigurationCollectorDoclet implements Doclet { + + private Path output; + private DocTrees docTrees; + + @Override + public void init(Locale locale, Reporter reporter) { + // no state to initialize + } + + @Override + public String getName() { + return "ConfigurationCollector"; + } + + @Override + public Set<? extends Option> getSupportedOptions() { + return Set.of(new SimpleOption( + List.of("--output", "-o"), + 1, + "The intermediate properties file to write discovered keys to", + "<file>", + args -> output = Paths.get(args.get(0)))); + } + + @Override + public SourceVersion getSupportedSourceVersion() { + return SourceVersion.latest(); + } + + @Override + public boolean run(DocletEnvironment environment) { + try { + return doRun(environment); + } catch (RuntimeException e) { + e.printStackTrace(System.err); + return false; + } + } + + private boolean doRun(DocletEnvironment environment) { + if (output == null) { + throw new IllegalStateException("Missing required --output option"); + } + docTrees = environment.getDocTrees(); + List<Map<String, String>> discoveredKeys = new ArrayList<>(); + + Set<TypeElement> types = ElementFilter.typesIn(environment.getIncludedElements()); + for (TypeElement type : types) { + for (VariableElement field : ElementFilter.fieldsIn(type.getEnclosedElements())) { + if (field.getConstantValue() == null) { + continue; + } + DocCommentTree docComment = docTrees.getDocCommentTree(field); + processField(type, field, docComment, discoveredKeys); + } + } + + writeProperties(discoveredKeys); + return true; + } + + private void processField( + TypeElement type, VariableElement field, DocCommentTree docComment, List<Map<String, String>> discovered) { + if (docComment == null) { + return; + } + Map<String, List<? extends DocTree>> blockTags = collectBlockTags(docComment); + if (!blockTags.containsKey("configurationSource")) { + return; + } + + String configurationType = getConfigurationType(renderContent(blockTags.get("configurationType"))); + String defValue = resolveDefaultValue(type, field, docComment, blockTags.get("configurationDefaultValue")); + + Map<String, String> entry = new LinkedHashMap<>(); + entry.put("key", String.valueOf(field.getConstantValue())); + entry.put("defaultValue", nvl(defValue, "")); + entry.put("fqName", type.getQualifiedName() + "." + field.getSimpleName()); + entry.put("description", cleanseJavadoc(renderContent(docComment.getFullBody()))); + entry.put("since", nvl(getSince(type, docComment), "")); + entry.put("configurationSource", getConfigurationSource(renderContent(blockTags.get("configurationSource")))); + entry.put("configurationType", configurationType); + entry.put("supportRepoIdSuffix", toYesNo(renderContent(blockTags.get("configurationRepoIdSuffix")))); + discovered.add(entry); + } + + private void writeProperties(List<Map<String, String>> discoveredKeys) { + Properties properties = new Properties(); + properties.setProperty("keys.count", String.valueOf(discoveredKeys.size())); + for (int i = 0; i < discoveredKeys.size(); i++) { + Map<String, String> entry = discoveredKeys.get(i); + for (Map.Entry<String, String> field : entry.entrySet()) { + properties.setProperty("keys." + i + "." + field.getKey(), field.getValue()); + } + } + try { + if (output.getParent() != null) { + Files.createDirectories(output.getParent()); + } + try (Writer writer = Files.newBufferedWriter(output, StandardCharsets.UTF_8)) { + properties.store(writer, "Generated by ConfigurationCollectorDoclet - DO NOT EDIT"); + } + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + // --- Javadoc extraction helpers ------------------------------------------------------------------------------- + + private Map<String, List<? extends DocTree>> collectBlockTags(DocCommentTree docComment) { + Map<String, List<? extends DocTree>> result = new LinkedHashMap<>(); + for (DocTree tag : docComment.getBlockTags()) { + if (tag instanceof UnknownBlockTagTree) { + UnknownBlockTagTree unknown = (UnknownBlockTagTree) tag; + result.put(unknown.getTagName(), unknown.getContent()); + } + } + return result; + } + + private String resolveDefaultValue( + TypeElement type, + VariableElement contextField, + DocCommentTree docComment, + List<? extends DocTree> content) { + if (content == null || content.isEmpty()) { + return null; + } + for (DocTree tree : content) { + if (tree instanceof LinkTree) { + LinkTree link = (LinkTree) tree; + if (link.getReference() != null) { + String signature = link.getReference().getSignature(); + // resolve the referenced constant using the fully qualified signature, so that references + // to constants declared in other types (e.g. {@link OtherType#CONSTANT}) can be resolved + VariableElement referenced = resolveReferencedField(contextField, docComment, link); + String value = referenced != null + ? lookupConstant(referenced) + : lookupConstant(type, signature.substring(signature.indexOf('#') + 1)); + if (value == null) { + // hard fail as in the original implementation: default value constants must be resolvable + throw new IllegalArgumentException("Could not look up {@link " + signature + + "} for configuration " + type.getQualifiedName()); + } + return value; + } + } + } + return renderContent(content); + } + + /** + * Resolves the {@link VariableElement} a {@code {@link ...}} reference points to using the fully qualified + * signature (so references into other types are supported). Returns {@code null} if the reference cannot be + * resolved to a field. + */ + private VariableElement resolveReferencedField( + VariableElement contextField, DocCommentTree docComment, LinkTree link) { + if (contextField == null || docComment == null) { + return null; + } + DocTreePath rootPath = new DocTreePath(docTrees.getPath(contextField), docComment); + DocTreePath refPath = DocTreePath.getPath(rootPath, link.getReference()); + if (refPath == null) { + return null; + } + Element element = docTrees.getElement(refPath); + return element instanceof VariableElement ? (VariableElement) element : null; + } + + private String lookupConstant(TypeElement type, String constantName) { + for (VariableElement field : ElementFilter.fieldsIn(type.getEnclosedElements())) { + if (field.getSimpleName().contentEquals(constantName)) { + String value = lookupConstant(field); + if (value != null) { + return value; + } + } + } + return null; + } + + private String lookupConstant(VariableElement field) { + if (field.getConstantValue() != null) { + return String.valueOf(field.getConstantValue()); + } + // enum constants don't expose a constant value, fall back to the enum value's name + if (field.getKind() == ElementKind.ENUM_CONSTANT) { + return createQualifiedEnumValueConstant(field, field.getSimpleName().toString()); + } + // the field may indirectly reference an enum variable, e.g. "SomeEnum.VALUE"; + // resolve it from the field's initializer + return resolveEnumReference(field); + } + + /** + * Resolves an enum constant that a field is initialized with, including the enum type in the result + * (e.g. a field declared as {@code SomeEnum FOO = SomeEnum.VALUE} resolves to {@code SomeEnum.VALUE}). + * Returns {@code null} if the field's initializer is not a simple enum reference. + */ + private String resolveEnumReference(VariableElement field) { + if (!(docTrees.getTree(field) instanceof VariableTree)) { + return null; + } + ExpressionTree initializer = ((VariableTree) docTrees.getTree(field)).getInitializer(); Review Comment: Minor: `docTrees.getTree(field)` is called twice — once for the `instanceof` guard and again for the cast. Since the project targets Java 17+, pattern matching `instanceof` eliminates the redundant call: ```suggestion private String resolveEnumReference(VariableElement field) { if (!(docTrees.getTree(field) instanceof VariableTree variableTree)) { return null; } ExpressionTree initializer = variableTree.getInitializer(); ``` -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
