Skip to content
This repository was archived by the owner on May 14, 2026. It is now read-only.

Commit ba22ec5

Browse files
miraleungmiraleung
authored
fix: use Objects.isNull instead of null equality (#297)
1 parent 694845e commit ba22ec5

4 files changed

Lines changed: 44 additions & 47 deletions

File tree

‎src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.google.api.generator.engine.ast.ThisObjectValue;
4040
import com.google.api.generator.engine.ast.ThrowExpr;
4141
import com.google.api.generator.engine.ast.TypeNode;
42+
import com.google.api.generator.engine.ast.UnaryOperationExpr;
4243
import com.google.api.generator.engine.ast.ValueExpr;
4344
import com.google.api.generator.engine.ast.VaporReference;
4445
import com.google.api.generator.engine.ast.Variable;
@@ -833,9 +834,8 @@ private static MethodDefinition createToStringListMethod(TypeNode thisClassType)
833834
Expr isNullCheck =
834835
MethodInvocationExpr.builder()
835836
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
836-
.setMethodName("equals")
837-
.setArguments(
838-
Arrays.asList(valueVarExpr, ValueExpr.withValue(NullObjectValue.create())))
837+
.setMethodName("isNull")
838+
.setArguments(valueVarExpr)
839839
.setReturnType(TypeNode.BOOLEAN)
840840
.build();
841841
Statement listAddEmptyStringStatement =
@@ -966,7 +966,6 @@ private static MethodDefinition createGetFieldValuesMapMethod(
966966

967967
// Innermost if-blocks.
968968
List<Statement> tokenIfStatements = new ArrayList<>();
969-
ValueExpr nullValExpr = ValueExpr.withValue(NullObjectValue.create());
970969
for (String token : getTokenSet(tokenHierarchies)) {
971970
VariableExpr tokenVarExpr = patternTokenVarExprs.get(token);
972971
Preconditions.checkNotNull(
@@ -979,14 +978,14 @@ private static MethodDefinition createGetFieldValuesMapMethod(
979978
.setMethodName("put")
980979
.setArguments(ValueExpr.withValue(tokenStrVal), tokenVarExpr)
981980
.build();
982-
// TODO(miraleung): Use neq operator here.
983-
MethodInvocationExpr notNullCheckExpr =
984-
MethodInvocationExpr.builder()
985-
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
986-
.setMethodName("notTodoEquals")
987-
.setArguments(tokenVarExpr, nullValExpr)
988-
.setReturnType(TypeNode.BOOLEAN)
989-
.build();
981+
Expr notNullCheckExpr =
982+
UnaryOperationExpr.logicalNotWithExpr(
983+
MethodInvocationExpr.builder()
984+
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
985+
.setMethodName("isNull")
986+
.setArguments(tokenVarExpr)
987+
.setReturnType(TypeNode.BOOLEAN)
988+
.build());
990989
tokenIfStatements.add(
991990
IfStatement.builder()
992991
.setConditionExpr(notNullCheckExpr)
@@ -1017,8 +1016,8 @@ private static MethodDefinition createGetFieldValuesMapMethod(
10171016
MethodInvocationExpr fieldValuesMapNullCheckExpr =
10181017
MethodInvocationExpr.builder()
10191018
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
1020-
.setMethodName("equals")
1021-
.setArguments(fieldValuesMapVarExpr, nullValExpr)
1019+
.setMethodName("isNull")
1020+
.setArguments(fieldValuesMapVarExpr)
10221021
.setReturnType(TypeNode.BOOLEAN)
10231022
.build();
10241023
IfStatement fieldValuesMapIfStatement =
@@ -1101,15 +1100,15 @@ private static MethodDefinition createToStringMethod(
11011100
}
11021101

11031102
VariableExpr fixedValueVarExpr = FIXED_CLASS_VARS.get("fixedValue");
1104-
// TODO(miraleung): Use neq operator, then swap the ternary exprs and do the following:
11051103
// Code: return fixedValue != null ? fixedValue : pathTemplate.instantiate(getFieldValuesMap())
1106-
MethodInvocationExpr fixedValueNullCheck =
1107-
MethodInvocationExpr.builder()
1108-
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
1109-
.setMethodName("equals")
1110-
.setArguments(fixedValueVarExpr, ValueExpr.withValue(NullObjectValue.create()))
1111-
.setReturnType(TypeNode.BOOLEAN)
1112-
.build();
1104+
Expr fixedValueNullCheck =
1105+
UnaryOperationExpr.logicalNotWithExpr(
1106+
MethodInvocationExpr.builder()
1107+
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
1108+
.setMethodName("isNull")
1109+
.setArguments(fixedValueVarExpr)
1110+
.setReturnType(TypeNode.BOOLEAN)
1111+
.build());
11131112

11141113
MethodInvocationExpr instantiateExpr =
11151114
MethodInvocationExpr.builder()
@@ -1122,9 +1121,8 @@ private static MethodDefinition createToStringMethod(
11221121
TernaryExpr returnExpr =
11231122
TernaryExpr.builder()
11241123
.setConditionExpr(fixedValueNullCheck)
1125-
// TODO(miraleung): Swap these when using the neq operator.
1126-
.setThenExpr(instantiateExpr)
1127-
.setElseExpr(fixedValueVarExpr)
1124+
.setElseExpr(instantiateExpr)
1125+
.setThenExpr(fixedValueVarExpr)
11281126
.build();
11291127

11301128
return MethodDefinition.builder()

‎src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -524,8 +524,8 @@ private static Expr createPagedListDescriptorAssignExpr(
524524
MethodInvocationExpr.builder()
525525
.setStaticReferenceType(
526526
TypeNode.withReference(ConcreteReference.withClazz(Objects.class)))
527-
.setMethodName("equals")
528-
.setArguments(getResponsesListExpr, ValueExpr.withValue(NullObjectValue.create()))
527+
.setMethodName("isNull")
528+
.setArguments(getResponsesListExpr)
529529
.setReturnType(TypeNode.BOOLEAN)
530530
.build();
531531
Expr thenExpr =

‎src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,7 @@ public void generateResourceNameClass_testingSessionOnePattern() {
384384
+ " public static List<String> toStringList(List<FoobarName> values) {\n"
385385
+ " List<String> list = new ArrayList<>(values.size());\n"
386386
+ " for (FoobarName value : values) {\n"
387-
+ " if (Objects.equals(value, null)) {\n"
387+
+ " if (Objects.isNull(value)) {\n"
388388
+ " list.add(\"\");\n"
389389
+ " } else {\n"
390390
+ " list.add(value.toString());\n"
@@ -402,18 +402,18 @@ public void generateResourceNameClass_testingSessionOnePattern() {
402402
+ "\n"
403403
+ " @Override\n"
404404
+ " public Map<String, String> getFieldValuesMap() {\n"
405-
+ " if (Objects.equals(fieldValuesMap, null)) {\n"
405+
+ " if (Objects.isNull(fieldValuesMap)) {\n"
406406
+ " synchronized (this) {\n"
407-
+ " if (Objects.equals(fieldValuesMap, null)) {\n"
407+
+ " if (Objects.isNull(fieldValuesMap)) {\n"
408408
+ " ImmutableMap.Builder<String, String> fieldMapBuilder ="
409409
+ " ImmutableMap.builder();\n"
410-
+ " if (Objects.notTodoEquals(project, null)) {\n"
410+
+ " if (!Objects.isNull(project)) {\n"
411411
+ " fieldMapBuilder.put(\"project\", project);\n"
412412
+ " }\n"
413-
+ " if (Objects.notTodoEquals(foobar, null)) {\n"
413+
+ " if (!Objects.isNull(foobar)) {\n"
414414
+ " fieldMapBuilder.put(\"foobar\", foobar);\n"
415415
+ " }\n"
416-
+ " if (Objects.notTodoEquals(variant, null)) {\n"
416+
+ " if (!Objects.isNull(variant)) {\n"
417417
+ " fieldMapBuilder.put(\"variant\", variant);\n"
418418
+ " }\n"
419419
+ " fieldValuesMap = fieldMapBuilder.build();\n"
@@ -429,9 +429,8 @@ public void generateResourceNameClass_testingSessionOnePattern() {
429429
+ "\n"
430430
+ " @Override\n"
431431
+ " public String toString() {\n"
432-
+ " return Objects.equals(fixedValue, null)\n"
433-
+ " ? pathTemplate.instantiate(getFieldValuesMap())\n"
434-
+ " : fixedValue;\n"
432+
+ " return !Objects.isNull(fixedValue) ? fixedValue :"
433+
+ " pathTemplate.instantiate(getFieldValuesMap());\n"
435434
+ " }\n"
436435
+ "\n"
437436
+ " /** Builder for projects/{project}/foobars/{foobar}. */\n"
@@ -606,7 +605,7 @@ public void generateResourceNameClass_testingSessionOnePattern() {
606605
+ " public static List<String> toStringList(List<SessionName> values) {\n"
607606
+ " List<String> list = new ArrayList<>(values.size());\n"
608607
+ " for (SessionName value : values) {\n"
609-
+ " if (Objects.equals(value, null)) {\n"
608+
+ " if (Objects.isNull(value)) {\n"
610609
+ " list.add(\"\");\n"
611610
+ " } else {\n"
612611
+ " list.add(value.toString());\n"
@@ -621,12 +620,12 @@ public void generateResourceNameClass_testingSessionOnePattern() {
621620
+ "\n"
622621
+ " @Override\n"
623622
+ " public Map<String, String> getFieldValuesMap() {\n"
624-
+ " if (Objects.equals(fieldValuesMap, null)) {\n"
623+
+ " if (Objects.isNull(fieldValuesMap)) {\n"
625624
+ " synchronized (this) {\n"
626-
+ " if (Objects.equals(fieldValuesMap, null)) {\n"
625+
+ " if (Objects.isNull(fieldValuesMap)) {\n"
627626
+ " ImmutableMap.Builder<String, String> fieldMapBuilder ="
628627
+ " ImmutableMap.builder();\n"
629-
+ " if (Objects.notTodoEquals(session, null)) {\n"
628+
+ " if (!Objects.isNull(session)) {\n"
630629
+ " fieldMapBuilder.put(\"session\", session);\n"
631630
+ " }\n"
632631
+ " fieldValuesMap = fieldMapBuilder.build();\n"

‎src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -307,7 +307,7 @@ private static List<Service> parseServices(
307307
+ " @Override\n"
308308
+ " public Iterable<EchoResponse> extractResources(PagedExpandResponse"
309309
+ " payload) {\n"
310-
+ " return Objects.equals(payload.getResponsesList(), null)\n"
310+
+ " return Objects.isNull(payload.getResponsesList())\n"
311311
+ " ? ImmutableList.<EchoResponse>of()\n"
312312
+ " : payload.getResponsesList();\n"
313313
+ " }\n"
@@ -877,7 +877,7 @@ private static List<Service> parseServices(
877877
+ " @Override\n"
878878
+ " public Iterable<LogEntry> extractResources(ListLogEntriesResponse"
879879
+ " payload) {\n"
880-
+ " return Objects.equals(payload.getResponsesList(), null)\n"
880+
+ " return Objects.isNull(payload.getResponsesList())\n"
881881
+ " ? ImmutableList.<LogEntry>of()\n"
882882
+ " : payload.getResponsesList();\n"
883883
+ " }\n"
@@ -927,7 +927,7 @@ private static List<Service> parseServices(
927927
+ " @Override\n"
928928
+ " public Iterable<MonitoredResourceDescriptor> extractResources(\n"
929929
+ " ListMonitoredResourceDescriptorsResponse payload) {\n"
930-
+ " return Objects.equals(payload.getResponsesList(), null)\n"
930+
+ " return Objects.isNull(payload.getResponsesList())\n"
931931
+ " ? ImmutableList.<MonitoredResourceDescriptor>of()\n"
932932
+ " : payload.getResponsesList();\n"
933933
+ " }\n"
@@ -967,7 +967,7 @@ private static List<Service> parseServices(
967967
+ "\n"
968968
+ " @Override\n"
969969
+ " public Iterable<String> extractResources(ListLogsResponse payload) {\n"
970-
+ " return Objects.equals(payload.getResponsesList(), null)\n"
970+
+ " return Objects.isNull(payload.getResponsesList())\n"
971971
+ " ? ImmutableList.<String>of()\n"
972972
+ " : payload.getResponsesList();\n"
973973
+ " }\n"
@@ -1616,7 +1616,7 @@ private static List<Service> parseServices(
16161616
+ "\n"
16171617
+ " @Override\n"
16181618
+ " public Iterable<Topic> extractResources(ListTopicsResponse payload) {\n"
1619-
+ " return Objects.equals(payload.getResponsesList(), null)\n"
1619+
+ " return Objects.isNull(payload.getResponsesList())\n"
16201620
+ " ? ImmutableList.<Topic>of()\n"
16211621
+ " : payload.getResponsesList();\n"
16221622
+ " }\n"
@@ -1661,7 +1661,7 @@ private static List<Service> parseServices(
16611661
+ " @Override\n"
16621662
+ " public Iterable<String> extractResources(ListTopicSubscriptionsResponse"
16631663
+ " payload) {\n"
1664-
+ " return Objects.equals(payload.getResponsesList(), null)\n"
1664+
+ " return Objects.isNull(payload.getResponsesList())\n"
16651665
+ " ? ImmutableList.<String>of()\n"
16661666
+ " : payload.getResponsesList();\n"
16671667
+ " }\n"
@@ -1703,7 +1703,7 @@ private static List<Service> parseServices(
17031703
+ " @Override\n"
17041704
+ " public Iterable<String> extractResources(ListTopicSnapshotsResponse"
17051705
+ " payload) {\n"
1706-
+ " return Objects.equals(payload.getResponsesList(), null)\n"
1706+
+ " return Objects.isNull(payload.getResponsesList())\n"
17071707
+ " ? ImmutableList.<String>of()\n"
17081708
+ " : payload.getResponsesList();\n"
17091709
+ " }\n"

0 commit comments

Comments
 (0)