From 7bfe9d57e83bf4317201adf89d1fc6d575b541d9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 19:23:31 -0800 Subject: [PATCH] [ggj][ast][engx] fix: add node validator to refactor/centralize null element checks (#455) * fix: swap assertEquals args in JavaWriterVisitorTest to match (expected, actusl) order * fix: swap assertEquals args in ImportWriterVisitorTest to match (expected, actusl) order * fix: add node validator to refactor/centralize null element checks --- .../engine/ast/MethodInvocationExpr.java | 19 +++++------- .../generator/engine/ast/NewObjectExpr.java | 7 ++--- .../generator/engine/ast/NodeValidator.java | 29 +++++++++++++++++++ .../engine/ast/ReferenceConstructorExpr.java | 8 ++--- 4 files changed, 41 insertions(+), 22 deletions(-) create mode 100644 src/main/java/com/google/api/generator/engine/ast/NodeValidator.java diff --git a/src/main/java/com/google/api/generator/engine/ast/MethodInvocationExpr.java b/src/main/java/com/google/api/generator/engine/ast/MethodInvocationExpr.java index 96cc2d738b..c7970e2990 100644 --- a/src/main/java/com/google/api/generator/engine/ast/MethodInvocationExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/MethodInvocationExpr.java @@ -19,7 +19,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; import javax.annotation.Nullable; @AutoValue @@ -117,17 +116,15 @@ public MethodInvocationExpr build() { || methodInvocationExpr.staticReferenceType() == null, "Only the expression reference or the static reference can be set, not both"); - Preconditions.checkState( - methodInvocationExpr.arguments().stream().allMatch(e -> !Objects.isNull(e)), - String.format( - "Found null expression in arguments %s for %s", - methodInvocationExpr.arguments(), methodInvocationExpr.methodIdentifier().name())); + NodeValidator.checkNoNullElements( + methodInvocationExpr.arguments(), + "arguments", + String.format("method invocation of %s", methodInvocationExpr.methodIdentifier().name())); - Preconditions.checkState( - methodInvocationExpr.generics().stream().allMatch(e -> !Objects.isNull(e)), - String.format( - "Found null expression in generics %s for %s", - methodInvocationExpr.generics(), methodInvocationExpr.methodIdentifier().name())); + NodeValidator.checkNoNullElements( + methodInvocationExpr.generics(), + "generics", + String.format("method invocation of %s", methodInvocationExpr.methodIdentifier().name())); return methodInvocationExpr; } diff --git a/src/main/java/com/google/api/generator/engine/ast/NewObjectExpr.java b/src/main/java/com/google/api/generator/engine/ast/NewObjectExpr.java index 40e5275f92..50fbbed983 100644 --- a/src/main/java/com/google/api/generator/engine/ast/NewObjectExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/NewObjectExpr.java @@ -20,7 +20,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; @AutoValue public abstract class NewObjectExpr implements Expr { @@ -70,10 +69,8 @@ public NewObjectExpr build() { Preconditions.checkState( TypeNode.isReferenceType(type()), "New object expression should be reference types."); - Preconditions.checkState( - arguments().stream().allMatch(e -> !Objects.isNull(e)), - String.format( - "Found null argument for the \"new\" constructor of %s", type().reference().name())); + NodeValidator.checkNoNullElements( + arguments(), "the \"new\" constructor", type().reference().name()); // Only the case where generics() is empty and isGeneric() is false, we set isGeneric() to // false. Otherwise, isGeneric() should be true. diff --git a/src/main/java/com/google/api/generator/engine/ast/NodeValidator.java b/src/main/java/com/google/api/generator/engine/ast/NodeValidator.java new file mode 100644 index 0000000000..5eba84e936 --- /dev/null +++ b/src/main/java/com/google/api/generator/engine/ast/NodeValidator.java @@ -0,0 +1,29 @@ +// Copyright 2020 Google LLC +// +// 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 com.google.api.generator.engine.ast; + +import com.google.common.base.Preconditions; +import java.util.Collection; +import java.util.Objects; + +public class NodeValidator { + public static void checkNoNullElements( + Collection collection, String fieldTypeName, String nodeContextInfo) { + Preconditions.checkState( + collection.stream().allMatch(e -> !Objects.isNull(e)), + String.format( + "Found null expression in %s %s for %s", fieldTypeName, collection, nodeContextInfo)); + } +} diff --git a/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java b/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java index 405712c905..25940b0d3d 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/ReferenceConstructorExpr.java @@ -20,7 +20,6 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; -import java.util.Objects; @AutoValue public abstract class ReferenceConstructorExpr implements Expr { @@ -77,11 +76,8 @@ public ReferenceConstructorExpr build() { referenceConstructorExpr.type().isReferenceType(referenceConstructorExpr.type()), "A this/super constructor must have a reference type."); - Preconditions.checkState( - arguments().stream().allMatch(e -> !Objects.isNull(e)), - String.format( - "Found null argumentfor in the \"this/super\" initialization of %s", - type().reference().name())); + NodeValidator.checkNoNullElements( + arguments(), "the \"this\" or \"super\" initialization", type().reference().name()); return referenceConstructorExpr; }