Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix eclipse warnings #1916

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-1916.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Fix warnings in eclipse
links:
- https://github.com/palantir/conjure-java/pull/1916

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@
import com.palantir.dialogue.TypeMarker;
import com.palantir.logsafe.SafeArg;
import com.palantir.logsafe.exceptions.SafeIllegalStateException;
import com.squareup.javapoet.AnnotationSpec;
import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.FieldSpec;
Expand All @@ -75,6 +74,7 @@ public final class DefaultStaticFactoryMethodGenerator implements StaticFactoryM
private final ParameterTypeMapper parameterTypes;
private final ReturnTypeMapper returnTypes;
private final StaticFactoryMethodType methodType;
private final ThreadLocal<Boolean> shouldGeneratePlainSerDe = ThreadLocal.withInitial(() -> false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ThreadLocals have a bunch of gotchas that can be really troublesome to debug. Is there a reason we can't pass info up and down the stack to set this as needed all within the generate() method and its calls?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


public DefaultStaticFactoryMethodGenerator(
Options options,
Expand All @@ -91,14 +91,10 @@ public DefaultStaticFactoryMethodGenerator(

@Override
public MethodSpec generate(ServiceDefinition def) {
shouldGeneratePlainSerDe.set(false);
ClassName className = getClassName(def);
TypeSpec.Builder impl = TypeSpec.anonymousClassBuilder("").addSuperinterface(className);

impl.addField(FieldSpec.builder(PlainSerDe.class, PLAIN_SER_DE)
.addModifiers(Modifier.PRIVATE, Modifier.FINAL)
.initializer(CodeBlock.of("$L.plainSerDe()", StaticFactoryMethodGenerator.RUNTIME))
.build());

def.getEndpoints().forEach(endpoint -> {
endpoint.getArgs().stream()
.filter(arg -> arg.getParamType().accept(ParameterTypeVisitor.IS_BODY))
Expand All @@ -113,6 +109,13 @@ public MethodSpec generate(ServiceDefinition def) {

impl.addMethod(DefaultStaticFactoryMethodGenerator.toStringMethod(className));

if (shouldGeneratePlainSerDe.get()) {
impl.addField(FieldSpec.builder(PlainSerDe.class, PLAIN_SER_DE)
.addModifiers(Modifier.PRIVATE, Modifier.FINAL)
.initializer(CodeBlock.of("$L.plainSerDe()", StaticFactoryMethodGenerator.RUNTIME))
.build());
}

String javadoc = methodType.switchBy(
"Creates a synchronous/blocking client for a $L service.",
"Creates an " + "asynchronous/non-blocking client for a $L service.");
Expand All @@ -124,6 +127,7 @@ public MethodSpec generate(ServiceDefinition def) {
.addParameter(ConjureRuntime.class, StaticFactoryMethodGenerator.RUNTIME)
.addCode(CodeBlock.builder().add("return $L;", impl.build()).build())
.build();

return method;
}

Expand Down Expand Up @@ -200,20 +204,12 @@ private MethodSpec clientImpl(EndpointDefinition def) {
.addParameters(params)
.addAnnotation(Override.class);

if (def.getDeprecated().isPresent()) {
methodBuilder.addAnnotation(AnnotationSpec.builder(SuppressWarnings.class)
.addMember("value", "$S", "deprecation")
.build());
}

TypeName returnType =
methodType.switchBy(returnTypes.baseType(def.getReturns()), returnTypes.async(def.getReturns()));
methodBuilder.returns(returnType);

CodeBlock.Builder requestParams = CodeBlock.builder();
def.getAuth()
.map(DefaultStaticFactoryMethodGenerator::generateAuthHeader)
.ifPresent(requestParams::add);
def.getAuth().map(this::generateAuthHeader).ifPresent(requestParams::add);

def.getArgs().stream()
.map(param -> generateParam(def.getEndpointName().get(), param))
Expand Down Expand Up @@ -311,6 +307,7 @@ private CodeBlock generatePlainSerializer(String method, String key, CodeBlock a
return type.accept(new Type.Visitor<CodeBlock>() {
@Override
public CodeBlock visitPrimitive(PrimitiveType primitiveType) {
shouldGeneratePlainSerDe.set(true);
return CodeBlock.of(
"$L.$L($S, $L.serialize$L($L));",
"_request",
Expand Down Expand Up @@ -388,7 +385,7 @@ private CodeBlock visitCollection(Type itemType) {
});
}

private static CodeBlock generateAuthHeader(AuthType auth) {
private CodeBlock generateAuthHeader(AuthType auth) {
return auth.accept(new AuthType.Visitor<CodeBlock>() {
@Override
public CodeBlock visitHeader(HeaderAuthType value) {
Expand All @@ -401,6 +398,7 @@ public CodeBlock visitHeader(HeaderAuthType value) {

@Override
public CodeBlock visitCookie(CookieAuthType value) {
shouldGeneratePlainSerDe.set(true);
return CodeBlock.of(
"$L.putHeaderParams($S, \"$L=\" + $L.serializeBearerToken($L));",
REQUEST,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -910,11 +910,6 @@ private static MethodSpec createWrapperAcceptMethod(
} else {
methodBuilder.addStatement("return $N.$N($N)", visitor, visitMethodName, valueName);
}
if (isDeprecated) {
methodBuilder.addAnnotation(AnnotationSpec.builder(SuppressWarnings.class)
.addMember("value", "$S", "deprecation")
.build());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change to remove the SuppressWarnings annotations seems independent from the plainSerDe change. Can they be separate PRs, or are they interrelated somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved deprecation stuff to #1917

return methodBuilder.build();
}

Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading