From 07257501b4d1af5f36707319cbfca41425e723de Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 17:13:32 -0800 Subject: [PATCH 01/17] fix: swap assertEquals args in JavaWriterVisitorTest to match (expected, actusl) order --- .../engine/writer/JavaWriterVisitorTest.java | 204 +++++++++--------- 1 file changed, 102 insertions(+), 102 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 dac3d20756..98379fac93 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 @@ -90,7 +90,7 @@ public void setUp() { public void writeIdentifier() { String idName = "foobar"; IdentifierNode.builder().setName(idName).build().accept(writerVisitor); - assertEquals(writerVisitor.write(), idName); + assertEquals(idName, writerVisitor.write()); } @Test @@ -98,7 +98,7 @@ public void writePrimitiveType() { TypeNode intType = TypeNode.INT; assertThat(intType).isNotNull(); intType.accept(writerVisitor); - assertEquals(writerVisitor.write(), "int"); + assertEquals("int", writerVisitor.write()); } @Test @@ -107,7 +107,7 @@ public void writePrimitiveArrayType() { TypeNode.builder().setTypeKind(TypeNode.TypeKind.BYTE).setIsArray(true).build(); assertThat(byteArrayType).isNotNull(); byteArrayType.accept(writerVisitor); - assertEquals(writerVisitor.write(), "byte[]"); + assertEquals("byte[]", writerVisitor.write()); } @Test @@ -144,14 +144,14 @@ public void writeReferenceType_useFullName() { public void writeAnnotation_simple() { AnnotationNode annotation = AnnotationNode.OVERRIDE; annotation.accept(writerVisitor); - assertEquals(writerVisitor.write(), "@Override\n"); + assertEquals("@Override\n", writerVisitor.write()); } @Test public void writeAnnotation_withDescription() { AnnotationNode annotation = AnnotationNode.withSuppressWarnings("all"); annotation.accept(writerVisitor); - assertEquals(writerVisitor.write(), "@SuppressWarnings(\"all\")\n"); + assertEquals("@SuppressWarnings(\"all\")\n", writerVisitor.write()); } @Test @@ -161,7 +161,7 @@ public void writeNewObjectExpr_basic() { TypeNode type = TypeNode.withReference(ref); NewObjectExpr newObjectExpr = NewObjectExpr.builder().setIsGeneric(true).setType(type).build(); newObjectExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "new List<>()"); + assertEquals("new List<>()", writerVisitor.write()); } @Test @@ -182,7 +182,7 @@ public void writeNewObjectExpr_withMethodExprArgs() { .setArguments(Arrays.asList(msgExpr, causeExpr)) .build(); newObjectExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "new IOException(message, cause())"); + assertEquals("new IOException(message, cause())", writerVisitor.write()); } @Test @@ -222,7 +222,7 @@ public void writeNewObjectExpr_withGenericsAndArgs() { .build(); newObjectExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), "new HashMap>(SomeClass.foobar(), num)"); + "new HashMap>(SomeClass.foobar(), num)", writerVisitor.write()); } /** =============================== EXPRESSIONS =============================== */ @@ -231,7 +231,7 @@ public void writeValueExpr() { Value value = PrimitiveValue.builder().setType(TypeNode.INT).setValue("3").build(); ValueExpr valueExpr = ValueExpr.builder().setValue(value).build(); valueExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "3"); + assertEquals("3", writerVisitor.write()); } @Test @@ -240,7 +240,7 @@ public void writeVariableExpr_basic() { VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x"); + assertEquals("x", writerVisitor.write()); } @Test @@ -257,7 +257,7 @@ public void writeVariableExpr_wildcardType() { VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "List x"); + assertEquals("List x", writerVisitor.write()); } @Test @@ -277,7 +277,7 @@ public void writeVariableExpr_wildcardTypeWithUpperBound() { VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "List x"); + assertEquals("List x", writerVisitor.write()); } @Test @@ -290,7 +290,7 @@ public void writeVariableExpr_staticReference() { .build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "Integer.MAX_VALUE"); + assertEquals("Integer.MAX_VALUE", writerVisitor.write()); } @Test @@ -305,7 +305,7 @@ public void writeVariableExpr_nonDeclIgnoresModifiers() { .build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x"); + assertEquals("x", writerVisitor.write()); } @Test @@ -314,7 +314,7 @@ public void writeVariableExpr_basicLocalDecl() { VariableExpr expr = VariableExpr.builder().setVariable(variable).setIsDecl(true).build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "int x"); + assertEquals("int x", writerVisitor.write()); } @Test @@ -325,7 +325,7 @@ public void writeVariableExpr_localFinalDecl() { VariableExpr.builder().setVariable(variable).setIsFinal(true).setIsDecl(true).build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "final boolean x"); + assertEquals("final boolean x", writerVisitor.write()); } @Test @@ -340,7 +340,7 @@ public void writeVariableExpr_scopedDecl() { .build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "private int x"); + assertEquals("private int x", writerVisitor.write()); } @Test @@ -357,7 +357,7 @@ public void writeVariableExpr_scopedStaticFinalDecl() { .build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public static final boolean x"); + assertEquals("public static final boolean x", writerVisitor.write()); } @Test @@ -375,7 +375,7 @@ public void writeVariableExpr_scopedStaticFinalVolatileDecl() { .build(); expr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "private static final volatile boolean x"); + assertEquals("private static final volatile boolean x", writerVisitor.write()); } @Test @@ -387,7 +387,7 @@ public void writeVariableExpr_basicReference() { variableExpr = VariableExpr.builder().setVariable(subVariable).setExprReferenceExpr(variableExpr).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x.length"); + assertEquals("x.length", writerVisitor.write()); } @Test @@ -405,7 +405,7 @@ public void writeVariableExpr_basicReferenceWithModifiersSet() { .setIsStatic(true) .build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x.length"); + assertEquals("x.length", writerVisitor.write()); } @Test @@ -424,7 +424,7 @@ public void writeVariableExpr_nestedReference() { variableExpr = VariableExpr.builder().setVariable(subVariable).setExprReferenceExpr(variableExpr).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x.someStringField.anotherStringField.lengthField"); + assertEquals("x.someStringField.anotherStringField.lengthField", writerVisitor.write()); } @Test @@ -463,7 +463,7 @@ public void writeBlockCommentStatement_basic() { CommentStatement commentStatement = CommentStatement.withComment(blockComment); String expected = String.format(createLines(3), "/*\n", "* this is a test comment\n", "*/\n"); commentStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -473,7 +473,7 @@ public void writeLineCommentStatement_basic() { CommentStatement commentStatement = CommentStatement.withComment(lineComment); String expected = "// this is a test comment\n"; commentStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -535,7 +535,7 @@ public void writeJavaDocCommentStatement_allComponents() { "* @deprecated Use the {@link ArchivedBookName} class instead.\n", "*/\n"); commentStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -546,7 +546,7 @@ public void writeBlockComment_shortLines() { String.format( createLines(4), "/*\n", "* Apache License\n", "* This is a test file header\n", "*/\n"); blockComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -566,7 +566,7 @@ public void writeBlockComment_newLineInBetween() { "* you may not use this file except in compliance with the License.\n", "*/\n"); blockComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -582,7 +582,7 @@ public void writeLineComment_longLine() { + " for 3 times,\n", "// blah, blah!\n"); lineComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -599,7 +599,7 @@ public void writeLineComment_specialChar() { + "// \\r" + "\"]']\n"; lineComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -639,7 +639,7 @@ public void writeJavaDocComment_specialChar() { "* RPC method comment may include special characters: <>&\"`'{@literal @}.\n", "*/\n"); javaDocComment.accept(writerVisitor); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -663,7 +663,7 @@ public void writeTernaryExpr_basic() { .setElseExpr(elseExpr) .build(); ternaryExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "condition ? 3 : 4"); + assertEquals("condition ? 3 : 4", writerVisitor.write()); } @Test @@ -679,7 +679,7 @@ public void writeAssignmentExpr_basicValue() { AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); assignExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "int x = 3"); + assertEquals("int x = 3", writerVisitor.write()); } @Test @@ -701,7 +701,7 @@ public void writeAssignmentExpr_varToVar() { AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); assignExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "private static final int foobar = y"); + assertEquals("private static final int foobar = y", writerVisitor.write()); } @Test @@ -717,7 +717,7 @@ public void writeAssignmentExpr_nullObjectValueReferenceType() { AssignmentExpr.builder().setVariableExpr(variableExpr).setValueExpr(valueExpr).build(); assignExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "String x = null"); + assertEquals("String x = null", writerVisitor.write()); } @Test @@ -749,7 +749,7 @@ public void writeMethodInvocationExpr_basic() { MethodInvocationExpr.builder().setMethodName("foobar").build(); methodExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "foobar()"); + assertEquals("foobar()", writerVisitor.write()); } @Test @@ -768,7 +768,7 @@ public void writeMethodInvocationExpr_staticRef() { .build(); methodExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "SomeClass.foobar()"); + assertEquals("SomeClass.foobar()", writerVisitor.write()); } @Test @@ -813,9 +813,9 @@ public void writeMethodInvocationExpr_genericWithArgs() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), "final String someStr = anArg.," - + " HashMap>>foobar(anArg, anArg, anArg)"); + + " HashMap>>foobar(anArg, anArg, anArg)", + writerVisitor.write()); } @Test @@ -841,7 +841,7 @@ public void writeMethodInvocationExpr_chained() { methodExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), "libraryClient.streamBooksCallable().doAnotherThing().call()"); + "libraryClient.streamBooksCallable().doAnotherThing().call()", writerVisitor.write()); } @Test @@ -854,7 +854,7 @@ public void writeCastExpr_basic() { .setExpr(varExpr) .build(); castExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "((Object) str)"); + assertEquals("((Object) str)", writerVisitor.write()); } @Test @@ -878,7 +878,7 @@ public void writeCastExpr_methodInvocation() { .setExpr(methodExpr) .build(); castExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "((Object) SomeClass.foobar())"); + assertEquals("((Object) SomeClass.foobar())", writerVisitor.write()); } @Test @@ -892,7 +892,7 @@ public void writeCastExpr_nested() { .build(); castExpr = CastExpr.builder().setType(TypeNode.STRING).setExpr(castExpr).build(); castExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "((String) ((Object) str))"); + assertEquals("((String) ((Object) str))", writerVisitor.write()); } @Test @@ -914,13 +914,13 @@ public void writeAnonymousClassExpr_basic() { AnonymousClassExpr.builder().setType(type).setMethods(Arrays.asList(method)).build(); anonymousClassExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(4), "new Runnable() {\n", "@Override\n", "public void run() {\n", - "boolean foobar = false;\n}\n\n}")); + "boolean foobar = false;\n}\n\n}"), + writerVisitor.write()); } @Test @@ -966,7 +966,7 @@ public void writeAnonymousClassExpr_withStatementsMethods() { "@Override\n", "public void run() {\n", "int x = 3;\n}\n\n}"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1023,7 +1023,7 @@ public void writeAnonymousClassExpr_generics() { "public MethodDefinition apply(List arg) {\n", "return returnArg;\n", "}\n\n}"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1032,7 +1032,7 @@ public void writeThrowExpr_basic() { TypeNode.withReference(ConcreteReference.withClazz(NullPointerException.class)); ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).build(); throwExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "throw new NullPointerException()"); + assertEquals("throw new NullPointerException()", writerVisitor.write()); } @Test @@ -1042,7 +1042,7 @@ public void writeThrowExpr_basicWithMessage() { String message = "Some message asdf"; ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).setMessageExpr(message).build(); throwExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "throw new NullPointerException(\"Some message asdf\")"); + assertEquals("throw new NullPointerException(\"Some message asdf\")", writerVisitor.write()); } @Test @@ -1056,7 +1056,7 @@ public void writeThrowExpr_messageExpr() { ThrowExpr throwExpr = ThrowExpr.builder().setType(npeType).setMessageExpr(messageExpr).build(); throwExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "throw new NullPointerException(foobar())"); + assertEquals("throw new NullPointerException(foobar())", writerVisitor.write()); } @Test @@ -1066,7 +1066,7 @@ public void writeInstanceofExpr() { InstanceofExpr instanceofExpr = InstanceofExpr.builder().setCheckType(TypeNode.STRING).setExpr(variableExpr).build(); instanceofExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "x instanceof String"); + assertEquals("x instanceof String", writerVisitor.write()); } @Test @@ -1080,7 +1080,7 @@ public void writeEnumRefExpr_basic() { EnumRefExpr enumRefExpr = EnumRefExpr.builder().setName("VOID").setType(enumType).build(); enumRefExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "TypeKind.VOID"); + assertEquals("TypeKind.VOID", writerVisitor.write()); } @Test @@ -1089,7 +1089,7 @@ public void writeEnumRefExpr_nested() { TypeNode.withReference(ConcreteReference.withClazz(TypeNode.TypeKind.class)); EnumRefExpr enumRefExpr = EnumRefExpr.builder().setName("VOID").setType(enumType).build(); enumRefExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "TypeNode.TypeKind.VOID"); + assertEquals("TypeNode.TypeKind.VOID", writerVisitor.write()); } @Test @@ -1097,7 +1097,7 @@ public void writeReturnExpr_basic() { ReturnExpr returnExpr = ReturnExpr.withExpr(ValueExpr.withValue(StringObjectValue.withValue("asdf"))); returnExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "return \"asdf\""); + assertEquals("return \"asdf\"", writerVisitor.write()); } /** =============================== STATEMENTS =============================== */ @@ -1118,14 +1118,14 @@ public void writeExprStatement() { ExprStatement exprStatement = ExprStatement.withExpr(methodExpr); exprStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "SomeClass.foobar();\n"); + assertEquals("SomeClass.foobar();\n", writerVisitor.write()); } @Test public void writeBlockStatement_empty() { BlockStatement blockStatement = BlockStatement.builder().build(); blockStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "{\n}\n"); + assertEquals("{\n}\n", writerVisitor.write()); } @Test @@ -1146,7 +1146,7 @@ public void writeBlockStatement_simple() { BlockStatement.builder().setBody(Arrays.asList(ExprStatement.withExpr(methodExpr))).build(); blockStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "{\nSomeClass.foobar();\n}\n"); + assertEquals("{\nSomeClass.foobar();\n}\n", writerVisitor.write()); } @Test @@ -1172,7 +1172,7 @@ public void writeBlockStatement_static() { .build(); blockStatement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "static {\nSomeClass.foobar();\nSomeClass.foobar();\n}\n"); + assertEquals("static {\nSomeClass.foobar();\nSomeClass.foobar();\n}\n", writerVisitor.write()); } @Test @@ -1187,8 +1187,8 @@ public void writeIfStatement_simple() { ifStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("%s%s%s%s", "if (condition) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + String.format("%s%s%s%s", "if (condition) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1207,7 +1207,6 @@ public void writeIfStatement_withElse() { ifStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s" + "%s%s%s%s", "if (condition) {\n", @@ -1216,7 +1215,8 @@ public void writeIfStatement_withElse() { "} else {\n", "int x = 3;\n", "int x = 3;\n", - "}\n")); + "}\n"), + writerVisitor.write()); } @Test @@ -1258,7 +1258,7 @@ public void writeIfStatement_elseIfs() { "boolean fooBar = true;\n", "}\n"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1322,7 +1322,7 @@ public void writeIfStatement_nested() { "boolean fooBar = true;\n", "}\n", "}\n"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1337,8 +1337,8 @@ public void writeWhileStatement_simple() { whileStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("%s%s%s%s", "while (condition) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + String.format("%s%s%s%s", "while (condition) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1359,10 +1359,10 @@ public void writeForStatement() { forStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s", - "for (String str : getSomeStrings()) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + "for (String str : getSomeStrings()) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1382,10 +1382,10 @@ public void writeGeneralForStatement_basicIsDecl() { forStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s", - "for (int i = 0; i < maxSize(); i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + "for (int i = 0; i < maxSize(); i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1405,9 +1405,9 @@ public void writeGeneralForStatement_basicIsNotDecl() { forStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( - "%s%s%s%s", "for (i = 1; i < 10; i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n")); + "%s%s%s%s", "for (i = 1; i < 10; i++) {\n", "int x = 3;\n", "int x = 3;\n", "}\n"), + writerVisitor.write()); } @Test @@ -1426,10 +1426,10 @@ public void writeTryCatchStatement_simple() { tryCatch.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s", - "try {\n", "int x = 3;\n", "} catch (IllegalArgumentException e) {\n", "}\n")); + "try {\n", "int x = 3;\n", "} catch (IllegalArgumentException e) {\n", "}\n"), + writerVisitor.write()); } @Test @@ -1452,14 +1452,14 @@ public void writeTryCatchStatement_withResources() { tryCatch.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s%s", "try (boolean aBool = false) {\n", "int y = 4;\n", "} catch (IllegalArgumentException e) {\n", "int foobar = 123;\n", - "}\n")); + "}\n"), + writerVisitor.write()); } @Test @@ -1477,7 +1477,7 @@ public void writeTryCatchStatement_sampleCodeNoCatch() { .build(); tryCatch.accept(writerVisitor); - assertEquals(writerVisitor.write(), String.format("%s%s%s", "try {\n", "int x = 3;\n", "}\n")); + assertEquals(String.format("%s%s%s", "try {\n", "int x = 3;\n", "}\n"), writerVisitor.write()); } @Test @@ -1501,14 +1501,14 @@ public void writeTryCatchStatement_sampleCodeWithCatch() { tryCatch.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s%s", "try (boolean aBool = false) {\n", "int y = 4;\n", "} catch (IllegalArgumentException e) {\n", "int foobar = 123;\n", - "}\n")); + "}\n"), + writerVisitor.write()); } @Test @@ -1524,8 +1524,8 @@ public void writeSynchronizedStatement_basicThis() { .build(); synchronizedStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format(createLines(3), "synchronized (this) {\n", "doStuff();\n", "}\n")); + String.format(createLines(3), "synchronized (this) {\n", "doStuff();\n", "}\n"), + writerVisitor.write()); } @Test @@ -1543,8 +1543,8 @@ public void writeSynchronizedStatement_basicVariableExpr() { .build(); synchronizedStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format(createLines(3), "synchronized (str) {\n", "doStuff();\n", "}\n")); + String.format(createLines(3), "synchronized (str) {\n", "doStuff();\n", "}\n"), + writerVisitor.write()); } @Test @@ -1560,8 +1560,8 @@ public void writeMethodDefinition_basic() { methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("%s%s%s", "public void close() {\n", "int x = 3;\n", "}\n\n")); + String.format("%s%s%s", "public void close() {\n", "int x = 3;\n", "}\n\n"), + writerVisitor.write()); } @Test @@ -1579,7 +1579,7 @@ public void writeMethodDefinition_constructor() { .build(); methodDefinition.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public LibrarySettings() {\n}\n\n"); + assertEquals("public LibrarySettings() {\n}\n\n", writerVisitor.write()); } @Test @@ -1592,7 +1592,7 @@ public void writeMethodDefinition_basicEmptyBody() { .build(); methodDefinition.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public void close() {\n}\n\n"); + assertEquals("public void close() {\n}\n\n", writerVisitor.write()); } @Test @@ -1609,8 +1609,8 @@ public void writeMethodDefinition_basicAbstract() { methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("%s%s%s", "public abstract void close() {\n", "int x = 3;\n", "}\n\n")); + String.format("%s%s%s", "public abstract void close() {\n", "int x = 3;\n", "}\n\n"), + writerVisitor.write()); } @Test @@ -1624,7 +1624,7 @@ public void writeMethodDefinition_basicAbstractEmptyBody() { .build(); methodDefinition.accept(writerVisitor); - assertEquals(writerVisitor.write(), "public abstract void close();\n"); + assertEquals("public abstract void close();\n", writerVisitor.write()); } @Test @@ -1658,13 +1658,13 @@ public void writeMethodDefinition_withArgumentsAndReturnExpr() { methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( "%s%s%s%s", "public int close(int x, int y) {\n", "boolean foobar = false;\n", "return 3;\n", - "}\n\n")); + "}\n\n"), + writerVisitor.write()); } @Test @@ -1742,7 +1742,7 @@ public void writeMethodDefinition_withCommentsAnnotationsAndThrows() { "boolean foobar = false;\n", "return 3;\n", "}\n\n"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -1779,12 +1779,12 @@ public void writeMethodDefinition_templatedReturnTypeAndArguments() { methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "public Map close(Map x, Map y) {\n", "return foobar();\n", - "}\n\n")); + "}\n\n"), + writerVisitor.write()); } @Test @@ -1801,7 +1801,6 @@ public void writeClassDefinition_basicWithFileHeader() { classDef.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(6), "/*\n", @@ -1809,7 +1808,8 @@ public void writeClassDefinition_basicWithFileHeader() { " */\n\n", "package com.google.example.library.v1.stub;\n", "\n", - "public class LibraryServiceStub {}\n")); + "public class LibraryServiceStub {}\n"), + writerVisitor.write()); } @Test @@ -1833,7 +1833,6 @@ public void writeClassDefinition_withAnnotationsExtendsAndImplements() { classDef.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(5), "package com.google.example.library.v1.stub;\n", @@ -1841,7 +1840,8 @@ public void writeClassDefinition_withAnnotationsExtendsAndImplements() { "@Deprecated\n", "@SuppressWarnings(\"all\")\n", "public final class LibraryServiceStub extends String implements Appendable," - + " Cloneable, Readable {}\n")); + + " Cloneable, Readable {}\n"), + writerVisitor.write()); } @Test @@ -1981,7 +1981,7 @@ public void writeClassDefinition_commentsStatementsAndMethods() { " }\n", " }\n", "}\n"); - assertEquals(writerVisitor.write(), expected); + assertEquals(expected, writerVisitor.write()); } @Test @@ -2032,8 +2032,8 @@ public void writeThisObjectValue_methodReturn() { .build(); methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format("public Student apply() {\n" + "return this;\n" + "}\n\n")); + String.format("public Student apply() {\n" + "return this;\n" + "}\n\n"), + writerVisitor.write()); } @Test @@ -2241,7 +2241,7 @@ public void writeAssignmentOperationExpr_xorAssignment() { public void writeEmptyLineStatement() { EmptyLineStatement statement = EmptyLineStatement.create(); statement.accept(writerVisitor); - assertEquals(writerVisitor.write(), "\n"); + assertEquals("\n", writerVisitor.write()); } @Test From 67f3a152087a9d650bb18bf67106025576f46e51 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 17:15:21 -0800 Subject: [PATCH 02/17] fix: swap assertEquals args in ImportWriterVisitorTest to match (expected, actusl) order --- .../writer/ImportWriterVisitorTest.java | 118 +++++++++--------- 1 file changed, 59 insertions(+), 59 deletions(-) diff --git a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java index 5463ac76ad..5c5b2d5773 100644 --- a/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java +++ b/src/test/java/com/google/api/generator/engine/writer/ImportWriterVisitorTest.java @@ -123,7 +123,7 @@ public void writeNewObjectExprImports_basic() { .setType(TypeNode.withReference(ConcreteReference.withClazz(ArrayList.class))) .build(); newObjectExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import java.util.ArrayList;\n\n"); + assertEquals("import java.util.ArrayList;\n\n", writerVisitor.write()); } @Test @@ -141,9 +141,9 @@ public void writeNewObjectExprImports_withArgs() { .build(); newObjectExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( - createLines(2), "import java.io.File;\n", "import java.io.FileOutputStream;\n\n")); + createLines(2), "import java.io.File;\n", "import java.io.FileOutputStream;\n\n"), + writerVisitor.write()); } @Test @@ -174,8 +174,8 @@ public void writeNewObjectExprImports_genericsAndVariableArgs() { .build(); newObjectExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format(createLines(2), "import java.util.HashMap;\n", "import java.util.List;\n\n")); + String.format(createLines(2), "import java.util.HashMap;\n", "import java.util.List;\n\n"), + writerVisitor.write()); } @Test @@ -203,9 +203,9 @@ public void writeNewObjectExprImports_methodExprArg() { .build(); newObjectExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( - createLines(2), "import java.io.IOException;\n", "import java.util.HashMap;\n\n")); + createLines(2), "import java.io.IOException;\n", "import java.util.HashMap;\n\n"), + writerVisitor.write()); } @Test @@ -239,12 +239,12 @@ public void writeTernaryExprImports() { .build(); ternaryExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.Expr;\n", "import com.google.api.generator.engine.ast.TypeNode;\n", - "import com.google.common.base.Strings;\n\n")); + "import com.google.common.base.Strings;\n\n"), + writerVisitor.write()); } @Test @@ -273,12 +273,12 @@ public void writeAssignmentExprImports() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.AstNode;\n", "import com.google.api.generator.engine.ast.ClassDefinition;\n", - "import com.google.api.some.pakkage.SomeClass;\n\n")); + "import com.google.api.some.pakkage.SomeClass;\n\n"), + writerVisitor.write()); } @Test @@ -328,7 +328,6 @@ public void writeAssignmentExprImports_concreteAndNestedGenerics() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(7), "import static java.util.Map.Entry;\n\n", @@ -337,7 +336,8 @@ public void writeAssignmentExprImports_concreteAndNestedGenerics() { "import com.google.api.generator.engine.ast.ClassDefinition;\n", "import java.util.ArrayList;\n", "import java.util.List;\n", - "import java.util.Map;\n\n")); + "import java.util.Map;\n\n"), + writerVisitor.write()); } @Test @@ -370,12 +370,12 @@ public void writeAssignmentExprImports_static() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import static java.util.Map.Entry;\n\n", "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.AstNode;\n\n")); + "import com.google.api.generator.engine.ast.AstNode;\n\n"), + writerVisitor.write()); } @Test @@ -408,12 +408,12 @@ public void writeAssignmentExprImports_notStatic() { assignExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", "import com.google.api.generator.engine.ast.AstNode;\n", - "import java.util.Map;\n\n")); + "import java.util.Map;\n\n"), + writerVisitor.write()); } @Test @@ -428,11 +428,11 @@ public void writeCastExprImports() { .build(); castExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.Expr;\n\n")); + "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -471,12 +471,12 @@ public void importFromVaporAndConcreteReferences() { varExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.MethodDefinition;\n", "import java.util.HashMap;\n", - "import java.util.List;\n\n")); + "import java.util.List;\n\n"), + writerVisitor.write()); } @Test @@ -489,8 +489,8 @@ public void writeVariableExprImports_basic() { VariableExpr variableExpr = VariableExpr.builder().setVariable(variable).build(); variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), - String.format(createLines(1), "import com.google.api.generator.engine.ast.Expr;\n\n")); + String.format(createLines(1), "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -509,11 +509,11 @@ public void writeVariableExprImports_staticReference() { variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.TypeNode;\n\n")); + "import com.google.api.generator.engine.ast.TypeNode;\n\n"), + writerVisitor.write()); } @Test @@ -531,7 +531,7 @@ public void writeVariableExprImports_wildcardType() { VariableExpr.builder().setIsDecl(true).setVariable(variable).build(); variableExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import java.util.List;\n\n"); + assertEquals("import java.util.List;\n\n", writerVisitor.write()); } @Test @@ -553,11 +553,11 @@ public void writeVariableExprImport_wildcardTypeWithUpperBound() { variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.Expr;\n", - "import java.util.List;\n\n")); + "import java.util.List;\n\n"), + writerVisitor.write()); } @Test @@ -578,11 +578,11 @@ public void writeVariableExprImports_reference() { VariableExpr.builder().setVariable(subVariable).setExprReferenceExpr(variableExpr).build(); variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.Expr;\n\n")); + "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -611,12 +611,12 @@ public void writeVariableExprImports_nestedReference() { variableExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(3), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", "import com.google.api.generator.engine.ast.Expr;\n", - "import com.google.api.generator.engine.ast.VariableExpr;\n\n")); + "import com.google.api.generator.engine.ast.VariableExpr;\n\n"), + writerVisitor.write()); } @Test @@ -685,14 +685,14 @@ public void writeAnonymousClassExprImports() { .build(); anonymousClassExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(5), "import com.google.api.generator.engine.ast.MethodDefinition;\n", "import com.google.common.base.Function;\n", "import java.io.IOException;\n", "import java.util.HashMap;\n", - "import java.util.List;\n\n")); + "import java.util.List;\n\n"), + writerVisitor.write()); } @Test @@ -703,7 +703,7 @@ public void writeThrowExprImports_basic() { ThrowExpr throwExpr = ThrowExpr.builder().setType(exceptionTypes).setMessageExpr(message).build(); throwExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import java.io.IOException;\n\n"); + assertEquals("import java.io.IOException;\n\n", writerVisitor.write()); } @Test @@ -727,11 +727,11 @@ public void writeThrowExprImports_messageExpr() { throwExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.Expr;\n", - "import com.google.api.generator.engine.ast.IfStatement;\n\n")); + "import com.google.api.generator.engine.ast.IfStatement;\n\n"), + writerVisitor.write()); } @Test @@ -747,11 +747,11 @@ public void writeInstanceofExprImports_basic() { InstanceofExpr.builder().setExpr(variableExpr).setCheckType(exprType).build(); expr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import com.google.api.generator.engine.ast.Expr;\n\n")); + "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -766,8 +766,8 @@ public void writeEnumRefExprImports_basic() { enumRefExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), - "import static com.google.api.generator.engine.ast.TypeNode.TypeKind;\n\n"); + "import static com.google.api.generator.engine.ast.TypeNode.TypeKind;\n\n", + writerVisitor.write()); } @Test @@ -776,7 +776,7 @@ public void writeEnumRefExprImports_nested() { TypeNode.withReference(ConcreteReference.withClazz(TypeNode.TypeKind.class)); EnumRefExpr enumRefExpr = EnumRefExpr.builder().setName("VOID").setType(enumType).build(); enumRefExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.TypeNode;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.TypeNode;\n\n", writerVisitor.write()); } @Test @@ -788,7 +788,7 @@ public void writeReturnExprImports_basic() { .setReturnType(TypeNode.withReference(ConcreteReference.withClazz(Expr.class))) .build()); returnExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.Expr;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.Expr;\n\n", writerVisitor.write()); } @Test @@ -827,11 +827,11 @@ public void writeMethodDefinitionImports_templatedMixedNamesAndTypes() { .build(); methodDefinition.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import java.util.Map;\n\n")); + "import java.util.Map;\n\n"), + writerVisitor.write()); } @Test @@ -842,7 +842,7 @@ public void writeReferenceConstructorExprImports_basic() { ReferenceConstructorExpr referenceConstructorExpr = ReferenceConstructorExpr.superBuilder().setType(classType).build(); referenceConstructorExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.example.v1.Parent;\n\n"); + assertEquals("import com.google.example.v1.Parent;\n\n", writerVisitor.write()); } @Test @@ -877,7 +877,7 @@ public void writeArithmeticOperationExprImports() { ArithmeticOperationExpr arithmeticOperationExpr = ArithmeticOperationExpr.concatWithExprs(lhsExpr, rhsExpr); arithmeticOperationExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.Expr;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.Expr;\n\n", writerVisitor.write()); } @Test @@ -897,11 +897,11 @@ public void writeSynchronizedStatementImports_basicThis() { .build(); synchronizedStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.Expr;\n", - "import java.util.Arrays;\n\n")); + "import java.util.Arrays;\n\n"), + writerVisitor.write()); } @Test @@ -920,7 +920,7 @@ public void writeSuperObjectValueImports() { .setReturnType(TypeNode.STRING) .build(); methodExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.example.examples.v1.Student;\n\n"); + assertEquals("import com.google.example.examples.v1.Student;\n\n", writerVisitor.write()); } @Test @@ -945,11 +945,11 @@ public void writeSynchronizedStatementImports_basicVariableExpr() { .build(); synchronizedStatement.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.ast.AssignmentExpr;\n", - "import java.util.Map;\n\n")); + "import java.util.Map;\n\n"), + writerVisitor.write()); } @Test @@ -962,7 +962,7 @@ public void writeUnaryOperationExprImports_LogicalNot() { .build(); UnaryOperationExpr unaryOperationExpr = UnaryOperationExpr.logicalNotWithExpr(expr); unaryOperationExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.Expr;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.Expr;\n\n", writerVisitor.write()); } @Test @@ -975,7 +975,7 @@ public void writeUnaryOperationExprImports_PostIncrement() { .build(); UnaryOperationExpr unaryOperationExpr = UnaryOperationExpr.postfixIncrementWithExpr(expr); unaryOperationExpr.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import com.google.api.generator.engine.ast.Expr;\n\n"); + assertEquals("import com.google.api.generator.engine.ast.Expr;\n\n", writerVisitor.write()); } @Test @@ -1002,11 +1002,11 @@ public void writeRelationalOperationExprImports() { RelationalOperationExpr.equalToWithExprs(lhsExpr, rhsExpr); relationalOperationExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), String.format( createLines(2), "import com.google.api.generator.engine.SomeClass;\n", - "import com.google.api.generator.engine.ast.Expr;\n\n")); + "import com.google.api.generator.engine.ast.Expr;\n\n"), + writerVisitor.write()); } @Test @@ -1024,8 +1024,8 @@ public void writeLogicalOperationExprImports() { LogicalOperationExpr.logicalAndWithExprs(lhsExpr, rhsExpr); logicalOperationExpr.accept(writerVisitor); assertEquals( - writerVisitor.write(), - "import com.google.api.generator.engine.ast.UnaryOperationExpr;\n\n"); + "import com.google.api.generator.engine.ast.UnaryOperationExpr;\n\n", + writerVisitor.write()); } @Test @@ -1052,7 +1052,7 @@ public void writePackageInfoDefinitionImports() { .build(); packageInfo.accept(writerVisitor); - assertEquals(writerVisitor.write(), "import javax.annotation.Generated;\n\n"); + assertEquals("import javax.annotation.Generated;\n\n", writerVisitor.write()); } /** =============================== HELPERS =============================== */ From 1e1a86b614fe7141cc851b436cc9da688c1d1b45 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 17:32:45 -0800 Subject: [PATCH 03/17] 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; } From 115e9fb2e941c30305dbebb3527bc56b30934d2c Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:16:49 -0800 Subject: [PATCH 04/17] fix: validate non-null elements for IfStatement --- .../com/google/api/generator/engine/ast/IfStatement.java | 4 ++++ .../google/api/generator/engine/ast/NodeValidator.java | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/IfStatement.java b/src/main/java/com/google/api/generator/engine/ast/IfStatement.java index 0252d61185..6451ab38ee 100644 --- a/src/main/java/com/google/api/generator/engine/ast/IfStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/IfStatement.java @@ -63,6 +63,10 @@ public Builder addElseIf(Expr conditionExpr, List body) { public IfStatement build() { IfStatement ifStatement = autoBuild(); + NodeValidator.checkNoNullElements(ifStatement.body(), "body", "if-statement"); + NodeValidator.checkNoNullElements(ifStatement.elseIfs(), "else-ifs", "if-statement"); + NodeValidator.checkNoNullElements(ifStatement.elseBody(), "else-body", "if-statement"); + Preconditions.checkState( ifStatement.conditionExpr().type().equals(TypeNode.BOOLEAN), "If-condition must be a boolean-typed expression"); 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 index 5eba84e936..b6eb9c6915 100644 --- a/src/main/java/com/google/api/generator/engine/ast/NodeValidator.java +++ b/src/main/java/com/google/api/generator/engine/ast/NodeValidator.java @@ -16,6 +16,7 @@ import com.google.common.base.Preconditions; import java.util.Collection; +import java.util.Map; import java.util.Objects; public class NodeValidator { @@ -26,4 +27,11 @@ public static void checkNoNullElements( String.format( "Found null expression in %s %s for %s", fieldTypeName, collection, nodeContextInfo)); } + + public static void checkNoNullElements( + Map map, String fieldTypeName, String nodeContextInfo) { + checkNoNullElements(map.keySet(), String.format("key of %s", fieldTypeName), nodeContextInfo); + checkNoNullElements( + map.values(), String.format("values of %s", fieldTypeName), nodeContextInfo); + } } From 1dac12039cec87ba50a6a050f9be96dec938de79 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:19:02 -0800 Subject: [PATCH 05/17] fix: validate non-null elements for AnonymousClassExpr --- .../generator/engine/ast/AnonymousClassExpr.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/AnonymousClassExpr.java b/src/main/java/com/google/api/generator/engine/ast/AnonymousClassExpr.java index e4ddc7e11d..ac0723c5e7 100644 --- a/src/main/java/com/google/api/generator/engine/ast/AnonymousClassExpr.java +++ b/src/main/java/com/google/api/generator/engine/ast/AnonymousClassExpr.java @@ -58,18 +58,26 @@ public Builder setMethods(MethodDefinition... methods) { public AnonymousClassExpr build() { AnonymousClassExpr anonymousClassExpr = autoBuild(); + // 1. The anonymous class expression should be reference types. Preconditions.checkState( TypeNode.isReferenceType(anonymousClassExpr.type()), "Anonymous class expression must be reference types."); + + // 2. Check that there are no null methods or statements. + String contextInfo = + String.format("anonymous class of type %s", anonymousClassExpr.type().reference().name()); + NodeValidator.checkNoNullElements(anonymousClassExpr.methods(), "methods", contextInfo); + NodeValidator.checkNoNullElements(anonymousClassExpr.statements(), "statements", contextInfo); + for (MethodDefinition method : anonymousClassExpr.methods()) { - // 2. Static methods are not allowed in anonymous class. + // 3. Static methods are not allowed in anonymous class. Preconditions.checkState(!method.isStatic(), "Anonymous class cannot have static methods."); - // 3. Anonymous class cannot have explicit constructors. + // 4. Anonymous class cannot have explicit constructors. Preconditions.checkState( !method.isConstructor(), "Anonymous class cannot have explicit constructors."); } - // 4. Static variable expression is not allowed unless it is final. + // 5. Static variable expression is not allowed unless it is final. for (Statement statement : anonymousClassExpr.statements()) { if (statement instanceof ExprStatement) { Expr expr = ((ExprStatement) statement).expression(); From 004d0f0711f7762d254e3f9a1e37edb5154af564 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:19:50 -0800 Subject: [PATCH 06/17] fix: validate non-null elements for BlockStatement --- .../google/api/generator/engine/ast/BlockStatement.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/BlockStatement.java b/src/main/java/com/google/api/generator/engine/ast/BlockStatement.java index 7ee8efa794..b5ff9373fd 100644 --- a/src/main/java/com/google/api/generator/engine/ast/BlockStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/BlockStatement.java @@ -40,6 +40,12 @@ public abstract static class Builder { public abstract Builder setIsStatic(boolean isStatic); - public abstract BlockStatement build(); + abstract BlockStatement autoBuild(); + + public BlockStatement build() { + BlockStatement blockStatement = autoBuild(); + NodeValidator.checkNoNullElements(blockStatement.body(), "body", "block statement"); + return autoBuild(); + } } } From 191e93cd91a9ee0c00384a035a3678a7fec7f07c Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:20:36 -0800 Subject: [PATCH 07/17] fix: validate non-null elements for ClassDefinition --- .../api/generator/engine/ast/ClassDefinition.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java b/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java index 76c797c7d3..ec8f4f7b43 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java +++ b/src/main/java/com/google/api/generator/engine/ast/ClassDefinition.java @@ -137,6 +137,7 @@ public ClassDefinition build() { setClassIdentifier(classIdentifier); ClassDefinition classDef = autoBuild(); + performNullChecks(classDef); // Only nested classes can forego having a package. if (!classDef.isNested()) { @@ -201,5 +202,19 @@ public ClassDefinition build() { return classDef; } + + void performNullChecks(ClassDefinition classDef) { + String contextInfo = String.format("class definition of %s", name()); + NodeValidator.checkNoNullElements( + classDef.headerCommentStatements(), "header comments", contextInfo); + NodeValidator.checkNoNullElements(classDef.annotations(), "annotations", contextInfo); + + NodeValidator.checkNoNullElements( + classDef.implementsTypes(), "implemented types", contextInfo); + NodeValidator.checkNoNullElements(classDef.statements(), "statements", contextInfo); + + NodeValidator.checkNoNullElements(classDef.methods(), "methods", contextInfo); + NodeValidator.checkNoNullElements(classDef.nestedClasses(), "nested classes", contextInfo); + } } } From fdb7160bb2f164983f262e75f348e819f53c6718 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:22:04 -0800 Subject: [PATCH 08/17] fix: validate non-null elements for ConcreteReference --- .../google/api/generator/engine/ast/ConcreteReference.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java index bb92b2bebf..64f34d1927 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/ConcreteReference.java @@ -231,9 +231,14 @@ public Builder setGenerics(Reference... references) { // Private. abstract Class clazz(); + abstract ImmutableList generics(); + abstract boolean isStaticImport(); public ConcreteReference build() { + NodeValidator.checkNoNullElements( + generics(), "generics", String.format("concrete reference %s", clazz().getSimpleName())); + setIsStaticImport(clazz().getEnclosingClass() != null && isStaticImport()); return autoBuild(); } From 96eca4bbede6d4b972905bb7437cadf118658011 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:23:04 -0800 Subject: [PATCH 09/17] fix: validate non-null elements for ForStatement --- .../java/com/google/api/generator/engine/ast/ForStatement.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/ForStatement.java b/src/main/java/com/google/api/generator/engine/ast/ForStatement.java index 7c6222d411..b1216b6210 100644 --- a/src/main/java/com/google/api/generator/engine/ast/ForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/ForStatement.java @@ -48,6 +48,7 @@ public abstract static class Builder { public ForStatement build() { ForStatement forStatement = autoBuild(); + NodeValidator.checkNoNullElements(forStatement.body(), "body", "for-statement"); Preconditions.checkState( forStatement.localVariableExpr().isDecl(), "For-statement variable must be a declaration"); From 10924ee1f0336f97842fb41935e2017dbb68431f Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:24:06 -0800 Subject: [PATCH 10/17] fix: validate non-null elements for GeneralForStatement --- .../google/api/generator/engine/ast/GeneralForStatement.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java index a1a9cb3073..93a5db44cb 100644 --- a/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/GeneralForStatement.java @@ -79,6 +79,8 @@ abstract static class Builder { // Type-checking will be done in the sub-expressions. GeneralForStatement build() { GeneralForStatement generalForStatement = autoBuild(); + NodeValidator.checkNoNullElements( + generalForStatement.body(), "body", "general for-statement"); Expr initExpr = generalForStatement.initializationExpr(); if (initExpr instanceof AssignmentExpr) { VariableExpr localVarExpr = ((AssignmentExpr) initExpr).variableExpr(); From 8493707f18050bc1b93ce0f55f9c0ec8166f5b84 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:24:50 -0800 Subject: [PATCH 11/17] fix: validate non-null elements for MethodDefinition --- .../generator/engine/ast/MethodDefinition.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/MethodDefinition.java b/src/main/java/com/google/api/generator/engine/ast/MethodDefinition.java index 687e752f3f..cc6a0a1b85 100644 --- a/src/main/java/com/google/api/generator/engine/ast/MethodDefinition.java +++ b/src/main/java/com/google/api/generator/engine/ast/MethodDefinition.java @@ -167,11 +167,14 @@ public Builder setReturnExpr(Expr expr) { abstract Builder setMethodIdentifier(IdentifierNode methodIdentifier); // Private accessors. - abstract String name(); + abstract ImmutableList headerCommentStatements(); + abstract ImmutableList annotations(); + abstract ImmutableList throwsExceptions(); + abstract TypeNode returnType(); abstract boolean isOverride(); @@ -195,6 +198,8 @@ public Builder setReturnExpr(Expr expr) { abstract ImmutableList returnTemplateNames(); public MethodDefinition build() { + performNullChecks(); + // Handle templates. setTemplateIdentifiers( templateNames().stream() @@ -329,5 +334,16 @@ public MethodDefinition build() { return method; } + + void performNullChecks() { + String contextInfo = String.format("method definition of %s", name()); + NodeValidator.checkNoNullElements(headerCommentStatements(), "header comments", contextInfo); + NodeValidator.checkNoNullElements(annotations(), "annotations", contextInfo); + NodeValidator.checkNoNullElements(throwsExceptions(), "declared exceptions", contextInfo); + NodeValidator.checkNoNullElements(body(), "body", contextInfo); + NodeValidator.checkNoNullElements(templateNames(), "template names", contextInfo); + NodeValidator.checkNoNullElements( + returnTemplateNames(), "return template names", contextInfo); + } } } From a5a572c2c4212c4987d8badd277df469d044ffd9 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:31:23 -0800 Subject: [PATCH 12/17] fix: validate non-null elements for PackageInfoDefinition --- .../generator/engine/ast/PackageInfoDefinition.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java b/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java index 4930701318..5d009fe0fe 100644 --- a/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java +++ b/src/main/java/com/google/api/generator/engine/ast/PackageInfoDefinition.java @@ -67,6 +67,16 @@ public Builder setAnnotations(AnnotationNode... annotations) { public abstract Builder setAnnotations(List annotations); - public abstract PackageInfoDefinition build(); + abstract PackageInfoDefinition autoBuild(); + + public PackageInfoDefinition build() { + PackageInfoDefinition packageInfo = autoBuild(); + String contextInfo = String.format("package info for %s", packageInfo.pakkage()); + NodeValidator.checkNoNullElements(packageInfo.fileHeader(), "file header", contextInfo); + NodeValidator.checkNoNullElements( + packageInfo.headerCommentStatements(), "header comments", contextInfo); + NodeValidator.checkNoNullElements(packageInfo.annotations(), "annotations", contextInfo); + return packageInfo; + } } } From e366777489cb41f92229e9f7cadcb798034dc692 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:53:07 -0800 Subject: [PATCH 13/17] fix: validate non-null elements for SynchronizedStatement --- .../generator/engine/ast/SynchronizedStatement.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/api/generator/engine/ast/SynchronizedStatement.java b/src/main/java/com/google/api/generator/engine/ast/SynchronizedStatement.java index 41a94ef351..5bd58d8d13 100644 --- a/src/main/java/com/google/api/generator/engine/ast/SynchronizedStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/SynchronizedStatement.java @@ -60,6 +60,14 @@ public Builder setBody(Statement... statements) { // Private. abstract Builder setLock(Expr expr); - public abstract SynchronizedStatement build(); + abstract ImmutableList body(); + + abstract SynchronizedStatement autoBuild(); + + public SynchronizedStatement build() { + NodeValidator.checkNoNullElements(body(), "body statements", "synchronized statement"); + + return autoBuild(); + } } } From ef10ebaa85f5649e4d01f6fa14be4c5e0fee5c17 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 18:54:46 -0800 Subject: [PATCH 14/17] fix: validate non-null elements for TryCatchStatement --- .../com/google/api/generator/engine/ast/TryCatchStatement.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/TryCatchStatement.java b/src/main/java/com/google/api/generator/engine/ast/TryCatchStatement.java index f1e063479f..3634374259 100644 --- a/src/main/java/com/google/api/generator/engine/ast/TryCatchStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/TryCatchStatement.java @@ -64,6 +64,9 @@ public abstract static class Builder { public TryCatchStatement build() { TryCatchStatement tryCatchStatement = autoBuild(); + NodeValidator.checkNoNullElements(tryCatchStatement.tryBody(), "try body", "try-catch"); + NodeValidator.checkNoNullElements(tryCatchStatement.catchBody(), "catch body", "try-catch"); + if (!tryCatchStatement.isSampleCode()) { Preconditions.checkNotNull( tryCatchStatement.catchVariableExpr(), From 3530735d1f3a4f45f81684bba749488c2c3f4905 Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 19:01:15 -0800 Subject: [PATCH 15/17] fix: validate non-null elements for VaporReference --- .../com/google/api/generator/engine/ast/VaporReference.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java index a2f0176c36..0461fabab4 100644 --- a/src/main/java/com/google/api/generator/engine/ast/VaporReference.java +++ b/src/main/java/com/google/api/generator/engine/ast/VaporReference.java @@ -178,6 +178,10 @@ public Builder setEnclosingClassNames(String... enclosingClassNames) { abstract VaporReference autoBuild(); public VaporReference build() { + String contextInfo = String.format("vapor reference %s", name()); + NodeValidator.checkNoNullElements(generics(), "generics", contextInfo); + NodeValidator.checkNoNullElements(enclosingClassNames(), "enclosingClassNames", contextInfo); + // Validate the name. IdentifierNode.builder().setName(name()).build(); // No exception thrown, so we can proceed. From 1e7beea91617ea046b8177d95d7a63fec993e6ea Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 19:03:07 -0800 Subject: [PATCH 16/17] fix: validate non-null elements for VariableExpr --- .../com/google/api/generator/engine/ast/VariableExpr.java | 6 ++++++ 1 file changed, 6 insertions(+) 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 51f2e8a52d..404afd516f 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 @@ -108,11 +108,17 @@ public abstract static class Builder { // Private. abstract Builder setTemplateNodes(List nodes); + abstract Variable variable(); + abstract ImmutableList templateObjects(); abstract VariableExpr autoBuild(); public VariableExpr build() { + NodeValidator.checkNoNullElements( + templateObjects(), + "template objects", + String.format("variable expr %s", variable().identifier().name())); setTemplateNodes( templateObjects().stream() .map( From b901a46fc9f327e76f7abeb5773d5f21280383da Mon Sep 17 00:00:00 2001 From: Mira Leung Date: Fri, 6 Nov 2020 19:07:00 -0800 Subject: [PATCH 17/17] fix: validate non-null elements for WhileStatement --- .../com/google/api/generator/engine/ast/WhileStatement.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/main/java/com/google/api/generator/engine/ast/WhileStatement.java b/src/main/java/com/google/api/generator/engine/ast/WhileStatement.java index 2e4dbfe811..b0807ee247 100644 --- a/src/main/java/com/google/api/generator/engine/ast/WhileStatement.java +++ b/src/main/java/com/google/api/generator/engine/ast/WhileStatement.java @@ -41,10 +41,14 @@ public abstract static class Builder { public abstract Builder setBody(List body); + // Private. + abstract ImmutableList body(); + abstract WhileStatement autoBuild(); public WhileStatement build() { WhileStatement whileStatement = autoBuild(); + NodeValidator.checkNoNullElements(body(), "body", "while statement"); Preconditions.checkState( whileStatement.conditionExpr().type().equals(TypeNode.BOOLEAN), "While condition must be a boolean-typed expression");