From 093902a8b11721ad635d6ea94843a06b2490b0bf Mon Sep 17 00:00:00 2001 From: Tom Daffurn Date: Tue, 24 Sep 2024 14:41:52 +1000 Subject: [PATCH] feat: Extract comments from Java code (#2788) Extract comments from Java code for Verbs, Data, Enum, Config, and Secrets Closes #2417 Schema for the test data module is: ``` module javacomments { // Config comment config config String // Secret comment secret secretString String // Comment on a data class. export data DataClass { field String } export data EnumType { name String ordinal Int } // Comment on a verb export verb MultilineCommentVerb(javacomments.DataClass) javacomments.EnumType } ``` --- .../xyz/block/ftl/deployment/CommentKey.java | 23 +++++++ .../block/ftl/deployment/ModuleBuilder.java | 40 +++++++----- .../block/ftl/deployment/ModuleProcessor.java | 42 ++++++++----- .../processor/AnnotationProcessor.java | 62 ++++++++++++++++++- .../testdata/java/javacomments/ftl.toml | 2 + .../testdata/java/javacomments/pom.xml | 22 +++++++ .../ftl/javacomments/CommentedModule.java | 33 ++++++++++ .../xyz/block/ftl/javacomments/DataClass.java | 14 +++++ .../xyz/block/ftl/javacomments/EnumType.java | 14 +++++ 9 files changed, 218 insertions(+), 34 deletions(-) create mode 100644 jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/CommentKey.java create mode 100644 jvm-runtime/testdata/java/javacomments/ftl.toml create mode 100644 jvm-runtime/testdata/java/javacomments/pom.xml create mode 100644 jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/CommentedModule.java create mode 100644 jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/DataClass.java create mode 100644 jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/EnumType.java diff --git a/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/CommentKey.java b/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/CommentKey.java new file mode 100644 index 0000000000..0b1c64bca7 --- /dev/null +++ b/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/CommentKey.java @@ -0,0 +1,23 @@ +package xyz.block.ftl.deployment; + +public class CommentKey { + public static String ofVerb(String verb) { + return "verb." + verb; + } + + public static String ofData(String data) { + return "data." + data; + } + + public static String ofEnum(String enumName) { + return "enum." + enumName; + } + + public static String ofConfig(String config) { + return "config." + config; + } + + public static String ofSecret(String secret) { + return "secret." + secret; + } +} diff --git a/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/ModuleBuilder.java b/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/ModuleBuilder.java index 7a078a04db..3d2627d4e8 100644 --- a/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/ModuleBuilder.java +++ b/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/ModuleBuilder.java @@ -11,6 +11,7 @@ import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import java.util.function.BiFunction; import java.util.function.Consumer; @@ -55,7 +56,6 @@ import xyz.block.ftl.v1.schema.MetadataCalls; import xyz.block.ftl.v1.schema.MetadataTypeMap; import xyz.block.ftl.v1.schema.Module; -import xyz.block.ftl.v1.schema.Optional; import xyz.block.ftl.v1.schema.Ref; import xyz.block.ftl.v1.schema.Time; import xyz.block.ftl.v1.schema.Type; @@ -84,11 +84,11 @@ public class ModuleBuilder { private final Map knownTopics; private final Map verbClients; private final FTLRecorder recorder; - private final Map verbDocs; + private final Map> comments; public ModuleBuilder(IndexView index, String moduleName, Map knownTopics, Map verbClients, FTLRecorder recorder, - Map verbDocs, Map typeAliases) { + Map> comments, Map typeAliases) { this.index = index; this.moduleName = moduleName; this.moduleBuilder = Module.newBuilder() @@ -97,7 +97,7 @@ public ModuleBuilder(IndexView index, String moduleName, Map(typeAliases); } @@ -184,8 +184,12 @@ public void registerVerbMethod(MethodInfo method, String className, String name = param.annotation(Secret.class).value().asString(); paramMappers.add(new VerbRegistry.SecretSupplier(name, paramType)); if (!knownSecrets.contains(name)) { - addDecls(Decl.newBuilder().setSecret(xyz.block.ftl.v1.schema.Secret.newBuilder() - .setType(buildType(param.type(), false)).setName(name)).build()); + xyz.block.ftl.v1.schema.Secret.Builder secretBuilder = xyz.block.ftl.v1.schema.Secret.newBuilder() + .setType(buildType(param.type(), false)) + .setName(name); + Optional.ofNullable(comments.get(CommentKey.ofSecret(name))) + .ifPresent(secretBuilder::addAllComments); + addDecls(Decl.newBuilder().setSecret(secretBuilder).build()); knownSecrets.add(name); } } else if (param.hasAnnotation(Config.class)) { @@ -194,8 +198,12 @@ public void registerVerbMethod(MethodInfo method, String className, String name = param.annotation(Config.class).value().asString(); paramMappers.add(new VerbRegistry.ConfigSupplier(name, paramType)); if (!knownConfig.contains(name)) { - addDecls(Decl.newBuilder().setConfig(xyz.block.ftl.v1.schema.Config.newBuilder() - .setType(buildType(param.type(), false)).setName(name)).build()); + xyz.block.ftl.v1.schema.Config.Builder configBuilder = xyz.block.ftl.v1.schema.Config.newBuilder() + .setType(buildType(param.type(), false)) + .setName(name); + Optional.ofNullable(comments.get(CommentKey.ofConfig(name))) + .ifPresent(configBuilder::addAllComments); + addDecls(Decl.newBuilder().setConfig(configBuilder).build()); knownConfig.add(name); } } else if (knownTopics.containsKey(param.type().name())) { @@ -242,9 +250,8 @@ public void registerVerbMethod(MethodInfo method, String className, .setExport(exported) .setRequest(buildType(bodyParamType, exported)) .setResponse(buildType(method.returnType(), exported)); - if (verbDocs.containsKey(verbName)) { - verbBuilder.addComments(verbDocs.get(verbName)); - } + Optional.ofNullable(comments.get(CommentKey.ofVerb(verbName))) + .ifPresent(verbBuilder::addAllComments); if (metadataCallback != null) { metadataCallback.accept(verbBuilder); @@ -301,7 +308,9 @@ public Type buildType(org.jboss.jandex.Type type, boolean export) { if (type.hasAnnotation(NOT_NULL)) { return primitive; } - return Type.newBuilder().setOptional(Optional.newBuilder().setType(primitive)).build(); + return Type.newBuilder().setOptional(xyz.block.ftl.v1.schema.Optional.newBuilder() + .setType(primitive)) + .build(); } if (info != null && info.hasDeclaredAnnotation(GENERATED_REF)) { var ref = info.declaredAnnotation(GENERATED_REF); @@ -347,6 +356,8 @@ public Type buildType(org.jboss.jandex.Type type, boolean export) { Data.Builder data = Data.newBuilder(); data.setName(clazz.name().local()); data.setExport(type.hasAnnotation(EXPORT) || export); + Optional.ofNullable(comments.get(CommentKey.ofData(clazz.name().local()))) + .ifPresent(data::addAllComments); buildDataElement(data, clazz.name()); moduleBuilder.addDecls(Decl.newBuilder().setData(data).build()); Ref ref = Ref.newBuilder().setName(data.getName()).setModule(moduleName).build(); @@ -367,9 +378,8 @@ public Type buildType(org.jboss.jandex.Type type, boolean export) { .build(); } else if (paramType.name().equals(DotNames.OPTIONAL)) { //TODO: optional kinda sucks - return Type.newBuilder() - .setOptional( - Optional.newBuilder().setType(buildType(paramType.arguments().get(0), export))) + return Type.newBuilder().setOptional(xyz.block.ftl.v1.schema.Optional.newBuilder() + .setType(buildType(paramType.arguments().get(0), export))) .build(); } else if (paramType.name().equals(DotName.createSimple(HttpRequest.class))) { return Type.newBuilder() diff --git a/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/ModuleProcessor.java b/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/ModuleProcessor.java index d287c0cf1e..bd2c31c4c9 100644 --- a/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/ModuleProcessor.java +++ b/jvm-runtime/ftl-runtime/common/deployment/src/main/java/xyz/block/ftl/deployment/ModuleProcessor.java @@ -5,6 +5,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.PosixFilePermission; +import java.util.Arrays; import java.util.Base64; import java.util.EnumSet; import java.util.HashMap; @@ -113,21 +114,7 @@ public void generateSchema(CombinedIndexBuildItem index, List typeAliasBuildItems, List schemaContributorBuildItems) throws Exception { String moduleName = moduleNameBuildItem.getModuleName(); - Map verbDocs = new HashMap<>(); - try (var input = Thread.currentThread().getContextClassLoader().getResourceAsStream("META-INF/ftl-verbs.txt")) { - if (input != null) { - var contents = new String(input.readAllBytes(), StandardCharsets.UTF_8).split("\n"); - for (var content : contents) { - var eq = content.indexOf('='); - if (eq == -1) { - continue; - } - String key = content.substring(0, eq); - String value = new String(Base64.getDecoder().decode(content.substring(eq + 1)), StandardCharsets.UTF_8); - verbDocs.put(key, value); - } - } - } + Map> comments = readComments(); Map existingRefs = new HashMap<>(); for (var i : typeAliasBuildItems) { String mn; @@ -146,7 +133,7 @@ public void generateSchema(CombinedIndexBuildItem index, } ModuleBuilder moduleBuilder = new ModuleBuilder(index.getComputingIndex(), moduleName, topicsBuildItem.getTopics(), - verbClientBuildItem.getVerbClients(), recorder, verbDocs, existingRefs); + verbClientBuildItem.getVerbClients(), recorder, comments, existingRefs); for (var i : schemaContributorBuildItems) { i.getSchemaContributor().accept(moduleBuilder); @@ -201,4 +188,27 @@ void openSocket(BuildProducer virtual, socket.produce(RequireSocketHttpBuildItem.MARKER); virtual.produce(RequireVirtualHttpBuildItem.MARKER); } + + /** + * Bytecode doesn't retain comments, so they are stored in a separate file + * Each line is a key value pair separated by an '='. The key is the DeclRef and the value is the comment + */ + private Map> readComments() throws IOException { + Map> comments = new HashMap<>(); + try (var input = Thread.currentThread().getContextClassLoader().getResourceAsStream("META-INF/ftl-verbs.txt")) { + if (input != null) { + var contents = new String(input.readAllBytes(), StandardCharsets.UTF_8).split("\n"); + for (var content : contents) { + var eq = content.indexOf('='); + if (eq == -1) { + continue; + } + String key = content.substring(0, eq); + String value = new String(Base64.getDecoder().decode(content.substring(eq + 1)), StandardCharsets.UTF_8); + comments.put(key, Arrays.asList(value.split("\n"))); + } + } + } + return comments; + } } diff --git a/jvm-runtime/ftl-runtime/java/runtime/src/main/java/xyz/block/ftl/runtime/processor/AnnotationProcessor.java b/jvm-runtime/ftl-runtime/java/runtime/src/main/java/xyz/block/ftl/runtime/processor/AnnotationProcessor.java index 99c92eff63..326174237b 100644 --- a/jvm-runtime/ftl-runtime/java/runtime/src/main/java/xyz/block/ftl/runtime/processor/AnnotationProcessor.java +++ b/jvm-runtime/ftl-runtime/java/runtime/src/main/java/xyz/block/ftl/runtime/processor/AnnotationProcessor.java @@ -19,12 +19,17 @@ import javax.lang.model.SourceVersion; import javax.lang.model.element.AnnotationMirror; import javax.lang.model.element.Element; +import javax.lang.model.element.ElementKind; import javax.lang.model.element.ExecutableElement; import javax.lang.model.element.TypeElement; +import javax.lang.model.element.VariableElement; import javax.tools.Diagnostic; import javax.tools.FileObject; import javax.tools.StandardLocation; +import xyz.block.ftl.Config; +import xyz.block.ftl.Export; +import xyz.block.ftl.Secret; import xyz.block.ftl.Verb; /** @@ -32,6 +37,10 @@ */ public class AnnotationProcessor implements Processor { private static final Pattern REMOVE_LEADING_SPACE = Pattern.compile("^ ", Pattern.MULTILINE); + private static final Pattern REMOVE_JAVADOC_TAGS = Pattern.compile( + "^\\s*@(param|return|throws|exception|see|author)\\b[^\\n]*$\\n*", + Pattern.MULTILINE); + private ProcessingEnvironment processingEnv; final Map saved = new HashMap<>(); @@ -43,7 +52,7 @@ public Set getSupportedOptions() { @Override public Set getSupportedAnnotationTypes() { - return Set.of(Verb.class.getName()); + return Set.of(Verb.class.getName(), Export.class.getName()); } @Override @@ -59,10 +68,36 @@ public void init(ProcessingEnvironment processingEnv) { @Override public boolean process(Set annotations, RoundEnvironment roundEnv) { //TODO: @VerbName, HTTP, CRON etc - roundEnv.getElementsAnnotatedWith(Verb.class) + roundEnv.getElementsAnnotatedWithAny(Set.of(Verb.class, Export.class)) .forEach(element -> { Optional javadoc = getJavadoc(element); - javadoc.ifPresent(doc -> saved.put(element.getSimpleName().toString(), doc)); + + javadoc.ifPresent(doc -> { + String strippedDownDoc = stripJavadocTags(doc); + String key = element.getSimpleName().toString(); + + if (element.getKind() == ElementKind.METHOD) { + saved.put("verb." + key, strippedDownDoc); + } else if (element.getKind() == ElementKind.CLASS) { + saved.put("data." + key, strippedDownDoc); + } else if (element.getKind() == ElementKind.ENUM) { + saved.put("enum." + key, strippedDownDoc); + } + + if (element.getKind() == ElementKind.METHOD) { + var executableElement = (ExecutableElement) element; + executableElement.getParameters().forEach(param -> { + Config config = param.getAnnotation(Config.class); + if (config != null) { + saved.put("config." + config.value(), extractCommentForParam(doc, param)); + } + Secret secret = param.getAnnotation(Secret.class); + if (secret != null) { + saved.put("secret." + secret.value(), extractCommentForParam(doc, param)); + } + }); + } + }); }); if (roundEnv.processingOver()) { @@ -117,4 +152,25 @@ public Optional getJavadoc(Element e) { .trim()); } + public String stripJavadocTags(String doc) { + // TODO extract JavaDoc tags to a rich markdown model supported by schema + return REMOVE_JAVADOC_TAGS.matcher(doc).replaceAll(""); + } + + /** + * Read the @param tag in a JavaDoc comment to extract Config and Secret comments + */ + private String extractCommentForParam(String doc, VariableElement param) { + String variableName = param.getSimpleName().toString(); + int startIdx = doc.indexOf("@param " + variableName + " "); + if (startIdx != -1) { + int endIndex = doc.indexOf("\n", startIdx); + if (endIndex == -1) { + endIndex = doc.length(); + } + return doc.substring(startIdx + variableName.length() + 8, endIndex); + } + return null; + } + } diff --git a/jvm-runtime/testdata/java/javacomments/ftl.toml b/jvm-runtime/testdata/java/javacomments/ftl.toml new file mode 100644 index 0000000000..ee2f93e1fb --- /dev/null +++ b/jvm-runtime/testdata/java/javacomments/ftl.toml @@ -0,0 +1,2 @@ +module = "javacomments" +language = "java" diff --git a/jvm-runtime/testdata/java/javacomments/pom.xml b/jvm-runtime/testdata/java/javacomments/pom.xml new file mode 100644 index 0000000000..f872bb7f7a --- /dev/null +++ b/jvm-runtime/testdata/java/javacomments/pom.xml @@ -0,0 +1,22 @@ + + + 4.0.0 + xyz.block.ftl.examples + javacomments + 1.0-SNAPSHOT + + + xyz.block.ftl + ftl-build-parent-java + 1.0-SNAPSHOT + + + + + xyz.block + web5-dids + 2.0.1-debug1 + + + + diff --git a/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/CommentedModule.java b/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/CommentedModule.java new file mode 100644 index 0000000000..a7fb5c4f03 --- /dev/null +++ b/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/CommentedModule.java @@ -0,0 +1,33 @@ +package xyz.block.ftl.javacomments; + +import org.jetbrains.annotations.NotNull; + +import xyz.block.ftl.Config; +import xyz.block.ftl.Export; +import xyz.block.ftl.Secret; +import xyz.block.ftl.Verb; + +/** + * Comment on a module class + */ +public class CommentedModule { + + /** + * Comment on a verb + * + * @param val Parameter comment + * @param configString Config comment + * @param secretString Secret comment + * @return Great success + */ + @Export + @Verb + public @NotNull EnumType MultilineCommentVerb( + @NotNull DataClass val, + @Config("config") String configString, + @Secret("secretString") String secretString) { + return EnumType.PORTENTOUS; + } + + //TODO TypeAlias, Database, Topic, Subscription, Lease, Cron +} diff --git a/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/DataClass.java b/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/DataClass.java new file mode 100644 index 0000000000..44be37a85f --- /dev/null +++ b/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/DataClass.java @@ -0,0 +1,14 @@ +package xyz.block.ftl.javacomments; + +import xyz.block.ftl.Export; + +/** + * Comment on a data class. + */ +@Export +public class DataClass { + /** + * Comment on a data field. + */ + private String field; +} diff --git a/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/EnumType.java b/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/EnumType.java new file mode 100644 index 0000000000..2c36f42822 --- /dev/null +++ b/jvm-runtime/testdata/java/javacomments/src/main/java/xyz/block/ftl/javacomments/EnumType.java @@ -0,0 +1,14 @@ +package xyz.block.ftl.javacomments; + +import xyz.block.ftl.Export; + +/** + * Comment on an enum type + */ +@Export +public enum EnumType { + /** + * Comment on an enum value + */ + PORTENTOUS +}