Skip to content

Commit

Permalink
Javadoc for generated gRPC classes has warnings and errors (#2139)
Browse files Browse the repository at this point in the history 
Motivation:

Generated gRPC classes have javadoc with illegal HTML characters
`<` and `>` and misses some `@param`. It causes build failures when
project compiles with `-Xwerror` flag.

Modifications:

- Replace parametrized types with non-parametrized for javadoc `@link`;
- Add missing `@param`;
- Include generated classes into `testJavadoc` task for
`servicetalk-grpc-protoc`;

Result:

Project generates javadoc successfully when run with `-Xwerror` flag.
  • Loading branch information
@idelpivnitskiy
idelpivnitskiy authored Mar 10, 2022
1 parent dd94aea commit dbcbfd8
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 24 deletions.
14 changes: 14 additions & 0 deletions servicetalk-grpc-protoc/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,17 @@ afterEvaluate {
clean {
delete protobuf.generatedFilesBaseDir
}

task testJavadoc(type: Javadoc) {
dependsOn tasks.compileTestJava

source = protobuf.generatedFilesBaseDir
classpath = sourceSets.test.compileClasspath
destinationDir = file("$buildDir/tmp/testjavadoc")
options.addBooleanOption("Xdoclint:syntax", true)
options.addBooleanOption("Xdoclint:html", true)
options.addBooleanOption("Xdoclint:accessibility", true)
// Don't use Xdoclint:missing for now because not all methods/types have javadocs.
}

test.finalizedBy(testJavadoc)
116 changes: 94 additions & 22 deletions servicetalk-grpc-protoc/src/main/java/io/servicetalk/grpc/protoc/Generator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@
import static io.servicetalk.grpc.protoc.Types.ResponseStreamingClientCall;
import static io.servicetalk.grpc.protoc.Types.ResponseStreamingRoute;
import static io.servicetalk.grpc.protoc.Types.Route;
import static io.servicetalk.grpc.protoc.Types.RouteExecutionStrategy;
import static io.servicetalk.grpc.protoc.Types.RouteExecutionStrategyFactory;
import static io.servicetalk.grpc.protoc.Types.Single;
import static io.servicetalk.grpc.protoc.Types.StreamingClientCall;
import static io.servicetalk.grpc.protoc.Types.StreamingRoute;
Expand All @@ -109,6 +111,7 @@
import static io.servicetalk.grpc.protoc.Words.Default;
import static io.servicetalk.grpc.protoc.Words.Factory;
import static io.servicetalk.grpc.protoc.Words.INSTANCE;
import static io.servicetalk.grpc.protoc.Words.JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT;
import static io.servicetalk.grpc.protoc.Words.JAVADOC_DEPRECATED;
import static io.servicetalk.grpc.protoc.Words.JAVADOC_PARAM;
import static io.servicetalk.grpc.protoc.Words.JAVADOC_RETURN;
Expand Down Expand Up @@ -343,7 +346,7 @@ private String addServiceRpcInterfaceSpec(final State state,
methodDescriptorType, methodDescFieldName, isAsync));

final FieldSpec.Builder pathSpecBuilder = FieldSpec.builder(String.class, RPC_PATH)
.addJavadoc(JAVADOC_DEPRECATED + " Use {@link #$L}." + lineSeparator(), methodDescriptor)
.addJavadoc(JAVADOC_DEPRECATED + "Use {@link #$L}." + lineSeparator(), methodDescriptor)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC, STATIC, FINAL) // redundant, default for interface field
.initializer("$S", context.methodPath(state.serviceProto, methodProto));
Expand Down Expand Up @@ -466,27 +469,44 @@ private TypeSpec.Builder addServiceFactory(final State state, final TypeSpec.Bui
.addType(newServiceFromRoutesClassSpec(serviceFromRoutesClass, state.serviceRpcInterfaces,
state.serviceClass))
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addModifiers(PUBLIC)
.addStatement("this($T.emptyList())", Collections)
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_DEPRECATED + " Use {@link #$L($T)} and {@link #$L($T)}." + lineSeparator(),
bufferDecoderGroup, BufferDecoderGroup, bufferEncoders, BufferEncoderList)
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + supportedMessageCodings + " the set of allowed encodings" +
lineSeparator())
.addJavadoc(JAVADOC_DEPRECATED + "Use {@link #$L($T)} and {@link #$L($T)}." + lineSeparator(),
bufferDecoderGroup, BufferDecoderGroup, bufferEncoders, Types.List)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addParameter(GrpcSupportedCodings, supportedMessageCodings, FINAL)
.addStatement("this.$L = $L", supportedMessageCodings, supportedMessageCodings)
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + strategyFactory +
" a factory that creates an execution strategy for different {@link $L#id() id}s" +
lineSeparator(), RouteExecutionStrategy)
.addModifiers(PUBLIC)
.addParameter(GrpcRouteExecutionStrategyFactory, strategyFactory, FINAL)
.addStatement("this($L, $T.emptyList())", strategyFactory, Collections)
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + strategyFactory +
" a factory that creates an execution strategy for different {@link $L#id() id}s" +
lineSeparator(), RouteExecutionStrategy)
.addJavadoc(JAVADOC_PARAM + supportedMessageCodings + " the set of allowed encodings" +
lineSeparator())
.addJavadoc(JAVADOC_DEPRECATED +
" Use {@link #$L($T)}, {@link #$L($T)}, and {@link #$L($T)}." + lineSeparator(),
Builder, GrpcRouteExecutionStrategyFactory, bufferDecoderGroup, BufferDecoderGroup,
bufferEncoders, BufferEncoderList)
"Use {@link #$L($T)}, {@link #$L($T)}, and {@link #$L($T)}." + lineSeparator(),
Builder, RouteExecutionStrategyFactory, bufferDecoderGroup, BufferDecoderGroup,
bufferEncoders, Types.List)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addParameter(GrpcRouteExecutionStrategyFactory, strategyFactory, FINAL)
Expand Down Expand Up @@ -606,16 +626,26 @@ private TypeSpec.Builder addServiceFactory(final State state, final TypeSpec.Bui
// Add ServiceFactory constructors for blocking and async services with and without content codings and
// execution strategy
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + service + " a service to handle incoming requests" +
lineSeparator())
.addModifiers(PUBLIC)
.addParameter(state.serviceClass, service, FINAL)
.addStatement("this(new $T().$L)", builderClass,
serviceFactoryBuilderInitChain(state.serviceProto, false))
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + service + " a service to handle incoming requests" +
lineSeparator())
.addJavadoc(JAVADOC_PARAM + supportedMessageCodings + " the set of allowed encodings" +
lineSeparator())
.addJavadoc(JAVADOC_DEPRECATED +
" Use {@link $L#$L()}, {@link $L#$L($T)}, and {@link $L#$L($T)}." + lineSeparator(),
"Use {@link $L#$L()}, {@link $L#$L($T)}, and {@link $L#$L($T)}." + lineSeparator(),
Builder, Builder, Builder, bufferDecoderGroup, BufferDecoderGroup,
Builder, bufferEncoders, BufferEncoderList)
Builder, bufferEncoders, Types.List)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addParameter(state.serviceClass, service, FINAL)
Expand All @@ -624,17 +654,33 @@ private TypeSpec.Builder addServiceFactory(final State state, final TypeSpec.Bui
serviceFactoryBuilderInitChain(state.serviceProto, false))
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + service + " a service to handle incoming requests" +
lineSeparator())
.addJavadoc(JAVADOC_PARAM + strategyFactory +
" a factory that creates an execution strategy for different {@link $L#id() id}s" +
lineSeparator(), RouteExecutionStrategy)
.addModifiers(PUBLIC)
.addParameter(state.serviceClass, service, FINAL)
.addParameter(GrpcRouteExecutionStrategyFactory, strategyFactory, FINAL)
.addStatement("this(new $T($L).$L)", builderClass, strategyFactory,
serviceFactoryBuilderInitChain(state.serviceProto, false))
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + service + " a service to handle incoming requests" +
lineSeparator())
.addJavadoc(JAVADOC_PARAM + strategyFactory +
" a factory that creates an execution strategy for different {@link $L#id() id}s" +
lineSeparator(), RouteExecutionStrategy)
.addJavadoc(JAVADOC_PARAM + supportedMessageCodings + " the set of allowed encodings" +
lineSeparator())
.addJavadoc(JAVADOC_DEPRECATED +
" Use {@link $L#$L($T)}, {@link $L#$L($T)}, and {@link $L#$L($T)}." + lineSeparator(),
Builder, Builder, GrpcRouteExecutionStrategyFactory, Builder, bufferDecoderGroup,
BufferDecoderGroup, Builder, bufferEncoders, BufferEncoderList)
"Use {@link $L#$L($T)}, {@link $L#$L($T)}, and {@link $L#$L($T)}." + lineSeparator(),
Builder, Builder, RouteExecutionStrategyFactory, Builder, bufferDecoderGroup,
BufferDecoderGroup, Builder, bufferEncoders, Types.List)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addParameter(state.serviceClass, service, FINAL)
Expand All @@ -644,16 +690,26 @@ private TypeSpec.Builder addServiceFactory(final State state, final TypeSpec.Bui
supportedMessageCodings, serviceFactoryBuilderInitChain(state.serviceProto, false))
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + service + " a service to handle incoming requests" +
lineSeparator())
.addModifiers(PUBLIC)
.addParameter(state.blockingServiceClass, service, FINAL)
.addStatement("this(new $T().$L)", builderClass,
serviceFactoryBuilderInitChain(state.serviceProto, true))
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + service + " a service to handle incoming requests" +
lineSeparator())
.addJavadoc(JAVADOC_PARAM + supportedMessageCodings + " the set of allowed encodings" +
lineSeparator())
.addJavadoc(JAVADOC_DEPRECATED +
" Use {@link $L#$L()}, {@link $L#$L($T)}, and {@link $L#$L($T)}." + lineSeparator(),
"Use {@link $L#$L()}, {@link $L#$L($T)}, and {@link $L#$L($T)}." + lineSeparator(),
Builder, Builder, Builder, bufferDecoderGroup, BufferDecoderGroup,
Builder, bufferEncoders, BufferEncoderList)
Builder, bufferEncoders, Types.List)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addParameter(state.blockingServiceClass, service, FINAL)
Expand All @@ -662,17 +718,33 @@ supportedMessageCodings, serviceFactoryBuilderInitChain(state.serviceProto, fals
serviceFactoryBuilderInitChain(state.serviceProto, true))
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + service + " a service to handle incoming requests" +
lineSeparator())
.addJavadoc(JAVADOC_PARAM + strategyFactory +
" a factory that creates an execution strategy for different {@link $L#id() id}s" +
lineSeparator(), RouteExecutionStrategy)
.addModifiers(PUBLIC)
.addParameter(state.blockingServiceClass, service, FINAL)
.addParameter(GrpcRouteExecutionStrategyFactory, strategyFactory, FINAL)
.addStatement("this(new $T($L).$L)", builderClass, strategyFactory,
serviceFactoryBuilderInitChain(state.serviceProto, true))
.build())
.addMethod(constructorBuilder()
.addJavadoc(JAVADOC_CONSTRUCTOR_DEFAULT_STATEMENT)
.addJavadoc(lineSeparator())
.addJavadoc(JAVADOC_PARAM + service + " a service to handle incoming requests" +
lineSeparator())
.addJavadoc(JAVADOC_PARAM + strategyFactory +
" a factory that creates an execution strategy for different {@link $L#id() id}s" +
lineSeparator(), RouteExecutionStrategy)
.addJavadoc(JAVADOC_PARAM + supportedMessageCodings + " the set of allowed encodings" +
lineSeparator())
.addJavadoc(JAVADOC_DEPRECATED +
" Use {@link $L#$L($T)}, {@link $L#$L($T)}, and {@link $L#$L($T)}." + lineSeparator(),
Builder, Builder, GrpcRouteExecutionStrategyFactory, Builder, bufferDecoderGroup,
BufferDecoderGroup, Builder, bufferEncoders, BufferEncoderList)
"Use {@link $L#$L($T)}, {@link $L#$L($T)}, and {@link $L#$L($T)}." + lineSeparator(),
Builder, Builder, RouteExecutionStrategyFactory, Builder, bufferDecoderGroup,
BufferDecoderGroup, Builder, bufferEncoders, Types.List)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC)
.addParameter(state.blockingServiceClass, service, FINAL)
Expand Down Expand Up @@ -702,15 +774,15 @@ private TypeSpec.Builder addClientMetadata(final State state, final TypeSpec.Bui

final ClassName metaDataClassName = ClassName.bestGuess(name);
final TypeSpec classSpec = classBuilder(name)
.addJavadoc(JAVADOC_DEPRECATED + " This class will be removed in the future in favor of direct " +
.addJavadoc(JAVADOC_DEPRECATED + "This class will be removed in the future in favor of direct " +
"usage of {@link $T}. Deprecation of {@link $T#path()} renders this type unnecessary."
+ lineSeparator(), GrpcClientMetadata, GrpcClientMetadata)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC, STATIC, FINAL)
.superclass(DefaultGrpcClientMetadata)
.addField(FieldSpec.builder(metaDataClassName, INSTANCE)
.addJavadoc(JAVADOC_DEPRECATED +
" This class will be removed in the future in favor of direct usage of {@link $T}."
"This class will be removed in the future in favor of direct usage of {@link $T}."
+ lineSeparator(), GrpcClientMetadata)
.addAnnotation(Deprecated.class)
.addModifiers(PUBLIC, STATIC, FINAL) // redundant, default for interface field
Expand Down Expand Up @@ -791,10 +863,10 @@ private TypeSpec.Builder addClientInterfaces(final State state, final TypeSpec.B
ClassName inClass = messageTypesMap.get(clientMetaData.methodProto.getInputType());
b.addModifiers(ABSTRACT).addParameter(clientMetaData.className, metadata)
.addAnnotation(Deprecated.class)
.addJavadoc(JAVADOC_DEPRECATED + " Use {@link #$L($T,$T)}." + lineSeparator(),
.addJavadoc(JAVADOC_DEPRECATED + "Use {@link #$L($T,$T)}." + lineSeparator(),
methodName,
GrpcClientMetadata, clientMetaData.methodProto.getClientStreaming() ?
ParameterizedTypeName.get(Publisher, inClass) : inClass);
Publisher : inClass);
if (printJavaDocs) {
extractJavaDocComments(state, methodIndex, b);
b.addJavadoc(JAVADOC_PARAM + metadata +
Expand Down Expand Up @@ -828,9 +900,9 @@ private TypeSpec.Builder addClientInterfaces(final State state, final TypeSpec.B
ClassName inClass = messageTypesMap.get(clientMetaData.methodProto.getInputType());
b.addModifiers(ABSTRACT).addParameter(clientMetaData.className, metadata)
.addAnnotation(Deprecated.class)
.addJavadoc(JAVADOC_DEPRECATED + " Use {@link #$L($T,$T)}." + lineSeparator(),
.addJavadoc(JAVADOC_DEPRECATED + "Use {@link #$L($T,$T)}." + lineSeparator(),
methodName, GrpcClientMetadata, clientMetaData.methodProto.getClientStreaming() ?
ParameterizedTypeName.get(ClassName.get(Iterable.class), inClass) : inClass);
Types.Iterable : inClass);
if (printJavaDocs) {
extractJavaDocComments(state, methodIndex, b);
b.addJavadoc(JAVADOC_PARAM + metadata +
Expand Down
Loading

0 comments on commit dbcbfd8

Please sign in to comment.