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 75a7401e4..36e6b7724 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.tools.Diagnostic; import java.io.IOException; -import java.io.UncheckedIOException; import java.io.Writer; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -133,15 +132,15 @@ public boolean run(DocletEnvironment environment) { try { return doRun(environment); } catch (RuntimeException e) { - reporter.print(Diagnostic.Kind.ERROR, "Error running ConfigurationCollectorDoclet: " + e.getMessage()); - e.printStackTrace(reporter.getStandardWriter()); + reportError("Error running ConfigurationCollectorDoclet: " + e.getMessage()); return false; } } private boolean doRun(DocletEnvironment environment) { if (output == null) { - throw new IllegalStateException("Missing required --output option"); + reportError("Missing required --output option"); + return false; } docTrees = environment.getDocTrees(); List> discoveredKeys = new ArrayList<>(); @@ -153,43 +152,85 @@ private boolean doRun(DocletEnvironment environment) { continue; } DocCommentTree docComment = docTrees.getDocCommentTree(field); - if ("maven".equals(mode)) { - processMavenField(type, field, docComment, discoveredKeys); - } else if ("resolver".equals(mode)) { - processResolverField(type, field, docComment, discoveredKeys); - } else { - throw new IllegalArgumentException("Unknown mode: " + mode); + try { + if ("maven".equals(mode)) { + processMavenField(type, field, docComment, discoveredKeys); + } else if ("resolver".equals(mode)) { + processResolverField(type, field, docComment, discoveredKeys); + } else { + // TODO: move to beginning of run() and validate mode before processing any types + reportError("Unknown mode: " + mode); + return false; + } + } catch (DocTreePathAwareRuntimeException e) { + reportError(e.getDocTreePath(), e.getMessage()); + } catch (RuntimeException e) { + DocTreePath rootPath = new DocTreePath(docTrees.getPath(field), docComment); + reportError(rootPath, e.getMessage()); } } } - writeProperties(discoveredKeys); + try { + writeProperties(discoveredKeys); + } catch (IOException e) { + reportError("Failed to write properties file: " + e.getMessage()); + return false; + } return true; } + /** + * Reports an error message at a specific DocTreePath location. + * + * @param path the DocTreePath where the error occurred + * @param message the error message + */ + private void reportError(DocTreePath path, String message) { + if (path != null) { + reporter.print(Diagnostic.Kind.ERROR, path, message); + } else { + reportError(message); + } + } + + /** + * Reports a global error message without location information. + * + * @param message the error message + */ + private void reportError(String message) { + reporter.print(Diagnostic.Kind.ERROR, message); + } + private void processResolverField( TypeElement type, VariableElement field, DocCommentTree docComment, List> discovered) { if (docComment == null) { return; } - Map> blockTags = collectBlockTags(docComment); + Map blockTags = collectBlockTags(docComment); if (!blockTags.containsKey("configurationSource")) { return; } String configurationType = - getConfigurationType(extractClassLink(field, docComment, blockTags.get("configurationType"))); + getConfigurationType(extractClassLink(field, docComment, blockTags, "configurationType")); String defValue = resolveDefaultValue(type, field, docComment, blockTags.get("configurationDefaultValue")); + UnknownBlockTagTree sourceTag = blockTags.get("configurationSource"); + UnknownBlockTagTree repoIdTag = blockTags.get("configurationRepoIdSuffix"); + Map 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); } @@ -216,8 +257,8 @@ private void processMavenField( Object value = attribute.getValue().getValue(); switch (name) { case "source": - source = value instanceof VariableElement - ? ((VariableElement) value).getSimpleName().toString() + source = value instanceof VariableElement variableElement + ? variableElement.getSimpleName().toString() : String.valueOf(value); break; case "defaultValue": @@ -278,7 +319,7 @@ private AnnotationMirror getAnnotation(Element element, String fqName) { return null; } - private void writeProperties(List> discoveredKeys) { + private void writeProperties(List> discoveredKeys) throws IOException { Properties properties = new Properties(); properties.setProperty("keys.count", String.valueOf(discoveredKeys.size())); for (int i = 0; i < discoveredKeys.size(); i++) { @@ -287,42 +328,50 @@ private void writeProperties(List> discoveredKeys) { 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> collectBlockTags(DocCommentTree docComment) { - Map> result = new LinkedHashMap<>(); + private Map collectBlockTags(DocCommentTree docComment) { + Map result = new LinkedHashMap<>(); for (DocTree tag : docComment.getBlockTags()) { - if (tag instanceof UnknownBlockTagTree) { - UnknownBlockTagTree unknown = (UnknownBlockTagTree) tag; - result.put(unknown.getTagName(), unknown.getContent()); + if (tag instanceof UnknownBlockTagTree unknownBlockTree) { + 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 content) { - if (content == null || content.isEmpty()) { + TypeElement type, VariableElement contextField, DocCommentTree docComment, UnknownBlockTagTree contentTag) { + if (contentTag == null) { + return null; + } + List content = contentTag.getContent(); + if (content.isEmpty()) { return null; } + DocTreePath tagPath = buildTagPath(contextField, docComment, contentTag); for (DocTree tree : content) { - if (tree instanceof LinkTree) { - LinkTree link = (LinkTree) tree; + if (tree instanceof LinkTree link) { if (link.getReference() != null) { String signature = link.getReference().getSignature(); // resolve the referenced constant using the fully qualified signature, so that references @@ -332,9 +381,12 @@ private String resolveDefaultValue( ? 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()); + // 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; } @@ -359,7 +411,7 @@ private VariableElement resolveReferencedField( return null; } Element element = docTrees.getElement(refPath); - return element instanceof VariableElement ? (VariableElement) element : null; + return element instanceof VariableElement variableElement ? variableElement : null; } private String lookupConstant(TypeElement type, String constantName) { @@ -417,23 +469,32 @@ private String resolveEnumReference(VariableElement field) { } private String extractClassLink( - VariableElement contextField, DocCommentTree docComment, List content) { - if (content == null || content.isEmpty()) { - throw new IllegalArgumentException("Missing content for @configurationDefaultValue"); + VariableElement contextField, + DocCommentTree docComment, + Map 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( - "Expected a class link in @configurationDefaultValue, but got a member reference: " - + signature); + // 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 @configurationDefaultValue"); + throw new DocTreePathAwareRuntimeException(tagPath, "No valid {@link ...} reference found in @" + tagName); } /** @@ -452,8 +513,8 @@ private String resolveReferencedType( return signature; } Element element = docTrees.getElement(refPath); - return element instanceof TypeElement - ? ((TypeElement) element).getQualifiedName().toString() + return element instanceof TypeElement typeElement + ? typeElement.getQualifiedName().toString() : signature; } 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 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> keys = readKeys(output); assertEquals(4, keys.size(), "expected four configuration keys"); @@ -96,17 +123,85 @@ void extractsBooleanStringAndEnumConfigurations(@TempDir Path tempDir) throws Ex assertEquals("No", enum2Key.get("supportRepoIdSuffix")); } - private static void runDoclet(Path sourceFile, Path output) throws Exception { + static final class CapturingDiagnosticsListener + implements javax.tools.DiagnosticListener { + private final javax.tools.Diagnostic.Kind threshold; + private final Collection> diagnostics = new ArrayList<>(); + + public CapturingDiagnosticsListener(javax.tools.Diagnostic.Kind threshold) { + this.threshold = threshold; + } + + @Override + public void report(javax.tools.Diagnostic diagnostic) { + if (diagnostic.getKind().compareTo(threshold) <= 0) { + diagnostics.add(diagnostic); + } + } + + public Collection> getDiagnostics() { + return diagnostics; + } + } + + @Test + void invalidMode() throws Exception { + CapturingDiagnosticsListener 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 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 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> iterator = + listener.getDiagnostics().iterator(); + Diagnostic 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 listener) + throws Exception { DocumentationTool documentationTool = ToolProvider.getSystemDocumentationTool(); try (StandardJavaFileManager fileManager = documentationTool.getStandardFileManager(null, null, StandardCharsets.UTF_8)) { Iterable units = fileManager.getJavaFileObjectsFromFiles(List.of(sourceFile.toFile())); - List options = List.of("--output", output.toString(), "-encoding", "UTF-8"); - StringWriter out = new StringWriter(); + final List 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() {} +}