From 15ffe6b9f9f08864b923b688e19a7001578a83fc Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Thu, 10 Sep 2020 16:55:04 -0700 Subject: [PATCH] fix: use Objects.isNull instead of null equality --- .../ResourceNameHelperClassComposer.java | 48 +++++++++---------- .../ServiceStubSettingsClassComposer.java | 4 +- .../ResourceNameHelperClassComposerTest.java | 25 +++++----- .../ServiceStubSettingsClassComposerTest.java | 14 +++--- 4 files changed, 44 insertions(+), 47 deletions(-) diff --git a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java index 249a4c4252..c191c86359 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposer.java @@ -39,6 +39,7 @@ 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; @@ -833,9 +834,8 @@ private static MethodDefinition createToStringListMethod(TypeNode thisClassType) Expr isNullCheck = MethodInvocationExpr.builder() .setStaticReferenceType(STATIC_TYPES.get("Objects")) - .setMethodName("equals") - .setArguments( - Arrays.asList(valueVarExpr, ValueExpr.withValue(NullObjectValue.create()))) + .setMethodName("isNull") + .setArguments(valueVarExpr) .setReturnType(TypeNode.BOOLEAN) .build(); Statement listAddEmptyStringStatement = @@ -966,7 +966,6 @@ private static MethodDefinition createGetFieldValuesMapMethod( // Innermost if-blocks. List tokenIfStatements = new ArrayList<>(); - ValueExpr nullValExpr = ValueExpr.withValue(NullObjectValue.create()); for (String token : getTokenSet(tokenHierarchies)) { VariableExpr tokenVarExpr = patternTokenVarExprs.get(token); Preconditions.checkNotNull( @@ -979,14 +978,14 @@ private static MethodDefinition createGetFieldValuesMapMethod( .setMethodName("put") .setArguments(ValueExpr.withValue(tokenStrVal), tokenVarExpr) .build(); - // TODO(miraleung): Use neq operator here. - MethodInvocationExpr notNullCheckExpr = - MethodInvocationExpr.builder() - .setStaticReferenceType(STATIC_TYPES.get("Objects")) - .setMethodName("notTodoEquals") - .setArguments(tokenVarExpr, nullValExpr) - .setReturnType(TypeNode.BOOLEAN) - .build(); + Expr notNullCheckExpr = + UnaryOperationExpr.logicalNotWithExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Objects")) + .setMethodName("isNull") + .setArguments(tokenVarExpr) + .setReturnType(TypeNode.BOOLEAN) + .build()); tokenIfStatements.add( IfStatement.builder() .setConditionExpr(notNullCheckExpr) @@ -1017,8 +1016,8 @@ private static MethodDefinition createGetFieldValuesMapMethod( MethodInvocationExpr fieldValuesMapNullCheckExpr = MethodInvocationExpr.builder() .setStaticReferenceType(STATIC_TYPES.get("Objects")) - .setMethodName("equals") - .setArguments(fieldValuesMapVarExpr, nullValExpr) + .setMethodName("isNull") + .setArguments(fieldValuesMapVarExpr) .setReturnType(TypeNode.BOOLEAN) .build(); IfStatement fieldValuesMapIfStatement = @@ -1101,15 +1100,15 @@ private static MethodDefinition createToStringMethod( } VariableExpr fixedValueVarExpr = FIXED_CLASS_VARS.get("fixedValue"); - // TODO(miraleung): Use neq operator, then swap the ternary exprs and do the following: // Code: return fixedValue != null ? fixedValue : pathTemplate.instantiate(getFieldValuesMap()) - MethodInvocationExpr fixedValueNullCheck = - MethodInvocationExpr.builder() - .setStaticReferenceType(STATIC_TYPES.get("Objects")) - .setMethodName("equals") - .setArguments(fixedValueVarExpr, ValueExpr.withValue(NullObjectValue.create())) - .setReturnType(TypeNode.BOOLEAN) - .build(); + Expr fixedValueNullCheck = + UnaryOperationExpr.logicalNotWithExpr( + MethodInvocationExpr.builder() + .setStaticReferenceType(STATIC_TYPES.get("Objects")) + .setMethodName("isNull") + .setArguments(fixedValueVarExpr) + .setReturnType(TypeNode.BOOLEAN) + .build()); MethodInvocationExpr instantiateExpr = MethodInvocationExpr.builder() @@ -1122,9 +1121,8 @@ private static MethodDefinition createToStringMethod( TernaryExpr returnExpr = TernaryExpr.builder() .setConditionExpr(fixedValueNullCheck) - // TODO(miraleung): Swap these when using the neq operator. - .setThenExpr(instantiateExpr) - .setElseExpr(fixedValueVarExpr) + .setElseExpr(instantiateExpr) + .setThenExpr(fixedValueVarExpr) .build(); return MethodDefinition.builder() diff --git a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java index 9e80c622c9..c3d12a803f 100644 --- a/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java +++ b/src/main/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposer.java @@ -524,8 +524,8 @@ private static Expr createPagedListDescriptorAssignExpr( MethodInvocationExpr.builder() .setStaticReferenceType( TypeNode.withReference(ConcreteReference.withClazz(Objects.class))) - .setMethodName("equals") - .setArguments(getResponsesListExpr, ValueExpr.withValue(NullObjectValue.create())) + .setMethodName("isNull") + .setArguments(getResponsesListExpr) .setReturnType(TypeNode.BOOLEAN) .build(); Expr thenExpr = diff --git a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java index 836c2cbcf8..51143833c1 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ResourceNameHelperClassComposerTest.java @@ -384,7 +384,7 @@ public void generateResourceNameClass_testingSessionOnePattern() { + " public static List toStringList(List values) {\n" + " List list = new ArrayList<>(values.size());\n" + " for (FoobarName value : values) {\n" - + " if (Objects.equals(value, null)) {\n" + + " if (Objects.isNull(value)) {\n" + " list.add(\"\");\n" + " } else {\n" + " list.add(value.toString());\n" @@ -402,18 +402,18 @@ public void generateResourceNameClass_testingSessionOnePattern() { + "\n" + " @Override\n" + " public Map getFieldValuesMap() {\n" - + " if (Objects.equals(fieldValuesMap, null)) {\n" + + " if (Objects.isNull(fieldValuesMap)) {\n" + " synchronized (this) {\n" - + " if (Objects.equals(fieldValuesMap, null)) {\n" + + " if (Objects.isNull(fieldValuesMap)) {\n" + " ImmutableMap.Builder fieldMapBuilder =" + " ImmutableMap.builder();\n" - + " if (Objects.notTodoEquals(project, null)) {\n" + + " if (!Objects.isNull(project)) {\n" + " fieldMapBuilder.put(\"project\", project);\n" + " }\n" - + " if (Objects.notTodoEquals(foobar, null)) {\n" + + " if (!Objects.isNull(foobar)) {\n" + " fieldMapBuilder.put(\"foobar\", foobar);\n" + " }\n" - + " if (Objects.notTodoEquals(variant, null)) {\n" + + " if (!Objects.isNull(variant)) {\n" + " fieldMapBuilder.put(\"variant\", variant);\n" + " }\n" + " fieldValuesMap = fieldMapBuilder.build();\n" @@ -429,9 +429,8 @@ public void generateResourceNameClass_testingSessionOnePattern() { + "\n" + " @Override\n" + " public String toString() {\n" - + " return Objects.equals(fixedValue, null)\n" - + " ? pathTemplate.instantiate(getFieldValuesMap())\n" - + " : fixedValue;\n" + + " return !Objects.isNull(fixedValue) ? fixedValue :" + + " pathTemplate.instantiate(getFieldValuesMap());\n" + " }\n" + "\n" + " /** Builder for projects/{project}/foobars/{foobar}. */\n" @@ -606,7 +605,7 @@ public void generateResourceNameClass_testingSessionOnePattern() { + " public static List toStringList(List values) {\n" + " List list = new ArrayList<>(values.size());\n" + " for (SessionName value : values) {\n" - + " if (Objects.equals(value, null)) {\n" + + " if (Objects.isNull(value)) {\n" + " list.add(\"\");\n" + " } else {\n" + " list.add(value.toString());\n" @@ -621,12 +620,12 @@ public void generateResourceNameClass_testingSessionOnePattern() { + "\n" + " @Override\n" + " public Map getFieldValuesMap() {\n" - + " if (Objects.equals(fieldValuesMap, null)) {\n" + + " if (Objects.isNull(fieldValuesMap)) {\n" + " synchronized (this) {\n" - + " if (Objects.equals(fieldValuesMap, null)) {\n" + + " if (Objects.isNull(fieldValuesMap)) {\n" + " ImmutableMap.Builder fieldMapBuilder =" + " ImmutableMap.builder();\n" - + " if (Objects.notTodoEquals(session, null)) {\n" + + " if (!Objects.isNull(session)) {\n" + " fieldMapBuilder.put(\"session\", session);\n" + " }\n" + " fieldValuesMap = fieldMapBuilder.build();\n" diff --git a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java index 2003af7eb2..981bcd5868 100644 --- a/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java +++ b/src/test/java/com/google/api/generator/gapic/composer/ServiceStubSettingsClassComposerTest.java @@ -307,7 +307,7 @@ private static List parseServices( + " @Override\n" + " public Iterable extractResources(PagedExpandResponse" + " payload) {\n" - + " return Objects.equals(payload.getResponsesList(), null)\n" + + " return Objects.isNull(payload.getResponsesList())\n" + " ? ImmutableList.of()\n" + " : payload.getResponsesList();\n" + " }\n" @@ -877,7 +877,7 @@ private static List parseServices( + " @Override\n" + " public Iterable extractResources(ListLogEntriesResponse" + " payload) {\n" - + " return Objects.equals(payload.getResponsesList(), null)\n" + + " return Objects.isNull(payload.getResponsesList())\n" + " ? ImmutableList.of()\n" + " : payload.getResponsesList();\n" + " }\n" @@ -927,7 +927,7 @@ private static List parseServices( + " @Override\n" + " public Iterable extractResources(\n" + " ListMonitoredResourceDescriptorsResponse payload) {\n" - + " return Objects.equals(payload.getResponsesList(), null)\n" + + " return Objects.isNull(payload.getResponsesList())\n" + " ? ImmutableList.of()\n" + " : payload.getResponsesList();\n" + " }\n" @@ -967,7 +967,7 @@ private static List parseServices( + "\n" + " @Override\n" + " public Iterable extractResources(ListLogsResponse payload) {\n" - + " return Objects.equals(payload.getResponsesList(), null)\n" + + " return Objects.isNull(payload.getResponsesList())\n" + " ? ImmutableList.of()\n" + " : payload.getResponsesList();\n" + " }\n" @@ -1616,7 +1616,7 @@ private static List parseServices( + "\n" + " @Override\n" + " public Iterable extractResources(ListTopicsResponse payload) {\n" - + " return Objects.equals(payload.getResponsesList(), null)\n" + + " return Objects.isNull(payload.getResponsesList())\n" + " ? ImmutableList.of()\n" + " : payload.getResponsesList();\n" + " }\n" @@ -1661,7 +1661,7 @@ private static List parseServices( + " @Override\n" + " public Iterable extractResources(ListTopicSubscriptionsResponse" + " payload) {\n" - + " return Objects.equals(payload.getResponsesList(), null)\n" + + " return Objects.isNull(payload.getResponsesList())\n" + " ? ImmutableList.of()\n" + " : payload.getResponsesList();\n" + " }\n" @@ -1703,7 +1703,7 @@ private static List parseServices( + " @Override\n" + " public Iterable extractResources(ListTopicSnapshotsResponse" + " payload) {\n" - + " return Objects.equals(payload.getResponsesList(), null)\n" + + " return Objects.isNull(payload.getResponsesList())\n" + " ? ImmutableList.of()\n" + " : payload.getResponsesList();\n" + " }\n"