From df731a5e7025e71ff8e3c280fb09a241f2bb5bcb Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Wed, 6 Jul 2022 19:00:17 +0000 Subject: [PATCH 01/10] change to AnnotationNode and JavaWriterVisitor to support multiple parameters with name and value in annotation. --- .../generator/engine/ast/AnnotationNode.java | 24 ++++++++++++------- .../engine/writer/JavaWriterVisitor.java | 12 +++++++--- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java b/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java index 1a3412aa6c..6bb8a372a9 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java +++ b/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java @@ -1,4 +1,4 @@ -// Copyright 2020 Google LLC +// Copyright 2022 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -16,6 +16,8 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; +import java.util.Arrays; +import java.util.List; import javax.annotation.Nullable; @AutoValue @@ -31,10 +33,8 @@ private static TypeNode annotationType(Class clazz) { public abstract TypeNode type(); - // TODO(unsupported): Any args that do not consist of a single string. However, this can easily be - // extended to enable such support. @Nullable - public abstract Expr descriptionExpr(); + public abstract List descriptionExprs(); @Override public void accept(AstNodeVisitor visitor) { @@ -45,6 +45,10 @@ public static AnnotationNode withTypeAndDescription(TypeNode type, String descri return AnnotationNode.builder().setType(type).setDescription(description).build(); } + public static AnnotationNode withTypeAndDescription(TypeNode type, List exprList) { + return AnnotationNode.builder().setType(type).setDescriptions(exprList).build(); + } + public static AnnotationNode withSuppressWarnings(String description) { return withTypeAndDescription(annotationType(SuppressWarnings.class), description); } @@ -62,12 +66,16 @@ public abstract static class Builder { public abstract Builder setType(TypeNode type); public Builder setDescription(String description) { - return setDescriptionExpr(ValueExpr.withValue(StringObjectValue.withValue(description))); + return setDescriptionExprs( + Arrays.asList(ValueExpr.withValue(StringObjectValue.withValue(description)))); + } + + public Builder setDescriptions(List exprList) { + return setDescriptionExprs(exprList); } - // This will never be anything other than a ValueExpr-wrapped StringObjectValue because - // this setter is private, and called only by setDescription above. - abstract Builder setDescriptionExpr(Expr descriptionExpr); + // this setter is private, and called only by setDescription() and setDescriptions() above. + abstract Builder setDescriptionExprs(List descriptionExprs); abstract AnnotationNode autoBuild(); diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 0a867b7cdb..5a05267c52 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -1,4 +1,4 @@ -// Copyright 2020 Google LLC +// Copyright 2022 Google LLC // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -169,9 +169,15 @@ public void visit(ScopeNode scope) { public void visit(AnnotationNode annotation) { buffer.append(AT); annotation.type().accept(this); - if (annotation.descriptionExpr() != null) { + if (annotation.descriptionExprs() != null) { leftParen(); - annotation.descriptionExpr().accept(this); + for (int i = 0; i < annotation.descriptionExprs().size(); i++) { + annotation.descriptionExprs().get(i).accept(this); + if (i < annotation.descriptionExprs().size() - 1) { + buffer.append(COMMA); + buffer.append(SPACE); + } + } rightParen(); } newline(); From bc36ce50b37ef0d14ab2c58c5a742b2d4e844a41 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Wed, 6 Jul 2022 19:03:59 +0000 Subject: [PATCH 02/10] add Value implementation class for Class type var. --- .../api/generator/engine/ast/ClazzValue.java | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 src/main/java/com/google/api/generator/engine/ast/ClazzValue.java diff --git a/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java b/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java new file mode 100644 index 0000000000..49d3d91c3c --- /dev/null +++ b/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java @@ -0,0 +1,47 @@ +// Copyright 2022 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.auto.value.AutoValue; + +@AutoValue +public abstract class ClazzValue implements Value { + @Override + public abstract TypeNode type(); + + @Override + public abstract String value(); + + public static Builder builder() { + return new AutoValue_ClazzValue.Builder(); + } + + @AutoValue.Builder + public abstract static class Builder { + public abstract Builder setType(TypeNode type); + + abstract Builder setValue(String value); + + public abstract TypeNode type(); + + abstract ClazzValue autoBuild(); + + public ClazzValue build() { + setValue(type().reference().name() + ".class"); + ClazzValue clazzValue = autoBuild(); + return clazzValue; + } + } +} From 4e1379c4f62adc33875970b9dc85afa4092a7214 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Wed, 6 Jul 2022 19:05:05 +0000 Subject: [PATCH 03/10] adding tests. --- .../engine/writer/JavaWriterVisitorTest.java | 57 ++++++++++++++++++- 1 file changed, 56 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index 39e448a263..363542c360 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -27,6 +27,7 @@ import com.google.api.generator.engine.ast.BreakStatement; import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; +import com.google.api.generator.engine.ast.ClazzValue; import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.EmptyLineStatement; @@ -152,12 +153,50 @@ public void writeAnnotation_simple() { } @Test - public void writeAnnotation_withDescription() { + public void writeAnnotation_withStringDescription() { AnnotationNode annotation = AnnotationNode.withSuppressWarnings("all"); annotation.accept(writerVisitor); assertEquals("@SuppressWarnings(\"all\")\n", writerVisitor.write()); } + @Test + public void writeAnnotation_withNamedDescriptions() { + TypeNode conditionalOnPropertyType = + TypeNode.withReference( + VaporReference.builder() + .setName("ConditionalOnProperty") + .setPakkage("org.springframework.boot.autoconfigure.condition") + .build()); + + AssignmentExpr matchIfMissing = + AssignmentExpr.builder() + .setVariableExpr( + VariableExpr.withVariable( + Variable.builder().setName("matchIfMissing").setType(TypeNode.BOOLEAN).build())) + .setValueExpr( + ValueExpr.withValue( + PrimitiveValue.builder().setValue("false").setType(TypeNode.BOOLEAN).build())) + .build(); + AssignmentExpr valueString = + AssignmentExpr.builder() + .setVariableExpr( + VariableExpr.withVariable( + Variable.builder().setName("value").setType(TypeNode.STRING).build())) + .setValueExpr( + ValueExpr.withValue( + StringObjectValue.withValue("spring.cloud.gcp.language.enabled"))) + .build(); + AnnotationNode annotation = + AnnotationNode.builder() + .setType(conditionalOnPropertyType) + .setDescriptions(Arrays.asList(valueString, matchIfMissing)) + .build(); + annotation.accept(writerVisitor); + assertEquals( + "@ConditionalOnProperty(value = \"spring.cloud.gcp.language.enabled\", matchIfMissing = false)\n", + writerVisitor.write()); + } + @Test public void writeNewObjectExpr_basic() { // isGeneric() is true, but generics() is empty. @@ -737,6 +776,22 @@ public void writeAssignmentExpr_stringObjectValue() { assertThat(writerVisitor.write()).isEqualTo("String x = \"Hi! World. \\n\""); } + @Test + public void writeAssignmentExpr_clazzValue() { + TypeNode clazz = TypeNode.withReference(ConcreteReference.withClazz(List.class)); + + VariableExpr variableExpr = + VariableExpr.withVariable(Variable.builder().setName("value").setType(clazz).build()); + + ValueExpr valueExpr = ValueExpr.withValue(ClazzValue.builder().setType(clazz).build()); + + AssignmentExpr assignExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); + + assignExpr.accept(writerVisitor); + assertEquals("value = List.class", writerVisitor.write()); + } + @Test public void writeMethodInvocationExpr_basic() { MethodInvocationExpr methodExpr = From 55634f65fbf9340730cd541c157eb1ed243087b8 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Wed, 6 Jul 2022 20:07:31 +0000 Subject: [PATCH 04/10] add test. --- .../generator/engine/ast/ClazzValueTest.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java diff --git a/src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java b/src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java new file mode 100644 index 0000000000..31e02461e6 --- /dev/null +++ b/src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java @@ -0,0 +1,44 @@ +// Copyright 2022 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 static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertEquals; + +import java.util.List; +import org.junit.Test; + +public class ClazzValueTest { + + @Test + public void createClazzValue_basic() { + TypeNode clazz = TypeNode.withReference(ConcreteReference.withClazz(List.class)); + + assertValidValue(ConcreteReference.withClazz(List.class), "List.class"); + assertValidValue( + VaporReference.builder() + .setName("ConditionalOnProperty") + .setPakkage("org.springframework.boot.autoconfigure.condition") + .build(), + "ConditionalOnProperty.class"); + } + + private static void assertValidValue(Reference reference, String value) { + TypeNode type = TypeNode.withReference(reference); + ClazzValue clazzValue = ClazzValue.builder().setType(type).build(); + assertEquals(value, clazzValue.value()); + assertThat(clazzValue.type()).isEqualTo(type); + } +} From 9f1b09c2375a83dcc993b8f8ea9e95a5e14da48f Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Thu, 7 Jul 2022 14:38:35 +0000 Subject: [PATCH 05/10] clean up code smells. --- .../api/generator/engine/ast/ClazzValue.java | 30 +++++++++---------- 1 file changed, 14 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java b/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java index 49d3d91c3c..1a7dd645e3 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java +++ b/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java @@ -1,17 +1,16 @@ -// Copyright 2022 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. - +/** + * Copyright 2022 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.auto.value.AutoValue; @@ -40,8 +39,7 @@ public abstract static class Builder { public ClazzValue build() { setValue(type().reference().name() + ".class"); - ClazzValue clazzValue = autoBuild(); - return clazzValue; + return autoBuild(); } } } From b9c6ef540fe7541cc71d5a25ffebbfa6533aff8a Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Fri, 8 Jul 2022 15:17:01 +0000 Subject: [PATCH 06/10] ClazzValue implements ObjectValue instead of Value. --- .../java/com/google/api/generator/engine/ast/ClazzValue.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java b/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java index 1a7dd645e3..147f1765a9 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java +++ b/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java @@ -16,7 +16,7 @@ import com.google.auto.value.AutoValue; @AutoValue -public abstract class ClazzValue implements Value { +public abstract class ClazzValue implements ObjectValue { @Override public abstract TypeNode type(); From 6a41bddef5fb3fda22afdbba164f2b0c5dde09c1 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Fri, 15 Jul 2022 14:14:48 +0000 Subject: [PATCH 07/10] restructure AnnotationNode methods for adding parameters and remove unnecessary ClazzValue class. --- .../generator/engine/ast/AnnotationNode.java | 64 +++++++- .../api/generator/engine/ast/ClazzValue.java | 45 ------ .../generator/engine/ast/ClazzValueTest.java | 44 ------ .../engine/writer/JavaWriterVisitorTest.java | 146 +++++++++++++++--- 4 files changed, 190 insertions(+), 109 deletions(-) delete mode 100644 src/main/java/com/google/api/generator/engine/ast/ClazzValue.java delete mode 100644 src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java diff --git a/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java b/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java index 6bb8a372a9..45464b333b 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java +++ b/src/main/java/com/google/api/generator/engine/ast/AnnotationNode.java @@ -16,6 +16,7 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; +import java.util.ArrayList; import java.util.Arrays; import java.util.List; import javax.annotation.Nullable; @@ -63,17 +64,78 @@ public static Builder builder() { @AutoValue.Builder public abstract static class Builder { + private static final String REPEAT_SINGLE_EXCEPTION_MESSAGE = + "Single parameter with no name cannot be set multiple times"; + + private static final String MULTIPLE_AFTER_SINGLE_EXCEPTION_MESSAGE = + "Multiple parameters must have names"; + + abstract List descriptionExprs(); + public abstract Builder setType(TypeNode type); + /** + * To set single String as description. + * + * @param description + * @return Builder + */ public Builder setDescription(String description) { + Preconditions.checkState(descriptionExprs() == null, REPEAT_SINGLE_EXCEPTION_MESSAGE); return setDescriptionExprs( Arrays.asList(ValueExpr.withValue(StringObjectValue.withValue(description)))); } - public Builder setDescriptions(List exprList) { + /** + * To set single ValueExpr as description. + * + * @param valueExpr + * @return Builder + */ + public Builder setDescription(ValueExpr valueExpr) { + Preconditions.checkState(descriptionExprs() == null, REPEAT_SINGLE_EXCEPTION_MESSAGE); + return setDescriptionExprs(Arrays.asList(valueExpr)); + } + + /** + * To set single VariableExpr as description. + * + * @param variableExpr + * @return Builder + */ + public Builder setDescription(VariableExpr variableExpr) { + Preconditions.checkState(descriptionExprs() == null, REPEAT_SINGLE_EXCEPTION_MESSAGE); + return setDescriptionExprs(Arrays.asList(variableExpr)); + } + + /** + * To add an AssignmentExpr as parameter. Can be used repeatedly to add multiple parameters. + * + * @param assignmentExpr + * @return Builder + */ + public Builder addDescription(AssignmentExpr assignmentExpr) { + return addDescriptionToList(assignmentExpr); + } + + private Builder setDescriptions(List exprList) { return setDescriptionExprs(exprList); } + // this method is private, and called only by addDescription(AssignmentExpr expr) + private Builder addDescriptionToList(Expr expr) { + List exprList = descriptionExprs(); + // avoid when single parameter is already set. + Preconditions.checkState( + exprList == null || exprList instanceof ArrayList, + MULTIPLE_AFTER_SINGLE_EXCEPTION_MESSAGE); + if (exprList == null) { + exprList = new ArrayList<>(); + } + exprList.add(expr); + return setDescriptions(exprList); + } + // this setter is private, and called only by setDescription() and setDescriptions() above. abstract Builder setDescriptionExprs(List descriptionExprs); diff --git a/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java b/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java deleted file mode 100644 index 147f1765a9..0000000000 --- a/src/main/java/com/google/api/generator/engine/ast/ClazzValue.java +++ /dev/null @@ -1,45 +0,0 @@ -/** - * Copyright 2022 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.auto.value.AutoValue; - -@AutoValue -public abstract class ClazzValue implements ObjectValue { - @Override - public abstract TypeNode type(); - - @Override - public abstract String value(); - - public static Builder builder() { - return new AutoValue_ClazzValue.Builder(); - } - - @AutoValue.Builder - public abstract static class Builder { - public abstract Builder setType(TypeNode type); - - abstract Builder setValue(String value); - - public abstract TypeNode type(); - - abstract ClazzValue autoBuild(); - - public ClazzValue build() { - setValue(type().reference().name() + ".class"); - return autoBuild(); - } - } -} diff --git a/src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java b/src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java deleted file mode 100644 index 31e02461e6..0000000000 --- a/src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java +++ /dev/null @@ -1,44 +0,0 @@ -// Copyright 2022 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 static com.google.common.truth.Truth.assertThat; -import static org.junit.Assert.assertEquals; - -import java.util.List; -import org.junit.Test; - -public class ClazzValueTest { - - @Test - public void createClazzValue_basic() { - TypeNode clazz = TypeNode.withReference(ConcreteReference.withClazz(List.class)); - - assertValidValue(ConcreteReference.withClazz(List.class), "List.class"); - assertValidValue( - VaporReference.builder() - .setName("ConditionalOnProperty") - .setPakkage("org.springframework.boot.autoconfigure.condition") - .build(), - "ConditionalOnProperty.class"); - } - - private static void assertValidValue(Reference reference, String value) { - TypeNode type = TypeNode.withReference(reference); - ClazzValue clazzValue = ClazzValue.builder().setType(type).build(); - assertEquals(value, clazzValue.value()); - assertThat(clazzValue.type()).isEqualTo(type); - } -} diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index 363542c360..935e0229d8 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -16,6 +16,7 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThrows; import com.google.api.generator.engine.ast.AnnotationNode; import com.google.api.generator.engine.ast.AnonymousClassExpr; @@ -27,7 +28,6 @@ import com.google.api.generator.engine.ast.BreakStatement; import com.google.api.generator.engine.ast.CastExpr; import com.google.api.generator.engine.ast.ClassDefinition; -import com.google.api.generator.engine.ast.ClazzValue; import com.google.api.generator.engine.ast.CommentStatement; import com.google.api.generator.engine.ast.ConcreteReference; import com.google.api.generator.engine.ast.EmptyLineStatement; @@ -160,7 +160,52 @@ public void writeAnnotation_withStringDescription() { } @Test - public void writeAnnotation_withNamedDescriptions() { + public void writeAnnotation_withValueDescription() { + TypeNode fakeAnnotationType = + TypeNode.withReference( + VaporReference.builder().setName("FakeAnnotation").setPakkage("com.foo.bar").build()); + AnnotationNode annotation = + AnnotationNode.builder() + .setType(fakeAnnotationType) + .setDescription( + ValueExpr.withValue( + PrimitiveValue.builder().setValue("12").setType(TypeNode.INT).build())) + .build(); + annotation.accept(writerVisitor); + assertEquals("@FakeAnnotation(12)\n", writerVisitor.write()); + } + + @Test + public void writeAnnotation_withVariableExprDescription() { + TypeNode conditionalOnPropertyType = + TypeNode.withReference( + VaporReference.builder() + .setName("ConditionalOnClass") + .setPakkage("org.springframework.boot.autoconfigure.condition") + .build()); + TypeNode clazzType = + TypeNode.withReference( + VaporReference.builder() + .setName("VisionServiceClient") + .setPakkage("com.foo.bar") + .build()); + + AnnotationNode annotation = + AnnotationNode.builder() + .setType(conditionalOnPropertyType) + .setDescription( + VariableExpr.builder() + .setVariable( + Variable.builder().setType(TypeNode.CLASS_OBJECT).setName("class").build()) + .setStaticReferenceType(clazzType) + .build()) + .build(); + annotation.accept(writerVisitor); + assertEquals("@ConditionalOnClass(VisionServiceClient.class)\n", writerVisitor.write()); + } + + @Test + public void writeAnnotation_withMultipleNamedDescriptions() { TypeNode conditionalOnPropertyType = TypeNode.withReference( VaporReference.builder() @@ -189,7 +234,8 @@ public void writeAnnotation_withNamedDescriptions() { AnnotationNode annotation = AnnotationNode.builder() .setType(conditionalOnPropertyType) - .setDescriptions(Arrays.asList(valueString, matchIfMissing)) + .addDescription(valueString) + .addDescription(matchIfMissing) .build(); annotation.accept(writerVisitor); assertEquals( @@ -197,6 +243,84 @@ public void writeAnnotation_withNamedDescriptions() { writerVisitor.write()); } + @Test + public void writeAnnotation_withInvalidDescriptions() { + TypeNode fakeAnnotationType = + TypeNode.withReference( + VaporReference.builder().setName("FakeAnnotation").setPakkage("com.foo.bar").build()); + String stringA = "a string"; + String stringB = "another string"; + + IllegalStateException exceptionForSetDescription = + assertThrows( + IllegalStateException.class, + () -> { + AnnotationNode.builder() + .setType(fakeAnnotationType) + .setDescription(stringA) + .setDescription( + ValueExpr.withValue( + PrimitiveValue.builder().setValue("12").setType(TypeNode.INT).build())) + .build(); + }); + assertThat(exceptionForSetDescription) + .hasMessageThat() + .contains("Single parameter with no name cannot be set multiple times"); + + assertThrows( + IllegalStateException.class, + () -> { + AnnotationNode.builder() + .setType(fakeAnnotationType) + .setDescription(stringA) + .setDescription(stringB) + .build(); + }); + + assertThrows( + IllegalStateException.class, + () -> { + AnnotationNode.builder() + .setType(fakeAnnotationType) + .setDescription(stringA) + .setDescription( + VariableExpr.builder() + .setVariable( + Variable.builder() + .setType(TypeNode.CLASS_OBJECT) + .setName("class") + .build()) + .setStaticReferenceType(TypeNode.LONG) + .build()) + .build(); + }); + + AssignmentExpr valueString = + AssignmentExpr.builder() + .setVariableExpr( + VariableExpr.withVariable( + Variable.builder().setName("value").setType(TypeNode.STRING).build())) + .setValueExpr( + ValueExpr.withValue( + StringObjectValue.withValue("spring.cloud.gcp.language.enabled"))) + .build(); + IllegalStateException exceptionForAddDescription = + assertThrows( + IllegalStateException.class, + () -> { + AnnotationNode.builder() + .setType(fakeAnnotationType) + .setDescription( + ValueExpr.withValue( + PrimitiveValue.builder().setValue("12").setType(TypeNode.INT).build())) + .addDescription(valueString) + .build(); + }); + assertThat(exceptionForAddDescription) + .hasMessageThat() + .contains("Multiple parameters must have names"); + } + @Test public void writeNewObjectExpr_basic() { // isGeneric() is true, but generics() is empty. @@ -776,22 +900,6 @@ public void writeAssignmentExpr_stringObjectValue() { assertThat(writerVisitor.write()).isEqualTo("String x = \"Hi! World. \\n\""); } - @Test - public void writeAssignmentExpr_clazzValue() { - TypeNode clazz = TypeNode.withReference(ConcreteReference.withClazz(List.class)); - - VariableExpr variableExpr = - VariableExpr.withVariable(Variable.builder().setName("value").setType(clazz).build()); - - ValueExpr valueExpr = ValueExpr.withValue(ClazzValue.builder().setType(clazz).build()); - - AssignmentExpr assignExpr = - AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); - - assignExpr.accept(writerVisitor); - assertEquals("value = List.class", writerVisitor.write()); - } - @Test public void writeMethodInvocationExpr_basic() { MethodInvocationExpr methodExpr = From 72c34b1befd1f25038b4eca4252609242b905ef5 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Fri, 15 Jul 2022 15:01:26 +0000 Subject: [PATCH 08/10] test updates to address code smells. --- .../engine/writer/JavaWriterVisitorTest.java | 69 ++++++++----------- 1 file changed, 28 insertions(+), 41 deletions(-) diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index 935e0229d8..16ec5fe7ab 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -250,18 +250,36 @@ public void writeAnnotation_withInvalidDescriptions() { VaporReference.builder().setName("FakeAnnotation").setPakkage("com.foo.bar").build()); String stringA = "a string"; String stringB = "another string"; + ValueExpr valueExpr = + ValueExpr.withValue(PrimitiveValue.builder().setValue("12").setType(TypeNode.INT).build()); + + VariableExpr clazzVariableExpr = + VariableExpr.builder() + .setVariable(Variable.builder().setType(TypeNode.CLASS_OBJECT).setName("class").build()) + .setExprReferenceExpr( + MethodInvocationExpr.builder() + .setMethodName("foobar") + .setReturnType(TypeNode.INT_OBJECT) + .build()) + .build(); + AssignmentExpr valueStringAssignmentExpr = + AssignmentExpr.builder() + .setVariableExpr( + VariableExpr.withVariable( + Variable.builder().setName("value").setType(TypeNode.STRING).build())) + .setValueExpr( + ValueExpr.withValue( + StringObjectValue.withValue("spring.cloud.gcp.language.enabled"))) + .build(); + + AnnotationNode.Builder annotationBuilder = + AnnotationNode.builder().setType(fakeAnnotationType).setDescription(stringA); IllegalStateException exceptionForSetDescription = assertThrows( IllegalStateException.class, () -> { - AnnotationNode.builder() - .setType(fakeAnnotationType) - .setDescription(stringA) - .setDescription( - ValueExpr.withValue( - PrimitiveValue.builder().setValue("12").setType(TypeNode.INT).build())) - .build(); + annotationBuilder.setDescription(valueExpr); }); assertThat(exceptionForSetDescription) .hasMessageThat() @@ -270,51 +288,20 @@ public void writeAnnotation_withInvalidDescriptions() { assertThrows( IllegalStateException.class, () -> { - AnnotationNode.builder() - .setType(fakeAnnotationType) - .setDescription(stringA) - .setDescription(stringB) - .build(); + annotationBuilder.setDescription(stringB); }); assertThrows( IllegalStateException.class, () -> { - AnnotationNode.builder() - .setType(fakeAnnotationType) - .setDescription(stringA) - .setDescription( - VariableExpr.builder() - .setVariable( - Variable.builder() - .setType(TypeNode.CLASS_OBJECT) - .setName("class") - .build()) - .setStaticReferenceType(TypeNode.LONG) - .build()) - .build(); + annotationBuilder.setDescription(clazzVariableExpr); }); - AssignmentExpr valueString = - AssignmentExpr.builder() - .setVariableExpr( - VariableExpr.withVariable( - Variable.builder().setName("value").setType(TypeNode.STRING).build())) - .setValueExpr( - ValueExpr.withValue( - StringObjectValue.withValue("spring.cloud.gcp.language.enabled"))) - .build(); IllegalStateException exceptionForAddDescription = assertThrows( IllegalStateException.class, () -> { - AnnotationNode.builder() - .setType(fakeAnnotationType) - .setDescription( - ValueExpr.withValue( - PrimitiveValue.builder().setValue("12").setType(TypeNode.INT).build())) - .addDescription(valueString) - .build(); + annotationBuilder.addDescription(valueStringAssignmentExpr); }); assertThat(exceptionForAddDescription) .hasMessageThat() From 4b61639be362be27557588eb987cb8c169afba9c Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Wed, 24 Aug 2022 14:49:43 +0000 Subject: [PATCH 09/10] Add optional annotation node to VariableExpr. --- .../generator/engine/ast/VariableExpr.java | 25 +++++++++++++- .../engine/writer/JavaWriterVisitor.java | 3 ++ .../engine/ast/VariableExprTest.java | 33 +++++++++++++++++++ .../engine/writer/JavaWriterVisitorTest.java | 19 +++++++++++ 4 files changed, 79 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java b/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java index 404afd516f..f882c97e8d 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/VariableExpr.java @@ -18,6 +18,8 @@ import com.google.auto.value.AutoValue; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; import java.util.stream.Collectors; import javax.annotation.Nullable; @@ -43,6 +45,9 @@ public abstract class VariableExpr implements Expr { public abstract boolean isVolatile(); + // Optional + public abstract ImmutableList annotations(); + // Please use this only in conjunction with methods. // Supports only parameterized types like Map. // TODO(unsupported): Fully generic arguments, e.g. foobar(K key, V value). @@ -77,7 +82,8 @@ public static Builder builder() { .setIsStatic(false) .setIsVolatile(false) .setScope(ScopeNode.LOCAL) - .setTemplateObjects(ImmutableList.of()); + .setTemplateObjects(ImmutableList.of()) + .setAnnotations(Collections.emptyList()); } public abstract Builder toBuilder(); @@ -102,6 +108,10 @@ public abstract static class Builder { public abstract Builder setIsVolatile(boolean isVolatile); + public abstract Builder setAnnotations(List annotations); + + abstract ImmutableList annotations(); + // This should be used only for method arguments. public abstract Builder setTemplateObjects(List objects); @@ -133,7 +143,20 @@ public VariableExpr build() { }) .collect(Collectors.toList())); + // Remove duplicates while maintaining insertion order. + ImmutableList processedAnnotations = annotations(); + setAnnotations( + new LinkedHashSet<>(processedAnnotations).stream().collect(Collectors.toList())); + VariableExpr variableExpr = autoBuild(); + + // TODO: should match on AnnotationNode @Target of ElementType.FIELD + if (!variableExpr.isDecl()) { + Preconditions.checkState( + variableExpr.annotations().isEmpty(), + "Annotation can only be added to variable declaration."); + } + if (variableExpr.isDecl() || variableExpr.exprReferenceExpr() != null) { Preconditions.checkState( variableExpr.isDecl() ^ (variableExpr.exprReferenceExpr() != null), diff --git a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java index 5a05267c52..731411bab6 100644 --- a/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java +++ b/src/main/java/com/google/api/generator/engine/writer/JavaWriterVisitor.java @@ -259,6 +259,9 @@ public void visit(VariableExpr variableExpr) { // VariableExpr will handle isDecl and exprReferenceExpr edge cases. if (variableExpr.isDecl()) { + // Annotations, if any. + annotations(variableExpr.annotations()); + if (!scope.equals(ScopeNode.LOCAL)) { scope.accept(this); space(); diff --git a/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java b/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java index 9e8b87e353..ccf76fae85 100644 --- a/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java @@ -173,6 +173,26 @@ public void validVariableExpr_templatedArgNameAndTypeInMethod() { .containsExactly(IdentifierNode.withName("RequestT"), TypeNode.STRING); } + @Test + public void validVariableExpr_declarationWithAnnotations() { + Variable variable = Variable.builder().setName("x").setType(TypeNode.BOOLEAN).build(); + VariableExpr variableExpr = + VariableExpr.builder() + .setVariable(variable) + .setIsDecl(true) + .setAnnotations( + Arrays.asList( + AnnotationNode.withSuppressWarnings("all"), + AnnotationNode.DEPRECATED, + AnnotationNode.DEPRECATED)) + .build(); + assertThat(variableExpr.variable()).isEqualTo(variable); + assertThat(variableExpr.type()).isEqualTo(TypeNode.VOID); + assertThat(variableExpr.isDecl()).isTrue(); + assertThat(variableExpr.annotations()) + .containsExactly(AnnotationNode.withSuppressWarnings("all"), AnnotationNode.DEPRECATED); + } + @Test public void invalidVariableExpr_templatedArgInMethodHasNonStringNonTypeNodeObject() { Variable variable = @@ -288,4 +308,17 @@ public void invalidVariableExpr_classFieldOnPrimitiveType() { .build()) .build()); } + + @Test + public void invalidVariableExpr_annotationNoDeclaration() { + Variable variable = Variable.builder().setName("x").setType(TypeNode.BOOLEAN).build(); + assertThrows( + IllegalStateException.class, + () -> + VariableExpr.builder() + .setVariable(variable) + .setIsDecl(false) + .setAnnotations(Arrays.asList(AnnotationNode.DEPRECATED)) + .build()); + } } diff --git a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java index 16ec5fe7ab..7bf7c4d13e 100644 --- a/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java @@ -887,6 +887,25 @@ public void writeAssignmentExpr_stringObjectValue() { assertThat(writerVisitor.write()).isEqualTo("String x = \"Hi! World. \\n\""); } + @Test + public void writeAssignmentExpr_variableDeclarationWithAnnotation() { + Variable variable = Variable.builder().setName("x").setType(TypeNode.STRING).build(); + VariableExpr variableExpr = + VariableExpr.builder() + .setVariable(variable) + .setIsDecl(true) + .setAnnotations(Arrays.asList(AnnotationNode.DEPRECATED, AnnotationNode.DEPRECATED)) + .build(); + + Value value = StringObjectValue.withValue("Hi! World. \n"); + Expr valueExpr = ValueExpr.builder().setValue(value).build(); + AssignmentExpr assignExpr = + AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); + + assignExpr.accept(writerVisitor); + assertThat(writerVisitor.write()).isEqualTo("@Deprecated\nString x = \"Hi! World. \\n\""); + } + @Test public void writeMethodInvocationExpr_basic() { MethodInvocationExpr methodExpr = From dcee621115f290612f9950ecb7928b18031e1938 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Wed, 24 Aug 2022 15:08:07 +0000 Subject: [PATCH 10/10] fix code smell: leave only invocation that throws exception inside lambda. --- .../api/generator/engine/ast/VariableExprTest.java | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java b/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java index ccf76fae85..43a5fd4f09 100644 --- a/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java +++ b/src/test/java/com/google/api/generator/engine/ast/VariableExprTest.java @@ -312,13 +312,11 @@ public void invalidVariableExpr_classFieldOnPrimitiveType() { @Test public void invalidVariableExpr_annotationNoDeclaration() { Variable variable = Variable.builder().setName("x").setType(TypeNode.BOOLEAN).build(); - assertThrows( - IllegalStateException.class, - () -> - VariableExpr.builder() - .setVariable(variable) - .setIsDecl(false) - .setAnnotations(Arrays.asList(AnnotationNode.DEPRECATED)) - .build()); + VariableExpr.Builder variableExprBuilder = + VariableExpr.builder() + .setVariable(variable) + .setIsDecl(false) + .setAnnotations(Arrays.asList(AnnotationNode.DEPRECATED)); + assertThrows(IllegalStateException.class, () -> variableExprBuilder.build()); } }