Skip to content

Commit

Permalink
[ggj][codegen] fix: replace Objects.isNull() with equality check for …
Browse files Browse the repository at this point in the history
…Java 7 compat (#583)

* fix: fix dep ordering in Bazel dedupe rules

* fix: replace Objects.isNull() with equality check for Java 7 compat
  • Loading branch information
miraleung authored Dec 3, 2020
1 parent b2f8f12 commit 2db0bc0
Show file tree
Hide file tree
Showing 47 changed files with 250 additions and 333 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,10 @@
import com.google.api.generator.engine.ast.MethodDefinition;
import com.google.api.generator.engine.ast.MethodInvocationExpr;
import com.google.api.generator.engine.ast.NewObjectExpr;
import com.google.api.generator.engine.ast.NullObjectValue;
import com.google.api.generator.engine.ast.PrimitiveValue;
import com.google.api.generator.engine.ast.Reference;
import com.google.api.generator.engine.ast.RelationalOperationExpr;
import com.google.api.generator.engine.ast.ScopeNode;
import com.google.api.generator.engine.ast.Statement;
import com.google.api.generator.engine.ast.TypeNode;
Expand All @@ -50,7 +52,6 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;

public class BatchingDescriptorComposer {
Expand Down Expand Up @@ -203,12 +204,8 @@ private static MethodDefinition createGetRequestBuilderMethod(
Arrays.asList(
IfStatement.builder()
.setConditionExpr(
MethodInvocationExpr.builder()
.setStaticReferenceType(toType(Objects.class))
.setMethodName("isNull")
.setArguments(builderVarExpr)
.setReturnType(TypeNode.BOOLEAN)
.build())
RelationalOperationExpr.equalToWithExprs(
builderVarExpr, ValueExpr.withValue(NullObjectValue.create())))
.setBody(Arrays.asList(ExprStatement.withExpr(toBuilderExpr)))
.setElseBody(Arrays.asList(ExprStatement.withExpr(addAllExpr)))
.build()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import com.google.api.generator.engine.ast.ThisObjectValue;
import com.google.api.generator.engine.ast.ThrowExpr;
import com.google.api.generator.engine.ast.TypeNode;
import com.google.api.generator.engine.ast.UnaryOperationExpr;
import com.google.api.generator.engine.ast.ValueExpr;
import com.google.api.generator.engine.ast.VaporReference;
import com.google.api.generator.engine.ast.Variable;
Expand Down Expand Up @@ -860,17 +859,13 @@ private static MethodDefinition createToStringListMethod(TypeNode thisClassType)
.build())
.build();

// TODO(miraleung): Use equality check instead of Objects.
VariableExpr valueVarExpr =
VariableExpr.withVariable(
Variable.builder().setName("value").setType(thisClassType).build());
// We use an equality check instead of Objects.isNull() for Java 7 compatibility.
Expr isNullCheck =
MethodInvocationExpr.builder()
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
.setMethodName("isNull")
.setArguments(valueVarExpr)
.setReturnType(TypeNode.BOOLEAN)
.build();
RelationalOperationExpr.equalToWithExprs(
valueVarExpr, ValueExpr.withValue(NullObjectValue.create()));
Statement listAddEmptyStringStatement =
ExprStatement.withExpr(
MethodInvocationExpr.builder()
Expand Down Expand Up @@ -1007,13 +1002,8 @@ private static MethodDefinition createGetFieldValuesMapMethod(
.setArguments(ValueExpr.withValue(tokenStrVal), tokenVarExpr)
.build();
Expr notNullCheckExpr =
UnaryOperationExpr.logicalNotWithExpr(
MethodInvocationExpr.builder()
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
.setMethodName("isNull")
.setArguments(tokenVarExpr)
.setReturnType(TypeNode.BOOLEAN)
.build());
RelationalOperationExpr.notEqualToWithExprs(
tokenVarExpr, ValueExpr.withValue(NullObjectValue.create()));
tokenIfStatements.add(
IfStatement.builder()
.setConditionExpr(notNullCheckExpr)
Expand All @@ -1040,14 +1030,9 @@ private static MethodDefinition createGetFieldValuesMapMethod(
middleIfBlockStatements.add(ExprStatement.withExpr(fieldValuesMapAssignExpr));

// Middle if-block, i.e. `if (fieldValuesMap == null)`.
// TODO(miraleung): Use eq operator here.
MethodInvocationExpr fieldValuesMapNullCheckExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
.setMethodName("isNull")
.setArguments(fieldValuesMapVarExpr)
.setReturnType(TypeNode.BOOLEAN)
.build();
Expr fieldValuesMapNullCheckExpr =
RelationalOperationExpr.equalToWithExprs(
fieldValuesMapVarExpr, ValueExpr.withValue(NullObjectValue.create()));
IfStatement fieldValuesMapIfStatement =
IfStatement.builder()
.setConditionExpr(fieldValuesMapNullCheckExpr)
Expand Down Expand Up @@ -1140,13 +1125,8 @@ private static MethodDefinition createToStringMethod(
VariableExpr fixedValueVarExpr = FIXED_CLASS_VARS.get("fixedValue");
// Code: return fixedValue != null ? fixedValue : pathTemplate.instantiate(getFieldValuesMap())
Expr fixedValueNullCheck =
UnaryOperationExpr.logicalNotWithExpr(
MethodInvocationExpr.builder()
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
.setMethodName("isNull")
.setArguments(fixedValueVarExpr)
.setReturnType(TypeNode.BOOLEAN)
.build());
RelationalOperationExpr.notEqualToWithExprs(
fixedValueVarExpr, ValueExpr.withValue(NullObjectValue.create()));

MethodInvocationExpr instantiateExpr =
MethodInvocationExpr.builder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import com.google.api.generator.engine.ast.PrimitiveValue;
import com.google.api.generator.engine.ast.Reference;
import com.google.api.generator.engine.ast.ReferenceConstructorExpr;
import com.google.api.generator.engine.ast.RelationalOperationExpr;
import com.google.api.generator.engine.ast.ScopeNode;
import com.google.api.generator.engine.ast.Statement;
import com.google.api.generator.engine.ast.SuperObjectValue;
Expand Down Expand Up @@ -1347,14 +1348,8 @@ static MethodInvocationExpr buildNestedSetterInvocationExpr(
VariableExpr.withVariable(
Variable.builder().setName(argumentName).setType(argumentType).build());
if (argument.isResourceNameHelper()) {
MethodInvocationExpr isNullCheckExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(OBJECTS_TYPE)
.setMethodName("isNull")
.setArguments(Arrays.asList(argVarExpr))
.setReturnType(TypeNode.BOOLEAN)
.build();
Expr nullExpr = ValueExpr.withValue(NullObjectValue.create());
Expr isNullCheckExpr = RelationalOperationExpr.equalToWithExprs(argVarExpr, nullExpr);
MethodInvocationExpr toStringExpr =
MethodInvocationExpr.builder()
.setExprReferenceExpr(argVarExpr)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import com.google.api.generator.engine.ast.NullObjectValue;
import com.google.api.generator.engine.ast.Reference;
import com.google.api.generator.engine.ast.ReferenceConstructorExpr;
import com.google.api.generator.engine.ast.RelationalOperationExpr;
import com.google.api.generator.engine.ast.ReturnExpr;
import com.google.api.generator.engine.ast.ScopeNode;
import com.google.api.generator.engine.ast.Statement;
Expand Down Expand Up @@ -545,13 +546,8 @@ private static Expr createPagedListDescriptorAssignExpr(
.setReturnType(returnType)
.build();
Expr conditionExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(
TypeNode.withReference(ConcreteReference.withClazz(Objects.class)))
.setMethodName("isNull")
.setArguments(getResponsesListExpr)
.setReturnType(TypeNode.BOOLEAN)
.build();
RelationalOperationExpr.equalToWithExprs(
getResponsesListExpr, ValueExpr.withValue(NullObjectValue.create()));
Expr thenExpr =
MethodInvocationExpr.builder()
.setStaticReferenceType(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public class AgentName implements ResourceName {
public static List<String> toStringList(List<AgentName> values) {
List<String> list = new ArrayList<>(values.size());
for (AgentName value : values) {
if (Objects.isNull(value)) {
if (value == null) {
list.add("");
} else {
list.add(value.toString());
Expand All @@ -137,14 +137,14 @@ public class AgentName implements ResourceName {

@Override
public Map<String, String> getFieldValuesMap() {
if (Objects.isNull(fieldValuesMap)) {
if (fieldValuesMap == null) {
synchronized (this) {
if (Objects.isNull(fieldValuesMap)) {
if (fieldValuesMap == null) {
ImmutableMap.Builder<String, String> fieldMapBuilder = ImmutableMap.builder();
if (!Objects.isNull(project)) {
if (project != null) {
fieldMapBuilder.put("project", project);
}
if (!Objects.isNull(location)) {
if (location != null) {
fieldMapBuilder.put("location", location);
}
fieldValuesMap = fieldMapBuilder.build();
Expand All @@ -160,7 +160,7 @@ public class AgentName implements ResourceName {

@Override
public String toString() {
return !Objects.isNull(fixedValue) ? fixedValue : pathTemplate.instantiate(getFieldValuesMap());
return fixedValue != null ? fixedValue : pathTemplate.instantiate(getFieldValuesMap());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ return new RequestBuilder<WriteLogEntriesRequest>() {
private WriteLogEntriesRequest.Builder builder;
@Override
public void appendRequest(WriteLogEntriesRequest request) {
if (Objects.isNull(builder)) {
if (builder == null) {
builder = request.toBuilder();
} else {
builder.addAllEntries(request.getEntriesList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ return new RequestBuilder<PublishRequest>() {
private PublishRequest.Builder builder;
@Override
public void appendRequest(PublishRequest request) {
if (Objects.isNull(builder)) {
if (builder == null) {
builder = request.toBuilder();
} else {
builder.addAllMessages(request.getMessagesList());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public class BillingAccountLocationName implements ResourceName {
public static List<String> toStringList(List<BillingAccountLocationName> values) {
List<String> list = new ArrayList<>(values.size());
for (BillingAccountLocationName value : values) {
if (Objects.isNull(value)) {
if (value == null) {
list.add("");
} else {
list.add(value.toString());
Expand All @@ -92,14 +92,14 @@ public class BillingAccountLocationName implements ResourceName {

@Override
public Map<String, String> getFieldValuesMap() {
if (Objects.isNull(fieldValuesMap)) {
if (fieldValuesMap == null) {
synchronized (this) {
if (Objects.isNull(fieldValuesMap)) {
if (fieldValuesMap == null) {
ImmutableMap.Builder<String, String> fieldMapBuilder = ImmutableMap.builder();
if (!Objects.isNull(billingAccount)) {
if (billingAccount != null) {
fieldMapBuilder.put("billing_account", billingAccount);
}
if (!Objects.isNull(location)) {
if (location != null) {
fieldMapBuilder.put("location", location);
}
fieldValuesMap = fieldMapBuilder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import com.google.showcase.v1beta1.stub.EchoStub;
import com.google.showcase.v1beta1.stub.EchoStubSettings;
import java.io.IOException;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import javax.annotation.Generated;

Expand Down Expand Up @@ -160,9 +159,7 @@ public class EchoClient implements BackgroundResource {
*/
public final EchoResponse echo(ResourceName parent) {
EchoRequest request =
EchoRequest.newBuilder()
.setParent(Objects.isNull(parent) ? null : parent.toString())
.build();
EchoRequest.newBuilder().setParent(parent == null ? null : parent.toString()).build();
return echo(request);
}

Expand All @@ -183,7 +180,7 @@ public class EchoClient implements BackgroundResource {
*/
public final EchoResponse echo(FoobarName name) {
EchoRequest request =
EchoRequest.newBuilder().setName(Objects.isNull(name) ? null : name.toString()).build();
EchoRequest.newBuilder().setName(name == null ? null : name.toString()).build();
return echo(request);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ import com.google.showcase.v1beta1.WaitRequest;
import com.google.showcase.v1beta1.WaitResponse;
import java.io.IOException;
import java.util.List;
import java.util.Objects;
import javax.annotation.Generated;
import org.threeten.bp.Duration;

Expand Down Expand Up @@ -135,7 +134,7 @@ public class EchoStubSettings extends StubSettings<EchoStubSettings> {

@Override
public Iterable<EchoResponse> extractResources(PagedExpandResponse payload) {
return Objects.isNull(payload.getResponsesList())
return payload.getResponsesList() == null
? ImmutableList.<EchoResponse>of()
: payload.getResponsesList();
}
Expand Down Expand Up @@ -171,7 +170,7 @@ public class EchoStubSettings extends StubSettings<EchoStubSettings> {

@Override
public Iterable<EchoResponse> extractResources(PagedExpandResponse payload) {
return Objects.isNull(payload.getResponsesList())
return payload.getResponsesList() == null
? ImmutableList.<EchoResponse>of()
: payload.getResponsesList();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ public class FoobarName implements ResourceName {
public static List<String> toStringList(List<FoobarName> values) {
List<String> list = new ArrayList<>(values.size());
for (FoobarName value : values) {
if (Objects.isNull(value)) {
if (value == null) {
list.add("");
} else {
list.add(value.toString());
Expand All @@ -225,20 +225,20 @@ public class FoobarName implements ResourceName {

@Override
public Map<String, String> getFieldValuesMap() {
if (Objects.isNull(fieldValuesMap)) {
if (fieldValuesMap == null) {
synchronized (this) {
if (Objects.isNull(fieldValuesMap)) {
if (fieldValuesMap == null) {
ImmutableMap.Builder<String, String> fieldMapBuilder = ImmutableMap.builder();
if (!Objects.isNull(project)) {
if (project != null) {
fieldMapBuilder.put("project", project);
}
if (!Objects.isNull(foobar)) {
if (foobar != null) {
fieldMapBuilder.put("foobar", foobar);
}
if (!Objects.isNull(variant)) {
if (variant != null) {
fieldMapBuilder.put("variant", variant);
}
if (!Objects.isNull(barFoo)) {
if (barFoo != null) {
fieldMapBuilder.put("bar_foo", barFoo);
}
fieldValuesMap = fieldMapBuilder.build();
Expand All @@ -254,7 +254,7 @@ public class FoobarName implements ResourceName {

@Override
public String toString() {
return !Objects.isNull(fixedValue) ? fixedValue : pathTemplate.instantiate(getFieldValuesMap());
return fixedValue != null ? fixedValue : pathTemplate.instantiate(getFieldValuesMap());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ import com.google.showcase.v1beta1.stub.IdentityStub;
import com.google.showcase.v1beta1.stub.IdentityStubSettings;
import java.io.IOException;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
import javax.annotation.Generated;

Expand Down Expand Up @@ -194,7 +193,7 @@ public class IdentityClient implements BackgroundResource {
*/
public final User getUser(UserName name) {
GetUserRequest request =
GetUserRequest.newBuilder().setName(Objects.isNull(name) ? null : name.toString()).build();
GetUserRequest.newBuilder().setName(name == null ? null : name.toString()).build();
return getUser(request);
}

Expand Down Expand Up @@ -245,9 +244,7 @@ public class IdentityClient implements BackgroundResource {
*/
public final void deleteUser(UserName name) {
DeleteUserRequest request =
DeleteUserRequest.newBuilder()
.setName(Objects.isNull(name) ? null : name.toString())
.build();
DeleteUserRequest.newBuilder().setName(name == null ? null : name.toString()).build();
deleteUser(request);
}

Expand Down
Loading

0 comments on commit 2db0bc0

Please sign in to comment.