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

[codegen/go] Honor import aliases for external types/resources #8833

Merged
merged 5 commits into from
Feb 3, 2022

Conversation

justinvp
Copy link
Member

@justinvp justinvp commented Jan 26, 2022

Problem Summary

Referencing external types/resources from Google Native doesn't work correctly in Go. For example, referencing the AuditConfig type leads to:

image (3)

Note the import at the top is iam but the usage is incorrectly iamv1. Attempting to workaround with a local import alias doesn't help.

This is happening because Google Native defines its own package import aliases, but they aren't taken into account fully when emitting references to the external types/resources. And any locally defined import aliases are also not taken into account.

Fix (this change)

This change fixes the Go SDK codegen to honor package import aliases when referencing external types/resources. Local aliases are preferred, otherwise, if the external package has an import alias, it is used.

@justinvp justinvp requested a review from a team January 26, 2022 00:47
@github-actions
Copy link

Diff for pulumi-azuread with merge commit dad9a2a

@github-actions
Copy link

Diff for pulumi-random with merge commit dad9a2a

@github-actions
Copy link

Diff for pulumi-random with merge commit d87c3db

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit dad9a2a

@github-actions
Copy link

Diff for pulumi-azuread with merge commit d87c3db

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit d87c3db

@github-actions
Copy link

Diff for pulumi-gcp with merge commit dad9a2a

@github-actions
Copy link

Diff for pulumi-gcp with merge commit d87c3db

@github-actions
Copy link

Diff for pulumi-azure with merge commit dad9a2a

@github-actions
Copy link

Diff for pulumi-azure with merge commit d87c3db

@github-actions
Copy link

Diff for pulumi-aws with merge commit d87c3db

@github-actions
Copy link

Diff for pulumi-aws with merge commit dad9a2a

@codecov
Copy link

codecov bot commented Jan 26, 2022

Codecov Report

Merging #8833 (468da42) into master (772953d) will increase coverage by 0.00%.
The diff coverage is 98.41%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #8833   +/-   ##
=======================================
  Coverage   59.35%   59.36%           
=======================================
  Files         642      642           
  Lines       99555    99550    -5     
  Branches     1389     1389           
=======================================
+ Hits        59093    59094    +1     
+ Misses      37090    37077   -13     
- Partials     3372     3379    +7     
Impacted Files Coverage Δ
pkg/codegen/internal/test/sdk_driver.go 78.35% <ø> (ø)
pkg/codegen/go/gen.go 89.93% <98.38%> (+0.22%) ⬆️
pkg/codegen/go/gen_program_expressions.go 77.31% <100.00%> (ø)
sdk/go/common/util/fsutil/walkup.go 81.25% <0.00%> (-9.38%) ⬇️
sdk/go/common/util/cmdutil/spinner.go 21.27% <0.00%> (-4.26%) ⬇️
sdk/go/common/workspace/paths.go 69.79% <0.00%> (-3.13%) ⬇️
sdk/go/common/workspace/workspace.go 64.00% <0.00%> (-1.60%) ⬇️
sdk/go/common/resource/plugin/plugin.go 66.51% <0.00%> (-1.36%) ⬇️
sdk/go/common/resource/plugin/host.go 50.80% <0.00%> (-1.21%) ⬇️
sdk/nodejs/automation/localWorkspace.ts 74.03% <0.00%> (-0.39%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 772953d...468da42. Read the comment docs.

@justinvp
Copy link
Member Author

Oops, I forgot to add the new generated test file for the other languages. Will add in the morning.

@github-actions
Copy link

Diff for pulumi-azuread with merge commit 55527c2

@github-actions
Copy link

Diff for pulumi-random with merge commit 55527c2

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit 55527c2

@github-actions
Copy link

Diff for pulumi-gcp with merge commit 55527c2

@github-actions
Copy link

Diff for pulumi-azure with merge commit 55527c2

@github-actions
Copy link

Diff for pulumi-aws with merge commit 55527c2

@github-actions
Copy link

Diff for pulumi-azure-native with merge commit 55527c2

@github-actions
Copy link

Diff for pulumi-azuread with merge commit beffbf4

@github-actions
Copy link

Diff for pulumi-random with merge commit beffbf4

@github-actions
Copy link

Diff for pulumi-kubernetes with merge commit beffbf4

@github-actions
Copy link

Diff for pulumi-gcp with merge commit beffbf4

@github-actions
Copy link

Diff for pulumi-azure with merge commit beffbf4

@github-actions
Copy link

Diff for pulumi-aws with merge commit beffbf4

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Diff for pulumi-random with merge commit d7169e7

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Diff for pulumi-azuread with merge commit d7169e7

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Diff for pulumi-kubernetes with merge commit d7169e7

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Diff for pulumi-gcp with merge commit d7169e7

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Diff for pulumi-azure with merge commit d7169e7

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Diff for pulumi-aws with merge commit d7169e7

@github-actions
Copy link

github-actions bot commented Feb 2, 2022

Diff for pulumi-azure-native with merge commit d7169e7

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-random with merge commit 8add2eb

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azuread with merge commit 8add2eb

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-kubernetes with merge commit 8add2eb

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-gcp with merge commit 8add2eb

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azure with merge commit 8add2eb

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-aws with merge commit 8add2eb

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azure-native with merge commit 8add2eb

justinvp and others added 5 commits February 3, 2022 06:23
This change fixes the Go SDK codegen to honor package import aliases when referencing external types/resources. Local aliases are preferred, otherwise, if the external package has an import alias, it is used.

Co-authored-by: James Nugent <[email protected]>
Co-authored-by: Alex Suttmiller <[email protected]>
@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-random with merge commit b4af013

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azuread with merge commit b4af013

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-kubernetes with merge commit b4af013

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-gcp with merge commit b4af013

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azure with merge commit b4af013

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-aws with merge commit b4af013

@github-actions
Copy link

github-actions bot commented Feb 3, 2022

Diff for pulumi-azure-native with merge commit b4af013

@justinvp justinvp merged commit 6c179e0 into master Feb 3, 2022
@pulumi-bot pulumi-bot deleted the justin/aliases branch February 3, 2022 15:43
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.

4 participants