This repository was archived by the owner on May 14, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 74
feat(ast): extend support for annotation named parameters #1012
Merged
Merged
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
df731a5
change to AnnotationNode and JavaWriterVisitor to support multiple pa…
zhumin8 bc36ce5
add Value implementation class for Class type var.
zhumin8 4e1379c
adding tests.
zhumin8 55634f6
add test.
zhumin8 9f1b09c
clean up code smells.
zhumin8 b9c6ef5
ClazzValue implements ObjectValue instead of Value.
zhumin8 6a41bdd
restructure AnnotationNode methods for adding parameters and remove u…
zhumin8 72c34b1
test updates to address code smells.
zhumin8 4b61639
Add optional annotation node to VariableExpr.
zhumin8 dcee621
fix code smell: leave only invocation that throws exception inside la…
zhumin8 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
45 changes: 45 additions & 0 deletions
45
src/main/java/com/google/api/generator/engine/ast/ClazzValue.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /** | ||
| * Copyright 2022 Google LLC | ||
| * | ||
| * <p>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 | ||
| * | ||
| * <p>http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * <p>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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We already have a way to achieve the same thing, see the code here and the golden test file here. I think it's better not to have different ways to achieve the same thing, it would create more maintenance burden in the future. If you want, you could extract the common logic to a util method/class, something like
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, thanks for the tip. I couldn't wrap my head around needing a |
||
| @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(); | ||
| } | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
44 changes: 44 additions & 0 deletions
44
src/test/java/com/google/api/generator/engine/ast/ClazzValueTest.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this improvement in general, but we should be careful exposing a setter that can set descriptions to any
Expr, you can technically pass aNewObjectExpror aReturnExprand everything will work until we try to compile the generated code, which could be too late to find an issue.Correct me if I'm wrong, I think we only allow
ValueExpr,AssignmentExpr, orVariableExpr(for clazz if you removeCLazzValue). So we could either add some type checks in thebuild()method, or only expose methods to add descriptions for the above types, likeaddDescription(ValueExpr expr),addDescription(AssignmentExpr expr)andaddDescription(VariableExpr expr).Basically anything that could restrict people from passing any types of
Exprwould be great.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, let me refactor this part and expose methods for each expr type.