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

fix(ast): add import generation for annotations on VariableExpr #1076

Merged
merged 3 commits into from
Nov 3, 2022

Conversation

emmileaf
Copy link
Contributor

This PR is a follow-up patch to #1012 which added support for annotations on VariableExpr. It updates ImportWriterVisitor so that import generation also covers these annotations.

Note: the Spring CodeGen branch (autoconfig-gen-draft2) will need a rebase after this is merged, and have its golden test files updated to reflect this change.

@emmileaf emmileaf changed the title fix: add import generation for annotations on VariableExpr fix(ast): add import generation for annotations on VariableExpr Oct 31, 2022
@emmileaf emmileaf marked this pull request as ready for review October 31, 2022 19:20
@emmileaf emmileaf requested review from a team as code owners October 31, 2022 19:20
@emmileaf emmileaf requested review from zhumin8 and blakeli0 October 31, 2022 19:21
AnnotationNode annotation =
AnnotationNode.builder()
.setType(fakeAnnotationType)
.setDescription(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have to set descriptions? If not, removing it makes the test more concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah true, not for the functionality this test intends to cover - replaced with Min's suggestion below!

Copy link
Contributor

@zhumin8 zhumin8 left a comment

Choose a reason for hiding this comment

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

One line fixes are the best!

VariableExpr.builder()
.setVariable(variable)
.setIsDecl(true)
.setAnnotations(Arrays.asList(annotation))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: extremely small point, might save just a few lines using the concrete Generated as test.

            .setAnnotations(Arrays.asList(
                AnnotationNode.withType(
                    TypeNode.withReference(ConcreteReference.withClazz(Generated.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.

Ohh I hadn't realized this - thank you for the suggestion and updated!

@sonarcloud
Copy link

sonarcloud bot commented Nov 1, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@emmileaf emmileaf requested review from blakeli0 and zhumin8 November 1, 2022 15:29
@emmileaf emmileaf merged commit f5d5524 into main Nov 3, 2022
@emmileaf emmileaf deleted the annotation-args-fix branch November 3, 2022 13:44
emmileaf added a commit that referenced this pull request Nov 3, 2022
* This PR is a follow-up patch to #1012 which added support for annotations on VariableExpr. It updates ImportWriterVisitor so that import generation also covers these annotations.
emmileaf added a commit that referenced this pull request Nov 3, 2022
emmileaf added a commit that referenced this pull request Nov 3, 2022
emmileaf added a commit that referenced this pull request Nov 3, 2022
* Revert "test: update golden files after annotations import fix"

This reverts commit 0bbfc59.

* Revert "fix(ast): add import generation for annotations on VariableExpr (#1076)"

This reverts commit 9e4721f.

* Revert "chore(main): release 2.10.3 (#1079)"

This reverts commit 63b9bce.

* Revert "fix: fixes regionTag breakage (#1068)"

This reverts commit cd35be8.

* Revert "fix: fix REST transport client creation generated javadoc sample (#1077)"

This reverts commit 5346eef.

* Revert "refactor: expose parsed api short name and version as fields in Service (#1075)"

This reverts commit 2ebe948.

* Revert "chore(deps): update dependency org.apache.maven.plugins:maven-shade-plugin to v3.4.1 (#1072)"

This reverts commit 6774245.

* Revert "chore(main): release 2.10.2 (#1067)"

This reverts commit e8ee82f.

* Revert "fix(deps): update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.5 (#1063)"

This reverts commit aa1782d.

* Revert "fix: update regionTag to use service name (#1047)"

This reverts commit 38010fd.

* Revert "chore(main): release 2.10.1 (#1036)"

This reverts commit 3282e0f.

* Revert "deps: Upgrade protobuf to 3.21.7 (#1048)"

This reverts commit c768691.

* Revert "fix: update sample region tag to parse host instead of proto package (#1040)"

This reverts commit eb94f30.

* Revert "chore(deps): update dependency com.google.auto.value:auto-value to v1.10 (#1058)"

This reverts commit bd5599f.

* Revert "fix: Get numeric value for Enum fields if it is configured as query param or path param (#1042)"

This reverts commit 4b5eadb.

* Revert "fix(deps): update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.4 (#1050)"

This reverts commit e58bf9d.

* Revert "fix(deps): update dependency org.yaml:snakeyaml to v1.33 (#1043)"

This reverts commit d615305.

* Revert "chore(deps): update dependency org.apache.maven.plugins:maven-shade-plugin to v3.4.0 (#1038)"

This reverts commit ce93705.

* Revert "fix(deps): update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.3 (#1039)"

This reverts commit dba69f4.

* Revert "fix(deps): update dependency org.yaml:snakeyaml to v1.32 (#1037)"

This reverts commit e754a7c.
suztomo pushed a commit that referenced this pull request Dec 16, 2022
* This PR is a follow-up patch to #1012 which added support for annotations on VariableExpr. It updates ImportWriterVisitor so that import generation also covers these annotations.
suztomo pushed a commit that referenced this pull request Mar 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants