Skip to content

Commit

Permalink
Resolve javadocAll errors (#2195)
Browse files Browse the repository at this point in the history
Motivation:
The release.sh script runs `javadocAll` task on JDK17 which
found a few errors.

Motivation:
- JDK8 javadoc generation has an JDK8 erroneous javadoc
  `warning - Tag @link: can't find` from generated code.
  Workaround the issue by using canonical type name in the link.
- Remove `h3` usage in Test sources
- Remove exclude of generated code that is linked in javadocs
  from non-generated code.
  • Loading branch information
Scottmitch authored Apr 18, 2022
1 parent d7a5f10 commit 42b1c49
Show file tree
Hide file tree
Showing 9 changed files with 99 additions and 48 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,8 @@
* invoked for every {@link #subscribe(Subscriber)} invocation, and the result is used as the delegate for subsequent
* {@link #onSubscribe(Cancellable)}, {@link #onComplete()}, and
* {@link #onError(Throwable)} calls. See {@link Builder} for more information.
*
* <h3>Defaults</h3>
* <p>
* Defaults:
* <ul>
* <li>Allows sequential but not concurrent subscribers.</li>
* <li>Sends {@link #onSubscribe(Cancellable)} automatically when subscribed to.</li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,8 @@
* invoked for every {@link #subscribe(Subscriber)} invocation, and the result is used as the delegate for subsequent
* {@link #onSubscribe(Subscription)}, {@link #onNext(Object[])}, {@link #onComplete()}, and
* {@link #onError(Throwable)} calls. See {@link Builder} for more information.
* <h3>Defaults</h3>
* <p>
* Defaults:
* <ul>
* <li>Allows sequential but not concurrent subscribers.</li>
* <li>Asserts that {@link #onNext(Object[])} is not called without sufficient demand.</li>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@
* invoked for every {@link #subscribe(Subscriber)} invocation, and the result is used as the delegate for subsequent
* {@link #onSubscribe(Cancellable)}, {@link #onSuccess(Object)}, and
* {@link #onError(Throwable)} calls. See {@link Builder} for more information.
*
* <h3>Defaults</h3>
* <p>
* Defaults:
* <ul>
* <li>Allows sequential but not concurrent subscribers.</li>
* <li>Sends {@link #onSubscribe(Cancellable)} automatically when subscribed to.</li>
Expand Down
4 changes: 0 additions & 4 deletions servicetalk-grpc-health/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ sourceSets {
}
}

javadoc {
exclude "**/${generatedJavaPkg.replace('.', '/')}/**"
}

protobuf {
protoc {
artifact = "com.google.protobuf:protoc:$protobufVersion"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ final class FileDescriptor implements GenerationContext {
private final boolean multipleClassFiles;
private final String javaPackageName;
private final String outerClassName;
private final String javaOuterScope;
@Nullable
private final String typeNameSuffix;
private final List<TypeSpec.Builder> serviceClassBuilders;
Expand Down Expand Up @@ -96,6 +97,7 @@ final class FileDescriptor implements GenerationContext {
javaPackageName = inferJavaPackageName(protoPackageName, sanitizedProtoFileName);
outerClassName = inferOuterClassName(sanitizedProtoFileName, protoFile);
}
javaOuterScope = multipleClassFiles ? javaPackageName() : javaPackageName() + '.' + outerJavaClassName();
reservedJavaTypeName.add(outerClassName);

serviceClassBuilders = new ArrayList<>(protoFile.getServiceCount());
Expand All @@ -118,8 +120,7 @@ List<ServiceDescriptorProto> protoServices() {
Map<String, ClassName> messageTypesMap() {
final Map<String, ClassName> messageTypesMap = new HashMap<>(protoFile.getMessageTypeCount());
addMessageTypes(protoFile.getMessageTypeList(), protoPackageName != null ? '.' + protoPackageName : null,
multipleClassFiles ? javaPackageName() : javaPackageName() + '.' + outerJavaClassName(),
messageTypesMap);
javaOuterScope, messageTypesMap);
return messageTypesMap;
}

Expand Down Expand Up @@ -158,7 +159,12 @@ public String deconflictJavaTypeName(final String name) {
}

@Override
public TypeSpec.Builder newServiceClassBuilder(final ServiceDescriptorProto serviceProto) {
public String deconflictJavaTypeName(final String outerClassName, final String name) {
return javaOuterScope + '.' + outerClassName + '.' + deconflictJavaTypeName(sanitizeClassName(name));
}

@Override
public ServiceClassBuilder newServiceClassBuilder(final ServiceDescriptorProto serviceProto) {
final String rawClassName = typeNameSuffix == null ? serviceProto.getName() :
serviceProto.getName() + typeNameSuffix;
final String className = deconflictJavaTypeName(sanitizeClassName(rawClassName));
Expand All @@ -178,7 +184,7 @@ public TypeSpec.Builder newServiceClassBuilder(final ServiceDescriptorProto serv

serviceClassBuilders.add(builder);
protoForServiceBuilder.put(builder, serviceProto);
return builder;
return new ServiceClassBuilder(builder, className);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,28 +17,44 @@

import com.google.protobuf.DescriptorProtos.MethodDescriptorProto;
import com.google.protobuf.DescriptorProtos.ServiceDescriptorProto;
import com.squareup.javapoet.TypeSpec;

interface GenerationContext {
/**
* Return a canonical and unique type name based upon the provided type name.
* The returned name will not conflict with any other identically named types
* in the same context.
* Return a canonical and unique type name based upon the provided type name. The returned name will not conflict
* with any other identically named types in the same context.
*
* @param name The Java type name to deconflict
* @return The deconflicted, possibly unchanged, Java type name.
*/
String deconflictJavaTypeName(String name);

/**
* Create and return the builder for a service class described by the provided
* descriptor. The builder will be registered for the destination file and the
* generator responsible for filling in the builder will
* Return a qualified, canonical, and unique type name based upon the provided type name. The returned name will not
* conflict with any other identically named types in the same context.
*
* @param outerClassName The outer class name that contains this type. This will be used to generate a
* more fully qualified return value.
* @param name The Java type name to deconflict
* @return Deconflicted, possibly unchanged, Java type name that is qualified with the outer java
* package+typename+{@code outerClassName} scope.
*/
String deconflictJavaTypeName(String outerClassName, String name);

/**
* Create and return the builder for a service class described by the provided descriptor. The builder will be
* registered for the destination file and the generator responsible for filling in the builder will
*
* @param serviceProto The service descriptor being created.
* @return a new builder for the service class
*/
TypeSpec.Builder newServiceClassBuilder(ServiceDescriptorProto serviceProto);
ServiceClassBuilder newServiceClassBuilder(ServiceDescriptorProto serviceProto);

/**
* Get the <a href="https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md">gRPC H2 path</a> for a method.
* @param serviceProto protobuf descriptor for the service.
* @param methodProto protobuf descriptor for the method.
* @return the <a href="https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md">gRPC H2 path</a> for a
* method.
*/
String methodPath(ServiceDescriptorProto serviceProto, MethodDescriptorProto methodProto);
}
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@
import static io.servicetalk.grpc.protoc.Words.COMMENT_POST_TAG;
import static io.servicetalk.grpc.protoc.Words.COMMENT_PRE_TAG;
import static io.servicetalk.grpc.protoc.Words.Call;
import static io.servicetalk.grpc.protoc.Words.Client;
import static io.servicetalk.grpc.protoc.Words.Default;
import static io.servicetalk.grpc.protoc.Words.Factory;
import static io.servicetalk.grpc.protoc.Words.INSTANCE;
Expand Down Expand Up @@ -209,19 +210,24 @@ private static final class State {
final ClassName clientClass;
final ClassName blockingClientClass;

private State(final ServiceDescriptorProto serviceProto, String name, int serviceIndex) {
private State(ServiceDescriptorProto serviceProto, GenerationContext context, String outerClassName,
int serviceIndex) {
this.serviceProto = serviceProto;
this.serviceIndex = serviceIndex;

final String sanitizedProtoName = sanitizeIdentifier(serviceProto.getName(), false);

// Filled in during addServiceRpcInterfaces()
serviceRpcInterfaces = new ArrayList<>(2 * serviceProto.getMethodCount());
serviceClass = ClassName.bestGuess(name);
blockingServiceClass = ClassName.bestGuess(Blocking + name);
serviceClass = ClassName.bestGuess(context.deconflictJavaTypeName(outerClassName,
sanitizedProtoName + Service));
blockingServiceClass = serviceClass.peerClass(Blocking + serviceClass.simpleName());
serviceFactoryClass = serviceClass.peerClass(Service + Factory);

// Filled in during addClientMetadata()
clientMetaDatas = new ArrayList<>(serviceProto.getMethodCount());
clientClass = ClassName.bestGuess(sanitizeIdentifier(serviceProto.getName(), false) + "Client");
clientClass = ClassName.bestGuess(context.deconflictJavaTypeName(outerClassName,
sanitizedProtoName + Client));
blockingClientClass = clientClass.peerClass(Blocking + clientClass.simpleName());
}
}
Expand All @@ -242,34 +248,30 @@ private State(final ServiceDescriptorProto serviceProto, String name, int servic
/**
* Generate Service class for the provided proto service descriptor.
*
*
* @param serviceProto The service descriptor.
* @param serviceIndex The index of the service within the current file (0 based).
* @return The service class builder
*/
TypeSpec.Builder generate(FileDescriptor f, final ServiceDescriptorProto serviceProto, final int serviceIndex) {
final String name = context.deconflictJavaTypeName(
sanitizeIdentifier(serviceProto.getName(), false) + Service);
final State state = new State(serviceProto, name, serviceIndex);

final TypeSpec.Builder serviceClassBuilder = context.newServiceClassBuilder(serviceProto);
final ServiceClassBuilder container = context.newServiceClassBuilder(serviceProto);
final State state = new State(serviceProto, context, container.className, serviceIndex);
if (printJavaDocs) {
serviceClassBuilder.addJavadoc("Class for $L Service", serviceProto.getName());
container.builder.addJavadoc("Class for $L Service", serviceProto.getName());
}

addSerializationProviderInit(state, serviceClassBuilder);
addSerializationProviderInit(state, container.builder);

addServiceRpcInterfaces(state, serviceClassBuilder);
addServiceInterfaces(state, serviceClassBuilder);
addServiceFactory(state, serviceClassBuilder);
addServiceRpcInterfaces(state, container.builder);
addServiceInterfaces(state, container.builder);
addServiceFactory(state, container.builder);

addClientMetadata(state, serviceClassBuilder);
addClientInterfaces(state, serviceClassBuilder);
addClientFactory(state, serviceClassBuilder);
addClientMetadata(state, container.builder);
addClientInterfaces(state, container.builder);
addClientFactory(state, container.builder);
// this empty class is a placeholder and get replaced with insertion point comment
serviceClassBuilder.addType(TypeSpec.classBuilder("__" + serviceFQN(f, serviceProto)).build());
container.builder.addType(TypeSpec.classBuilder("__" + serviceFQN(f, serviceProto)).build());

return serviceClassBuilder;
return container.builder;
}

private String serviceFQN(FileDescriptor f, ServiceDescriptorProto serviceDescriptorProto) {
Expand Down Expand Up @@ -615,8 +617,9 @@ private TypeSpec.Builder addServiceFactory(final State state, final TypeSpec.Bui
.addJavadoc(JAVADOC_PARAM + service + " the {@link $T} implementation to add." + lineSeparator(),
state.blockingServiceClass)
.addJavadoc(JAVADOC_RETURN + "this." + lineSeparator())
.addJavadoc(JAVADOC_DEPRECATED + "Use {@link #$L($T)}." + lineSeparator(), addBlockingService,
state.blockingServiceClass)
.addJavadoc(JAVADOC_DEPRECATED + "Use {@link #$L($L)}." + lineSeparator(),
// force canonicalName to work around JDK8 erroneous javadoc `warning - Tag @link: can't find`
addBlockingService, state.blockingServiceClass.canonicalName())
.returns(builderClass)
.addParameter(state.blockingServiceClass, service, FINAL)
.addStatement("return $L($L)", addBlockingService, service)
Expand Down Expand Up @@ -953,12 +956,12 @@ private TypeSpec.Builder addClientInterfaces(final State state, final TypeSpec.B
}

private TypeSpec.Builder addClientFactory(final State state, final TypeSpec.Builder serviceClassBuilder) {
final ClassName clientFactoryClass = state.clientClass.peerClass("Client" + Factory);
final ClassName defaultClientClass = clientFactoryClass.peerClass(Default + state.clientClass.simpleName());
final ClassName defaultBlockingClientClass = clientFactoryClass.peerClass(Default +
state.blockingClientClass.simpleName());
final ClassName clientToBlockingClientClass = clientFactoryClass.peerClass(state.clientClass.simpleName() + To +
final ClassName clientFactoryClass = state.clientClass.peerClass(Client + Factory);
final ClassName defaultClientClass = clientFactoryClass.nestedClass(Default + state.clientClass.simpleName());
final ClassName defaultBlockingClientClass = clientFactoryClass.nestedClass(Default +
state.blockingClientClass.simpleName());
final ClassName clientToBlockingClientClass = clientFactoryClass.nestedClass(state.clientClass.simpleName() + To
+ state.blockingClientClass.simpleName());

final TypeSpec.Builder clientFactorySpecBuilder = classBuilder(clientFactoryClass)
.addModifiers(PUBLIC, STATIC)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/*
* Copyright © 2022 Apple Inc. and the ServiceTalk project authors
*
* Licensed 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 io.servicetalk.grpc.protoc;

import com.squareup.javapoet.TypeSpec;

final class ServiceClassBuilder {
final TypeSpec.Builder builder;
final String className;

ServiceClassBuilder(final TypeSpec.Builder builder, final String className) {
this.builder = builder;
this.className = className;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ final class Words {
static final String methodDescriptor = "methodDescriptor";
static final String methodDescriptors = methodDescriptor + "s";
static final String Service = "Service";
static final String Client = "Client";
static final String Blocking = "Blocking";
static final String Builder = "Builder";
static final String Call = "Call";
Expand Down

0 comments on commit 42b1c49

Please sign in to comment.