This is an automated email from the ASF dual-hosted git repository. kwin pushed a commit to branch bugfix/improve-error-reporting-of-doclet in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
commit ee2bf8fa1ef2e52a1ff319d03b248ace08ec96a9 Author: Konrad Windszus <[email protected]> AuthorDate: Mon Jul 20 14:08:58 2026 +0200 Improve error reporting Add unit tests to check error reporting --- .../aether/tools/ConfigurationCollectorDoclet.java | 145 ++++++++++----------- .../tools/DocTreePathAwareRuntimeException.java | 41 ++++++ .../tools/ConfigurationCollectorDocletTest.java | 121 +++++++++++++++-- .../sample/InvalidSampleConfigurationKeys.java | 55 ++++++++ 4 files changed, 272 insertions(+), 90 deletions(-) diff --git a/maven-resolver-tools/src/main/java/org/eclipse/aether/tools/ConfigurationCollectorDoclet.java b/maven-resolver-tools/src/main/java/org/eclipse/aether/tools/ConfigurationCollectorDoclet.java index 7aea2fe2a..12b84e387 100644 --- a/maven-resolver-tools/src/main/java/org/eclipse/aether/tools/ConfigurationCollectorDoclet.java +++ b/maven-resolver-tools/src/main/java/org/eclipse/aether/tools/ConfigurationCollectorDoclet.java @@ -30,7 +30,6 @@ import javax.lang.model.util.ElementFilter; import javax.tools.Diagnostic; import java.io.IOException; -import java.io.UncheckedIOException; import java.io.Writer; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -159,39 +158,28 @@ public class ConfigurationCollectorDoclet implements Doclet { } else if ("resolver".equals(mode)) { processResolverField(type, field, docComment, discoveredKeys); } else { - reportError(field, "Unknown mode: " + mode); + // TODO: move to beginning of run() and validate mode before processing any types + reportError("Unknown mode: " + mode); + return false; } - } catch (IllegalArgumentException e) { - reportError(field, e.getMessage()); + } catch (DocTreePathAwareRuntimeException e) { + reportError(e.getDocTreePath(), e.getMessage()); } catch (RuntimeException e) { - reportError(field, "Failed to process: " + e.getMessage()); + DocTreePath rootPath = new DocTreePath(docTrees.getPath(field), docComment); + reportError(rootPath, e.getMessage()); } } } try { writeProperties(discoveredKeys); - } catch (UncheckedIOException e) { + } catch (IOException e) { reportError("Failed to write properties file: " + e.getMessage()); return false; } return true; } - /** - * Reports an error message at a specific element location. - * - * @param element the element where the error occurred - * @param message the error message - */ - private void reportError(Element element, String message) { - if (element != null) { - reporter.print(Diagnostic.Kind.ERROR, element, message); - } else { - reportError(message); - } - } - /** * Reports an error message at a specific DocTreePath location. * @@ -220,30 +208,34 @@ public class ConfigurationCollectorDoclet implements Doclet { if (docComment == null) { return; } - Map<String, List<? extends DocTree>> blockTags = collectBlockTags(docComment); + Map<String, UnknownBlockTagTree> blockTags = collectBlockTags(docComment); if (!blockTags.containsKey("configurationSource")) { return; } - String configurationType = getConfigurationType( - extractClassLink(field, docComment, blockTags, "configurationType")); - String defValue = - resolveDefaultValue(type, field, docComment, blockTags.get("configurationDefaultValue")); + String configurationType = + getConfigurationType(extractClassLink(field, docComment, blockTags, "configurationType")); + String defValue = resolveDefaultValue(type, field, docComment, blockTags.get("configurationDefaultValue")); if (defValue == null) { // Error was already reported, skip this field return; } + UnknownBlockTagTree sourceTag = blockTags.get("configurationSource"); + UnknownBlockTagTree repoIdTag = blockTags.get("configurationRepoIdSuffix"); + 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( + "configurationSource", + getConfigurationSource(renderContent(sourceTag != null ? sourceTag.getContent() : null))); entry.put("configurationType", configurationType); - entry.put("supportRepoIdSuffix", toYesNo(renderContent(blockTags.get("configurationRepoIdSuffix")))); + entry.put("supportRepoIdSuffix", toYesNo(renderContent(repoIdTag != null ? repoIdTag.getContent() : null))); discovered.add(entry); } @@ -332,7 +324,7 @@ public class ConfigurationCollectorDoclet implements Doclet { return null; } - private void writeProperties(List<Map<String, String>> discoveredKeys) { + private void writeProperties(List<Map<String, String>> discoveredKeys) throws IOException { Properties properties = new Properties(); properties.setProperty("keys.count", String.valueOf(discoveredKeys.size())); for (int i = 0; i < discoveredKeys.size(); i++) { @@ -341,38 +333,48 @@ public class ConfigurationCollectorDoclet implements Doclet { 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); + 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"); } } // --- Javadoc extraction helpers ------------------------------------------------------------------------------- - private Map<String, List<? extends DocTree>> collectBlockTags(DocCommentTree docComment) { - Map<String, List<? extends DocTree>> result = new LinkedHashMap<>(); + private Map<String, UnknownBlockTagTree> collectBlockTags(DocCommentTree docComment) { + Map<String, UnknownBlockTagTree> result = new LinkedHashMap<>(); for (DocTree tag : docComment.getBlockTags()) { if (tag instanceof UnknownBlockTagTree unknownBlockTree) { - result.put(unknownBlockTree.getTagName(), unknownBlockTree.getContent()); + result.put(unknownBlockTree.getTagName(), unknownBlockTree); } } return result; } + /** + * Builds a {@link DocTreePath} pointing to a specific block tag within a field's doc comment, enabling + * precise Javadoc-level error messages via {@link Reporter#print(Diagnostic.Kind, DocTreePath, String)}. + */ + private DocTreePath buildTagPath(VariableElement field, DocCommentTree docComment, UnknownBlockTagTree tag) { + if (field == null || docComment == null || tag == null) { + return null; + } + DocTreePath docCommentPath = new DocTreePath(docTrees.getPath(field), docComment); + return new DocTreePath(docCommentPath, tag); + } + private String resolveDefaultValue( - TypeElement type, - VariableElement contextField, - DocCommentTree docComment, - List<? extends DocTree> content) { - if (content == null || content.isEmpty()) { + TypeElement type, VariableElement contextField, DocCommentTree docComment, UnknownBlockTagTree contentTag) { + if (contentTag == null) { return null; } + List<? extends DocTree> content = contentTag.getContent(); + if (content.isEmpty()) { + return null; + } + DocTreePath tagPath = buildTagPath(contextField, docComment, contentTag); for (DocTree tree : content) { if (tree instanceof LinkTree link) { if (link.getReference() != null) { @@ -384,20 +386,12 @@ public class ConfigurationCollectorDoclet implements Doclet { ? lookupConstant(referenced) : lookupConstant(type, signature.substring(signature.indexOf('#') + 1)); if (value == null) { - // hard fail: default value constants must be resolvable - DocTreePath linkPath = resolveDocTreePath(contextField, docComment, link); - if (linkPath != null) { - reportError( - linkPath, - "Could not look up {@link " + signature + "} for configuration " - + type.getQualifiedName()); - } else { - reportError( - contextField, - "Could not look up {@link " + signature + "} for configuration " - + type.getQualifiedName()); - } - throw new IllegalArgumentException("Could not resolve link: " + signature); + // hard fail: default value constants must be resolvable; report at the precise + // link-reference location if we can resolve a path to it, otherwise at the block tag + DocTreePath rootPath = new DocTreePath(docTrees.getPath(contextField), docComment); + DocTreePath linkRefPath = DocTreePath.getPath(rootPath, link.getReference()); + throw new DocTreePathAwareRuntimeException( + linkRefPath != null ? linkRefPath : tagPath, "Could not resolve link: " + signature); } return value; } @@ -425,18 +419,6 @@ public class ConfigurationCollectorDoclet implements Doclet { return element instanceof VariableElement variableElement ? variableElement : null; } - /** - * Resolves the {@link DocTreePath} for a {@code {@link ...}} reference. Returns {@code null} if the path cannot - * be resolved. - */ - private DocTreePath resolveDocTreePath(VariableElement contextField, DocCommentTree docComment, LinkTree link) { - if (contextField == null || docComment == null) { - return null; - } - DocTreePath rootPath = new DocTreePath(docTrees.getPath(contextField), docComment); - return DocTreePath.getPath(rootPath, link.getReference()); - } - private String lookupConstant(TypeElement type, String constantName) { for (VariableElement field : ElementFilter.fieldsIn(type.getEnclosedElements())) { if (field.getSimpleName().contentEquals(constantName)) { @@ -492,23 +474,32 @@ public class ConfigurationCollectorDoclet implements Doclet { } private String extractClassLink( - VariableElement contextField, DocCommentTree docComment, Map<String, List<? extends DocTree>> blockTags, String tagName) { - List<? extends DocTree> content = blockTags.get(tagName); - if (content == null || content.isEmpty()) { + VariableElement contextField, + DocCommentTree docComment, + Map<String, UnknownBlockTagTree> blockTags, + String tagName) { + UnknownBlockTagTree tag = blockTags.get(tagName); + if (tag == null || tag.getContent().isEmpty()) { + throw new IllegalArgumentException("Missing content for @" + tagName); } - for (DocTree tree : content) { + DocTreePath tagPath = buildTagPath(contextField, docComment, tag); + for (DocTree tree : tag.getContent()) { // just use the first link, ignore any other content (e.g. text) in the tag if (tree instanceof LinkTree link) { String signature = link.getReference().getSignature(); if (signature.contains("#")) { - throw new IllegalArgumentException( + // report at the precise link reference node within the block tag + DocTreePath rootPath = new DocTreePath(docTrees.getPath(contextField), docComment); + DocTreePath linkRefPath = DocTreePath.getPath(rootPath, link.getReference()); + throw new DocTreePathAwareRuntimeException( + linkRefPath != null ? linkRefPath : tagPath, "Expected a class link in @" + tagName + ", but got a member reference: " + signature); } return resolveReferencedType(contextField, docComment, link, signature); } } - throw new IllegalArgumentException("No valid {@link ...} reference found in @" + tagName); + throw new DocTreePathAwareRuntimeException(tagPath, "No valid {@link ...} reference found in @" + tagName); } /** diff --git a/maven-resolver-tools/src/main/java/org/eclipse/aether/tools/DocTreePathAwareRuntimeException.java b/maven-resolver-tools/src/main/java/org/eclipse/aether/tools/DocTreePathAwareRuntimeException.java new file mode 100644 index 000000000..ea3d9bd9b --- /dev/null +++ b/maven-resolver-tools/src/main/java/org/eclipse/aether/tools/DocTreePathAwareRuntimeException.java @@ -0,0 +1,41 @@ +/* + * 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 com.sun.source.util.DocTreePath; + +public class DocTreePathAwareRuntimeException extends RuntimeException { + + private static final long serialVersionUID = -4295354135012887795L; + private final DocTreePath docTreePath; + + public DocTreePathAwareRuntimeException(DocTreePath docTreePath, String message) { + super(message); + this.docTreePath = docTreePath; + } + + public DocTreePathAwareRuntimeException(DocTreePath docTreePath, String message, Throwable cause) { + super(message, cause); + this.docTreePath = docTreePath; + } + + public DocTreePath getDocTreePath() { + return docTreePath; + } +} diff --git a/maven-resolver-tools/src/test/java/org/eclipse/aether/tools/ConfigurationCollectorDocletTest.java b/maven-resolver-tools/src/test/java/org/eclipse/aether/tools/ConfigurationCollectorDocletTest.java index 238360b08..e315ba560 100644 --- a/maven-resolver-tools/src/test/java/org/eclipse/aether/tools/ConfigurationCollectorDocletTest.java +++ b/maven-resolver-tools/src/test/java/org/eclipse/aether/tools/ConfigurationCollectorDocletTest.java @@ -18,6 +18,8 @@ */ package org.eclipse.aether.tools; +import javax.tools.Diagnostic; +import javax.tools.DiagnosticListener; import javax.tools.DocumentationTool; import javax.tools.JavaFileObject; import javax.tools.StandardJavaFileManager; @@ -26,19 +28,24 @@ import javax.tools.ToolProvider; import java.io.InputStream; import java.io.Reader; import java.io.StringWriter; +import java.io.Writer; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Properties; +import org.junit.jupiter.api.BeforeEach; 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.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -50,17 +57,37 @@ class ConfigurationCollectorDocletTest { */ 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); + /** + * Classpath location of the a fixture with invalid javadoc (missing/invalid elements). + */ + private static final String INVALID_FIXTURE = "/org/eclipse/aether/sample/InvalidSampleConfigurationKeys.java"; + + private Path output; + + @BeforeEach + void setUp(@TempDir Path tempDir) { + output = tempDir.resolve("configuration-keys.properties"); + } + + private Path getSourceFile(String resourcePath, Path tempDir) throws Exception { + if (!resourcePath.startsWith("/")) { + throw new IllegalArgumentException("resource path must start with '/': " + resourcePath); + } + Path sourceDir = + Files.createDirectories(tempDir.resolve(resourcePath.substring(1, resourcePath.lastIndexOf('/')))); + Path sourceFile = sourceDir.resolve(resourcePath.substring(resourcePath.lastIndexOf('/') + 1)); + try (InputStream in = ConfigurationCollectorDocletTest.class.getResourceAsStream(resourcePath)) { + assertNotNull(in, "resource path not found on classpath: " + resourcePath); Files.copy(in, sourceFile); } - Path output = tempDir.resolve("configuration-keys.properties"); + return sourceFile; + } - runDoclet(sourceFile, output); + @Test + void extractsBooleanStringAndEnumConfigurations(@TempDir Path tempDir) throws Exception { + StringWriter out = new StringWriter(); + assertTrue( + runDoclet(out, getSourceFile(FIXTURE, tempDir), output), "doclet run should succeed, output:\n" + out); Map<String, Map<String, String>> keys = readKeys(output); assertEquals(4, keys.size(), "expected four configuration keys"); @@ -96,17 +123,85 @@ class ConfigurationCollectorDocletTest { assertEquals("No", enum2Key.get("supportRepoIdSuffix")); } - private static void runDoclet(Path sourceFile, Path output) throws Exception { + static final class CapturingDiagnosticsListener<T extends JavaFileObject> + implements javax.tools.DiagnosticListener<T> { + private final javax.tools.Diagnostic.Kind threshold; + private final Collection<Diagnostic<? extends JavaFileObject>> diagnostics = new ArrayList<>(); + + public CapturingDiagnosticsListener(javax.tools.Diagnostic.Kind threshold) { + this.threshold = threshold; + } + + @Override + public void report(javax.tools.Diagnostic<? extends T> diagnostic) { + if (diagnostic.getKind().compareTo(threshold) <= 0) { + diagnostics.add(diagnostic); + } + } + + public Collection<Diagnostic<? extends JavaFileObject>> getDiagnostics() { + return diagnostics; + } + } + + @Test + void invalidMode() throws Exception { + CapturingDiagnosticsListener<JavaFileObject> listener = + new CapturingDiagnosticsListener<>(javax.tools.Diagnostic.Kind.ERROR); + StringWriter out = new StringWriter(); + assertFalse(runDoclet(out, getSourceFile(FIXTURE, output.getParent()), output, "invalid-mode", listener)); + // check that the diagnostics contain an error message about the invalid mode + Diagnostic<? extends JavaFileObject> diagnostic = + listener.getDiagnostics().iterator().next(); + assertEquals(javax.tools.Diagnostic.Kind.ERROR, diagnostic.getKind()); + assertEquals("Unknown mode: invalid-mode", diagnostic.getMessage(null)); + assertEquals(1, listener.getDiagnostics().size(), "expected one error diagnostic"); + } + + @Test + void invalidTaglets() throws Exception { + CapturingDiagnosticsListener<JavaFileObject> listener = + new CapturingDiagnosticsListener<>(javax.tools.Diagnostic.Kind.ERROR); + StringWriter out = new StringWriter(); + Path sourceFile = getSourceFile(INVALID_FIXTURE, output.getParent()); + assertFalse(runDoclet(out, sourceFile, output, "resolver", listener)); + // check that the diagnostics contain two error messages + Iterator<Diagnostic<? extends JavaFileObject>> iterator = + listener.getDiagnostics().iterator(); + Diagnostic<? extends JavaFileObject> diagnostic = iterator.next(); + assertEquals(javax.tools.Diagnostic.Kind.ERROR, diagnostic.getKind()); + assertEquals("Missing content for @configurationType", diagnostic.getMessage(null)); + assertEquals(sourceFile.toString(), diagnostic.getSource().getName()); + assertEquals(30, diagnostic.getLineNumber()); + assertEquals(2, listener.getDiagnostics().size(), "expected two error diagnostics"); + diagnostic = iterator.next(); + assertEquals(javax.tools.Diagnostic.Kind.ERROR, diagnostic.getKind()); + assertEquals("No valid {@link ...} reference found in @configurationType", diagnostic.getMessage(null)); + assertEquals(sourceFile.toString(), diagnostic.getSource().getName()); + assertEquals(46, diagnostic.getLineNumber()); + } + + private static Boolean runDoclet(Writer writer, Path sourceFile, Path output) throws Exception { + return runDoclet(writer, sourceFile, output, null, null); + } + + private static Boolean runDoclet( + Writer writer, Path sourceFile, Path output, String mode, DiagnosticListener<JavaFileObject> listener) + throws Exception { DocumentationTool documentationTool = ToolProvider.getSystemDocumentationTool(); try (StandardJavaFileManager fileManager = documentationTool.getStandardFileManager(null, null, StandardCharsets.UTF_8)) { Iterable<? extends JavaFileObject> units = fileManager.getJavaFileObjectsFromFiles(List.of(sourceFile.toFile())); - List<String> options = List.of("--output", output.toString(), "-encoding", "UTF-8"); - StringWriter out = new StringWriter(); + final List<String> options; + if (mode != null) { + options = List.of("--output", output.toString(), "--mode", mode, "-encoding", "UTF-8"); + } else { + options = List.of("--output", output.toString(), "-encoding", "UTF-8"); + } DocumentationTool.DocumentationTask task = documentationTool.getTask( - out, fileManager, null, ConfigurationCollectorDoclet.class, options, units); - assertTrue(task.call(), "doclet run should succeed, output:\n" + out); + writer, fileManager, listener, ConfigurationCollectorDoclet.class, options, units); + return task.call(); } } diff --git a/maven-resolver-tools/src/test/resources/org/eclipse/aether/sample/InvalidSampleConfigurationKeys.java b/maven-resolver-tools/src/test/resources/org/eclipse/aether/sample/InvalidSampleConfigurationKeys.java new file mode 100644 index 000000000..c01a3c295 --- /dev/null +++ b/maven-resolver-tools/src/test/resources/org/eclipse/aether/sample/InvalidSampleConfigurationKeys.java @@ -0,0 +1,55 @@ +/* + * 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.sample; + + +/** + * Sample source declaring configuration keys of type {@link Boolean}, {@link String} and a custom enum, using the same + * Javadoc block tags that {@code ConfigurationCollectorDoclet} extracts. Used as a fixture by the doclet test. + */ +public final class InvalidSampleConfigurationKeys { + + // missing mandatory @configurationType tag, should be reported as an error by the doclet + /** + * A boolean flag. + * + * @since 1.2.3 + * @configurationSource {@link System#getProperty(String,String)} + * @configurationDefaultValue {@link #DEFAULT_BOOL} + * @configurationRepoIdSuffix No + */ + public static final String BOOL_KEY = "sample.bool"; + + public static final boolean DEFAULT_BOOL = true; + + // invalid @configurationType tag value, should be reported as an error by the doclet + /** + * A string value. + * + * @configurationSource {@link System#getProperty(String,String)} + * @configurationType invalid + * @configurationDefaultValue {@link #DEFAULT_STRING} + * @configurationRepoIdSuffix Yes + */ + public static final String STRING_KEY = "sample.string"; + + public static final String DEFAULT_STRING = "hello"; + + private InvalidSampleConfigurationKeys() {} +}
