Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ggj] feat: support file header (apache license) for outer class definition #157

Merged
merged 30 commits into from
Aug 19, 2020
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
4d48254
add file header comment
xiaozhenliu-gg5 Aug 6, 2020
712a2b9
gapic unit tests pass
xiaozhenliu-gg5 Aug 6, 2020
aecb241
fix bazel rules
xiaozhenliu-gg5 Aug 6, 2020
35cf94c
merge master
xiaozhenliu-gg5 Aug 6, 2020
d8d9cb1
clean up
xiaozhenliu-gg5 Aug 6, 2020
4044ee4
format code
xiaozhenliu-gg5 Aug 6, 2020
85e87a4
file header class
xiaozhenliu-gg5 Aug 6, 2020
44c0cf5
simplify create header
xiaozhenliu-gg5 Aug 6, 2020
23813f8
format
xiaozhenliu-gg5 Aug 6, 2020
18f6267
new line after file header
xiaozhenliu-gg5 Aug 6, 2020
7b0eea2
merge
xiaozhenliu-gg5 Aug 7, 2020
c0d3be7
file header is optional for class
xiaozhenliu-gg5 Aug 7, 2020
88d71cf
add composer test
xiaozhenliu-gg5 Aug 7, 2020
45dbadb
Merge branch 'master' of github.com:googleapis/gapic-generator-java
xiaozhenliu-gg5 Aug 8, 2020
0f59ef8
multiple line comment
xiaozhenliu-gg5 Aug 8, 2020
2707ea1
Merge branch 'fix-block-comment' of github.com:googleapis/gapic-gener…
xiaozhenliu-gg5 Aug 8, 2020
0f34aec
multiple line comment for header
xiaozhenliu-gg5 Aug 8, 2020
6364167
rename apache license
xiaozhenliu-gg5 Aug 10, 2020
590c7c6
move file header addition to gapic class
xiaozhenliu-gg5 Aug 11, 2020
a356da9
format
xiaozhenliu-gg5 Aug 11, 2020
50c1fb1
convert change in Composer
xiaozhenliu-gg5 Aug 11, 2020
31c0468
add edge unit test
xiaozhenliu-gg5 Aug 12, 2020
7ef4b4a
move header addition to composer class
xiaozhenliu-gg5 Aug 12, 2020
ede98cb
Merge branch 'fix-block-comment' of github.com:googleapis/gapic-gener…
xiaozhenliu-gg5 Aug 12, 2020
d59e1a3
format
xiaozhenliu-gg5 Aug 12, 2020
7a19396
feedback
xiaozhenliu-gg5 Aug 14, 2020
7879471
merge master
xiaozhenliu-gg5 Aug 16, 2020
5eccd75
Merge branch 'master' of github.com:googleapis/gapic-generator-java i…
xiaozhenliu-gg5 Aug 17, 2020
43f24c0
feedback
xiaozhenliu-gg5 Aug 18, 2020
e051381
clean up
xiaozhenliu-gg5 Aug 19, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

