Skip to content

Commit

Permalink
Merge pull request gradle#27284 Improve readability of inferred versi…
Browse files Browse the repository at this point in the history 
…on catalog values in javadocs

Current javadocs generated for version catalog accessors are rendered as a monotone single paragraph text, making it hard to get a value out of looking at them. At the same time, they already contain useful information inferred from the values in the actual version catalog.

This PR improves the generated javadocs by
- consistently marking all inferred values as bold
- rewording the scaffolding sentences to make them more concise and clear
- splitting each javadoc into multiple paragraphs, keeping different information visually separated
- provides a sensible javadoc for empty bundles
- fixes the indentation of the generated accessor sources

## Before

Versions
<img width="700" alt="image" src="https://github.com/gradle/gradle/assets/2759152/912966bd-a51c-41cd-9b1f-8af3c825cc35">

Libraries
<img width="700" alt="image" src="https://github.com/gradle/gradle/assets/2759152/ed482c55-9150-4417-ab10-bf8a0bec3d87">

Plugins
<img width="700" alt="image" src="https://github.com/gradle/gradle/assets/2759152/61070cea-7600-4827-938d-1282f4bf373d">

## After

Versions
<img width="699" alt="image" src="https://github.com/gradle/gradle/assets/2759152/de0db4d9-e414-41a2-a0ac-33160476d57c">

Libraries
<img width="700" alt="image" src="https://github.com/gradle/gradle/assets/2759152/5f7c34b0-4eac-47bd-93a4-0fb5e8eea015">

Plugins
<img width="700" alt="image" src="https://github.com/gradle/gradle/assets/2759152/b8eda2b6-97ac-4ef0-aa0e-4547245c792b">

Co-authored-by: Alex Semin <asemin@gradle.com>
  • Loading branch information
@bot-gradle @alllex
bot-gradle and alllex committed Dec 6, 2023
2 parents 110e289 + b888c1b commit a22feef
Show file tree
Hide file tree
Showing 3 changed files with 65 additions and 59 deletions.
2 changes: 1 addition & 1 deletion ...anagement/src/main/java/org/gradle/api/internal/catalog/DefaultVersionCatalogBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public DefaultVersionCatalogBuilder(
this.objects = objects;
this.dependencyResolutionServicesSupplier = dependencyResolutionServicesSupplier;
this.strictVersionParser = new StrictVersionParser(strings);
this.description = objects.property(String.class).convention("A catalog of dependencies accessible via the `" + name + "` extension.");
this.description = objects.property(String.class).convention("A catalog of dependencies accessible via the {@code " + name + "} extension.");
}

@Inject
Expand Down
106 changes: 56 additions & 50 deletions ...cy-management/src/main/java/org/gradle/api/internal/catalog/LibrariesSourceGenerator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
import org.gradle.plugin.use.PluginDependency;
import org.gradle.util.internal.TextUtil;

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import javax.inject.Inject;
import java.io.IOException;
Expand All @@ -51,6 +50,7 @@
import java.util.Map;
import java.util.Set;

import static java.util.Objects.requireNonNull;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;
import static org.gradle.api.internal.catalog.problems.DefaultCatalogProblemBuilder.VERSION_CATALOG_PROBLEMS;
Expand Down Expand Up @@ -158,7 +158,7 @@ private void generateFactoryClass(String packageName, ThrowingConsumer<EntryPoin
writeLn();
addImports();
writeLn();
String description = TextUtil.normaliseLineSeparators(config.getDescription());
String description = requireNonNull(TextUtil.normaliseLineSeparators(config.getDescription()));
writeLn("/**");
for (String descLine : Splitter.on('\n').split(description)) {
writeLn(" * " + descLine);
Expand Down Expand Up @@ -253,7 +253,7 @@ private void writePluginSubClasses(ClassNode classNode) throws IOException {
private void writeBundleAccessorClass(ClassNode classNode, boolean deprecated) throws IOException {
if (deprecated) {
writeLn("/**");
writeDeprecationJavadocTag(true);
writeDeprecationJavadocTag(true, false);
writeLn(" */");
writeDeprecationAnnotation(true);
}
Expand Down Expand Up @@ -423,7 +423,7 @@ private void writeSubAccessorFieldsOf(ClassNode classNode, AccessorKind kind) th
private void writeLibraryAccessorClass(ClassNode classNode, boolean deprecated) throws IOException {
if (deprecated) {
writeLn("/**");
writeDeprecationJavadocTag(true);
writeDeprecationJavadocTag(true, false);
writeLn(" */");
writeDeprecationAnnotation(true);
}
Expand Down Expand Up @@ -477,7 +477,7 @@ private void writeVersionAccessorClass(ClassNode classNode) throws IOException {
if (!classNode.hasChild(childName)) {
VersionModel vm = config.getVersion(alias);
String context = vm.getContext();
indent(() -> writeSingleVersionAccessor(alias, context, vm.getVersion().getDisplayName(), false));
writeSingleVersionAccessor(alias, context, vm.getVersion().getDisplayName(), false);
}
}
for (ClassNode child : classNode.getChildren()) {
Expand All @@ -490,10 +490,12 @@ private void writeVersionAccessorClass(ClassNode classNode) throws IOException {

private void writeSingleVersionAccessor(String versionAlias, @Nullable String context, String version, boolean asProvider) throws IOException {
writeLn("/**");
writeLn(" * Returns the version associated to this alias: " + versionAlias + " (" + version + ")");
writeLn(" * If the version is a rich version and that its not expressible as a");
writeLn(" * single version string, then an empty string is returned.");
writeLn(" * Version alias <b>" + versionAlias + "</b> with value <b>" + version + "</b>");
writeLn(" * <p>");
writeLn(" * If the version is a rich version and cannot be represented as a");
writeLn(" * single version string, an empty string is returned.");
if (context != null) {
writeLn(" * <p>");
writeLn(" * This version was declared in " + sanitizeUnicodeEscapes(context));
}
writeLn(" */");
Expand Down Expand Up @@ -521,7 +523,6 @@ private RuntimeException throwVersionCatalogProblemException(ReportableProblem p
throw throwErrorWithNewProblemsApi(ERROR_HEADER, ImmutableList.of(problem));
}

@Nonnull
private static ProblemBuilder configureVersionCatalogError(ProblemBuilderDefiningLabel builder, String message, VersionCatalogProblemId catalogProblemId) {
return builder
.label(message)
Expand All @@ -548,7 +549,6 @@ private void assertUnique(List<String> names, String prefix, String suffix) {
maybeThrowError(ERROR_HEADER, errors);
}

@Nonnull
private String getProblemPrefix() {
return getProblemInVersionCatalog(config.getName()) + ", ";
}
Expand All @@ -559,33 +559,34 @@ private static String coordinatesDescriptorFor(DependencyModel dependencyData) {

private void writeDependencyAccessor(String alias, DependencyModel dependency, boolean asProvider, boolean deprecated) throws IOException {
String name = leafNodeForAlias(alias);
writeLn(" /**");
writeLn(" * Creates a dependency provider for " + name + " (" + coordinatesDescriptorFor(dependency) + ")");
writeLn("/**");
writeLn(" * Dependency provider for <b>" + name + "</b> with <b>" + coordinatesDescriptorFor(dependency) + "</b> coordinates and");
writeVersionInformation(dependency.getVersionRef(), dependency.getVersion());
String context = dependency.getContext();
if (context != null) {
writeLn(" * This dependency was declared in " + sanitizeUnicodeEscapes(context));
writeLn(" * <p>");
writeLn(" * This dependency was declared in " + sanitizeUnicodeEscapes(context));
}
writeDeprecationJavadocTag(deprecated);
writeLn(" */");
writeDeprecationJavadocTag(deprecated, true);
writeLn(" */");
writeDeprecationAnnotation(deprecated);
String methodName = asProvider ? "asProvider" : "get" + toJavaName(name);
writeLn(" public Provider<MinimalExternalModuleDependency> " + methodName + "() {");
writeLn("public Provider<MinimalExternalModuleDependency> " + methodName + "() {");
writeDeprecationLog(deprecated);
writeLn(" return create(\"" + alias + "\");");
writeLn(" return create(\"" + alias + "\");");
writeLn("}");
writeLn();
}

private void writeVersionInformation(@Nullable String versionRef, ImmutableVersionConstraint version) throws IOException {
if (versionRef != null) {
writeLn(" * with versionRef '" + versionRef + "'.");
writeLn(" * with version reference <b>" + versionRef + "</b>");
} else {
String versionDisplay = version.getDisplayName();
if (versionDisplay.isEmpty()) {
writeLn(" * with no version specified");
writeLn(" * with <b>no version specified</b>");
} else {
writeLn(" * with version '" + versionDisplay + "'.");
writeLn(" * with version <b>" + versionDisplay + "</b>");
}
}
}
Expand All @@ -603,8 +604,8 @@ private void writeSubAccessor(ClassNode classNode, AccessorKind kind, boolean de
String className = getClassName(classNode);
String getter = classNode.name;
writeLn("/**");
writeLn(" * Returns the group of " + kind.getDescription() + " at " + classNode.getPath());
writeDeprecationJavadocTag(deprecated);
writeLn(" * Group of " + kind.getDescription() + " at <b>" + classNode.getPath() + "</b>");
writeDeprecationJavadocTag(deprecated, true);
writeLn(" */");
writeDeprecationAnnotation(deprecated);
writeLn("public " + className + " get" + toJavaName(getter) + "() {");
Expand All @@ -615,31 +616,37 @@ private void writeSubAccessor(ClassNode classNode, AccessorKind kind, boolean de
}

private void writeBundle(String alias, List<String> coordinates, @Nullable String context, boolean asProvider, boolean deprecated) throws IOException {
indent(() -> {
writeLn("/**");
writeLn(" * Creates a dependency bundle provider for " + alias + " which is an aggregate for the following dependencies:");
writeLn("/**");
if (coordinates.isEmpty()) {
writeLn(" * Dependency bundle provider for <b>" + alias + "</b> which contains no dependencies");
} else {
writeLn(" * Dependency bundle provider for <b>" + alias + "</b> which contains the following dependencies:");
writeLn(" * <ul>");
for (String coordinate : coordinates) {
writeLn(" * <li>" + coordinate + "</li>");
}
writeLn(" * </ul>");
if (context != null) {
writeLn(" * This bundle was declared in " + sanitizeUnicodeEscapes(context));
}
writeDeprecationJavadocTag(deprecated);
writeLn(" */");
writeDeprecationAnnotation(deprecated);
String methodName = asProvider ? "asProvider" : "get" + toJavaName(leafNodeForAlias(alias));
writeLn("public Provider<ExternalModuleDependencyBundle> " + methodName + "() {");
writeDeprecationLog(deprecated);
writeLn(" return createBundle(\"" + alias + "\");");
writeLn("}");
});
}
if (context != null) {
writeLn(" * <p>");
writeLn(" * This bundle was declared in " + sanitizeUnicodeEscapes(context));
}
writeDeprecationJavadocTag(deprecated, true);
writeLn(" */");
writeDeprecationAnnotation(deprecated);
String methodName = asProvider ? "asProvider" : "get" + toJavaName(leafNodeForAlias(alias));
writeLn("public Provider<ExternalModuleDependencyBundle> " + methodName + "() {");
writeDeprecationLog(deprecated);
writeLn(" return createBundle(\"" + alias + "\");");
writeLn("}");
writeLn();
}

private void writeDeprecationJavadocTag(boolean deprecated) throws IOException {
private void writeDeprecationJavadocTag(boolean deprecated, boolean separate) throws IOException {
if (deprecated) {
if (separate) {
writeLn(" *");
}
writeLn(" * @deprecated Will be removed in Gradle 9.0.");
}
}
Expand All @@ -664,18 +671,17 @@ private void writeDeprecationLog(boolean deprecated) throws IOException {
}

private void writePlugin(String alias, PluginModel plugin, boolean asProvider) throws IOException {
indent(() -> {
writeLn("/**");
writeLn(" * Creates a plugin provider for " + alias + " to the plugin id '" + plugin.getId() + "'");
writeVersionInformation(plugin.getVersionRef(), plugin.getVersion());
String context = plugin.getContext();
if (context != null) {
writeLn(" * This plugin was declared in " + sanitizeUnicodeEscapes(context));
}
writeLn(" */");
String methodName = asProvider ? "asProvider" : "get" + toJavaName(leafNodeForAlias(alias));
writeLn("public Provider<PluginDependency> " + methodName + "() { return createPlugin(\"" + alias + "\"); }");
});
writeLn("/**");
writeLn(" * Plugin provider for <b>" + alias + "</b> with plugin id <b>" + plugin.getId() + "</b> and");
writeVersionInformation(plugin.getVersionRef(), plugin.getVersion());
String context = plugin.getContext();
if (context != null) {
writeLn(" * <p>");
writeLn(" * This plugin was declared in " + sanitizeUnicodeEscapes(context));
}
writeLn(" */");
String methodName = asProvider ? "asProvider" : "get" + toJavaName(leafNodeForAlias(alias));
writeLn("public Provider<PluginDependency> " + methodName + "() { return createPlugin(\"" + alias + "\"); }");
writeLn();
}

Expand Down
16 changes: 8 additions & 8 deletions ...ement/src/test/groovy/org/gradle/api/internal/catalog/LibrariesSourceGeneratorTest.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -166,14 +166,14 @@ class LibrariesSourceGeneratorTest extends AbstractVersionCatalogTest implements
}

then:
sources.hasDependencyAlias('foo', 'getFoo', "with version '1.0'")
sources.hasDependencyAlias('fooBaz', 'getFooBaz', "with version '{strictly [1.0, 2.0[; prefer 1.2}'")
sources.hasDependencyAlias('bar', 'getBar', "with versionRef 'barVersion'")
sources.hasDependencyAlias('boo', 'getBoo', "with no version specified")

sources.hasPlugin('fooPlugin', 'getFooPlugin', "with version '1.0'")
sources.hasPlugin('barPlugin', 'getBarPlugin', "with versionRef 'barVersion'")
sources.hasPlugin('bazPlugin', 'getBazPlugin', "with no version specified")
sources.hasDependencyAlias('foo', 'getFoo', "with version <b>1.0</b>")
sources.hasDependencyAlias('fooBaz', 'getFooBaz', "with version <b>{strictly [1.0, 2.0[; prefer 1.2}</b>")
sources.hasDependencyAlias('bar', 'getBar', "with version reference <b>barVersion</b>")
sources.hasDependencyAlias('boo', 'getBoo', "with <b>no version specified</b>")

sources.hasPlugin('fooPlugin', 'getFooPlugin', "with version <b>1.0</b>")
sources.hasPlugin('barPlugin', 'getBarPlugin', "with version reference <b>barVersion</b>")
sources.hasPlugin('bazPlugin', 'getBazPlugin', "with <b>no version specified</b>")
}

@VersionCatalogProblemTestFor(
Expand Down

0 comments on commit a22feef

Please sign in to comment.