@AutoValue
public abstract class ClassDefinition implements AstNode {
// Optional.
public abstract ImmutableList<CommentStatement> fileHeader();
// Required.
public abstract ScopeNode scope();
// Required.
Expand Down Expand Up @@ -66,6 +68,7 @@ public void accept(AstNodeVisitor visitor) {

public static Builder builder() {
return new AutoValue_ClassDefinition.Builder()
.setFileHeader(Collections.emptyList())
.setHeaderCommentStatements(Collections.emptyList())
.setIsNested(false)
.setIsFinal(false)
Expand All @@ -78,8 +81,12 @@ public static Builder builder() {
.setNestedClasses(Collections.emptyList());
}

public abstract Builder toBuilder();

@AutoValue.Builder
public abstract static class Builder {
public abstract Builder setFileHeader(List<CommentStatement> fileHeader);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to error-out in build() if this is a nested class?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, added one condition check in build to confirm nested classes do not have file headers.


public abstract Builder setHeaderCommentStatements(
List<CommentStatement> headerCommentStatements);

Expand Down Expand Up @@ -129,6 +136,9 @@ public ClassDefinition build() {
Preconditions.checkState(!classDef.isStatic(), "Outer classes cannot be static");
Preconditions.checkState(
!classDef.scope().equals(ScopeNode.PRIVATE), "Outer classes cannot be private");
} else {
Preconditions.checkState(
classDef.fileHeader().isEmpty(), "Nested classes cannot have file header");
xiaozhenliu-gg5 marked this conversation as resolved.
Show resolved Hide resolved
}

// Abstract classes cannot be marked final.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,8 @@ public void visit(MethodDefinition methodDefinition) {
@Override
public void visit(ClassDefinition classDefinition) {
if (!classDefinition.isNested()) {
statements(classDefinition.fileHeader().stream().collect(Collectors.toList()));
newline();
importWriterVisitor.initialize(
classDefinition.packageString(), classDefinition.classIdentifier().name());
buffer.append(String.format("package %s;", classDefinition.packageString()));
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/google/api/generator/gapic/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package(default_visibility = ["//visibility:public"])
filegroup(
name = "gapic_files",
srcs = glob(["*.java"]) + [
"//src/test/java/com/google/api/generator/gapic/composer:composer_files",
"//src/main/java/com/google/api/generator/gapic/composer:composer_files",
"//src/main/java/com/google/api/generator/gapic/model:model_files",
"//src/main/java/com/google/api/generator/gapic/protoparser:protoparser_files",
],
miraleung marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import com.google.api.generator.gapic.model.Message;
import com.google.api.generator.gapic.model.ResourceName;
import com.google.api.generator.gapic.model.Service;
import com.google.api.generator.gapic.utils.ApacheLicense;
import com.google.common.annotations.VisibleForTesting;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
Expand All @@ -36,7 +38,7 @@ public static List<GapicClass> composeServiceClasses(GapicContext context) {
clazzes.addAll(generateServiceClasses(service, context.messages()));
}
clazzes.addAll(generateResourceNameHelperClasses(context.helperResourceNames()));
return clazzes;
return addApacheLicense(clazzes);
}

public static List<GapicClass> generateServiceClasses(
Expand Down Expand Up @@ -106,4 +108,20 @@ private static GapicClass generateGenericClass(Kind kind, String name, Service s
.build();
return GapicClass.create(kind, classDef);
}

@VisibleForTesting
protected static List<GapicClass> addApacheLicense(List<GapicClass> gapicClassList) {
return gapicClassList.stream()
.map(
gapicClass -> {
ClassDefinition classWithHeader =
gapicClass
.classDefinition()
.toBuilder()
.setFileHeader(ApacheLicense.APACHE_LICENSE_COMMENT_STATEMENT)
.build();
return GapicClass.create(gapicClass.kind(), classWithHeader);
})
.collect(Collectors.toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,8 @@ private static Statement createMethodDescriptorVariableDecl(
return ExprStatement.withExpr(
AssignmentExpr.builder()
.setVariableExpr(
methodDescriptorVarExpr.toBuilder()
methodDescriptorVarExpr
.toBuilder()
.setIsDecl(true)
.setScope(ScopeNode.PRIVATE)
.setIsStatic(true)
Expand Down Expand Up @@ -524,7 +525,9 @@ private static List<MethodDefinition> createConstructorMethods(
secondCtorExprs.add(
AssignmentExpr.builder()
.setVariableExpr(
classMemberVarExprs.get("callableFactory").toBuilder()
classMemberVarExprs
.get("callableFactory")
.toBuilder()
.setExprReferenceExpr(thisExpr)
.build())
.setValueExpr(callableFactoryVarExpr)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,8 @@ private static MethodDefinition createSetResponsesMethod(Service service) {
Expr responseAssignExpr =
AssignmentExpr.builder()
.setVariableExpr(
responsesVarExpr.toBuilder()
responsesVarExpr
.toBuilder()
.setExprReferenceExpr(
ValueExpr.withValue(ThisObjectValue.withType(getThisClassType(service))))
.build())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,9 @@ private static List<Statement> createClassStatements(
// "projects/{project}/locations/{location}/autoscalingPolicies/{autoscaling_policy}");
for (int i = 0; i < patterns.size(); i++) {
VariableExpr varExpr =
templateFinalVarExprs.get(i).toBuilder()
templateFinalVarExprs
.get(i)
.toBuilder()
.setIsDecl(true)
.setScope(ScopeNode.PRIVATE)
.setIsStatic(true)
Expand All @@ -202,7 +204,9 @@ private static List<Statement> createClassStatements(
}

memberVars.add(
FIXED_CLASS_VARS.get("fieldValuesMap").toBuilder()
FIXED_CLASS_VARS
.get("fieldValuesMap")
.toBuilder()
.setIsDecl(true)
.setScope(ScopeNode.PRIVATE)
.setIsVolatile(true)
Expand Down Expand Up @@ -1244,7 +1248,9 @@ private static ClassDefinition createNestedBuilderClass(
.setStaticReferenceType(STATIC_TYPES.get("Objects"))
.setMethodName("equals")
.setArguments(
FIXED_CLASS_VARS.get("pathTemplate").toBuilder()
FIXED_CLASS_VARS
.get("pathTemplate")
.toBuilder()
.setExprReferenceExpr(outerClassVarExpr)
.build(),
templateFinalVarExpr)
Expand Down Expand Up @@ -1272,7 +1278,8 @@ private static ClassDefinition createNestedBuilderClass(
AssignmentExpr.builder()
.setVariableExpr(currClassTokenVarExpr)
.setValueExpr(
currClassTokenVarExpr.toBuilder()
currClassTokenVarExpr
.toBuilder()
.setExprReferenceExpr(outerClassVarExpr)
.build())
.build());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,8 @@ private static List<Statement> createClassStatements(

// Assign DEFAULT_SERVICE_SCOPES.
VariableExpr defaultServiceScopesDeclVarExpr =
DEFAULT_SERVICE_SCOPES_VAR_EXPR.toBuilder()
DEFAULT_SERVICE_SCOPES_VAR_EXPR
.toBuilder()
.setIsDecl(true)
.setScope(ScopeNode.PRIVATE)
.setIsStatic(true)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// 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.gapic.utils;

import com.google.api.generator.engine.ast.BlockComment;
import com.google.api.generator.engine.ast.CommentStatement;
import java.util.Arrays;
import java.util.List;

public class ApacheLicense {
private static final String fileHeadeString =
xiaozhenliu-gg5 marked this conversation as resolved.
Show resolved Hide resolved
"Copyright 2020 Google LLC\n\n"
+ "Licensed under the Apache License, Version 2.0 (the \"License\");\n"
+ "you may not use this file except in compliance with the License.\n"
+ "You may obtain a copy of the License at\n\n"
+ " https://www.apache.org/licenses/LICENSE-2.0\n\n"
+ "Unless required by applicable law or agreed to in writing, software\n"
+ "distributed under the License is distributed on an \"AS IS\" BASIS,\n"
+ "WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n"
+ "See the License for the specific language governing permissions and\n"
+ "limitations under the License.";

public static final List<CommentStatement> APACHE_LICENSE_COMMENT_STATEMENT =
Arrays.asList(CommentStatement.withComment(BlockComment.withComment(fileHeadeString)));
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ java_library(
":utils_files",
],
deps = [
"//src/main/java/com/google/api/generator/engine/ast",
"@com_google_guava_guava//jar",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
public class ClassDefinitionTest {

@Test
public void validClassDefinition_basicWithComments() {
public void validClassDefinition_basicWithCommentsAndHeader() {
LineComment lineComment = LineComment.withComment("AUTO-GENERATED DOCUMENTATION AND CLASS");
JavaDocComment javaDocComment =
JavaDocComment.builder()
Expand All @@ -32,6 +32,7 @@ public void validClassDefinition_basicWithComments() {
"This class is for advanced usage and reflects the underlying API directly.")
.build();
ClassDefinition.builder()
.setFileHeader(createFileHeader())
.setHeaderCommentStatements(
Arrays.asList(
CommentStatement.withComment(lineComment),
Expand Down Expand Up @@ -118,6 +119,21 @@ public void validClassDefinition_statementsAndMethods() {
// No exception thrown, we're good.
}

@Test
public void invalidClassDefinition_nestedWithFileHeader() {
assertThrows(
IllegalStateException.class,
() -> {
ClassDefinition.builder()
.setPackageString("com.google.example.library.v1.stub")
.setName("LibraryServiceStub")
.setIsNested(true)
.setScope(ScopeNode.PUBLIC)
.setFileHeader(createFileHeader())
.build();
});
}

@Test
public void invalidClassDefinition_implementsNullType() {
assertThrows(
Expand Down Expand Up @@ -347,4 +363,8 @@ private static ForStatement createForStatement() {
.setBody(Arrays.asList(ExprStatement.withExpr(createAssignmentExpr())))
.build();
}
// Create a simple block comment to stand for the Apache License header.
private static List<CommentStatement> createFileHeader() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is just used once, do we need a helper method for it? Would prefer to just add the logic to the method where it's used.

Copy link
Contributor Author

@xiaozhenliu-gg5 xiaozhenliu-gg5 Aug 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated with adding a unit test (nestClassWithFileHeader) which also uses this. So I guess we can keep it. But this similar helper is only called once in JavaWriterVisitorTest, so removed it there. Thanks

return Arrays.asList(CommentStatement.withComment(BlockComment.withComment("Apache License")));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1610,9 +1610,12 @@ public void writeMethodDefinition_templatedReturnTypeAndArguments() {
}

@Test
public void writeClassDefinition_basic() {
public void writeClassDefinition_basicWithFileHeader() {
List<CommentStatement> fileHeader =
Arrays.asList(CommentStatement.withComment(BlockComment.withComment("Apache License")));
ClassDefinition classDef =
ClassDefinition.builder()
.setFileHeader(fileHeader)
.setPackageString("com.google.example.library.v1.stub")
.setName("LibraryServiceStub")
.setScope(ScopeNode.PUBLIC)
Expand All @@ -1622,7 +1625,10 @@ public void writeClassDefinition_basic() {
assertEquals(
writerVisitor.write(),
String.format(
createLines(3),
createLines(6),
"/*\n",
" * Apache License\n",
" */\n\n",
"package com.google.example.library.v1.stub;\n",
"\n",
"public class LibraryServiceStub {}\n"));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package(default_visibility = ["//visibility:public"])

TESTS = [
"ComposerTest",
"GrpcServiceCallableFactoryClassComposerTest",
"GrpcServiceStubClassComposerTest",
"MockServiceClassComposerTest",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
// 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.gapic.composer;

import static junit.framework.Assert.assertEquals;

import com.google.api.generator.engine.ast.ClassDefinition;
import com.google.api.generator.engine.ast.ScopeNode;
import com.google.api.generator.engine.writer.JavaWriterVisitor;
import com.google.api.generator.gapic.model.GapicClass;
import com.google.api.generator.gapic.model.GapicClass.Kind;
import java.util.Arrays;
import java.util.List;
import org.junit.Test;

public class ComposerTest {
@Test
public void gapicClass_addApacheLicense() {
ClassDefinition classDef =
ClassDefinition.builder()
.setPackageString("com.google.showcase.v1beta1.stub")
.setName("EchoStubSettings")
.setScope(ScopeNode.PUBLIC)
.build();
List<GapicClass> gapicClassWithHeaderList =
Composer.addApacheLicense(Arrays.asList(GapicClass.create(Kind.TEST, classDef)));
JavaWriterVisitor visitor = new JavaWriterVisitor();
gapicClassWithHeaderList.get(0).classDefinition().accept(visitor);
assertEquals(visitor.write(), EXPECTED_CLASS_STRING);
}

private static final String EXPECTED_CLASS_STRING =
"/*\n"
+ " * Copyright 2020 Google LLC\n"
+ " *\n"
+ " * Licensed under the Apache License, Version 2.0 (the \"License\");\n"
+ " * you may not use this file except in compliance with the License.\n"
+ " * You may obtain a copy of the License at\n"
+ " *\n"
+ " * https://www.apache.org/licenses/LICENSE-2.0\n"
+ " *\n"
+ " * Unless required by applicable law or agreed to in writing, software\n"
+ " * distributed under the License is distributed on an \"AS IS\" BASIS,\n"
+ " * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.\n"
+ " * See the License for the specific language governing permissions and\n"
+ " * limitations under the License.\n"
+ " */\n\n"
+ "package com.google.showcase.v1beta1.stub;\n\n"
+ "public class EchoStubSettings {}\n";
}