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

Generate basic Go module with structures #2

Merged
merged 10 commits into from
Mar 18, 2020

Conversation

JordonPhillips
Copy link
Member

@JordonPhillips JordonPhillips commented Mar 9, 2020

This adds the ability to generate structure shapes as well as most primitive shapes. Primitives that require imports and specializations of shapes based on traits (e.g. errors / enums) are also not yet supported. Wholly unsupported shape types currently output nil symbols.

The scope of this initial PR was deliberately limited to try to keep its size down, as it already requires a decent amount of infrastructure to get things going.

Currently the following files are generated:

.
├── go.mod
└── api_types.go

The contents of go.mod will look like:

module weather

The base module name here is configurable.

Based on the smithy-go-codegen-test model, the contents of api_types.go will look like:

// Code generated by smithy-go-codegen DO NOT EDIT.
package weather

type CityCoordinates struct {
	Latitude  *float32
	Longitude *float32
}

type CitySummary struct {
	Case   *string
	CityId *string
	Name   *string
	Number *string
}

type GetCityImageInput struct {
	CityId *string
}

type GetCityImageOutput struct {
	Image []byte
}

type GetCityInput struct {
	CityId *string
}

type GetCityOutput struct {
	City        CitySummary
	Coordinates CityCoordinates
	Name        *string
}

type GetCurrentTimeOutput struct {
	Time nil
}

type GetForecastInput struct {
	CityId *string
}

type GetForecastOutput struct {
	ChanceOfRain *float32
}

type ListCitiesInput struct {
	NextToken *string
	PageSize  *int32
}

type ListCitiesOutput struct {
	Items     []CitySummary
	NextToken *string
}

(Note that go fmt is being run on the output)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@JordonPhillips JordonPhillips requested review from jasdel and kstich March 9, 2020 19:33
@jasdel
Copy link
Contributor

jasdel commented Mar 10, 2020

Minor suggestion for file/folder naming. I think the following pattern will help the SDK breakup go files into smaller chunks making it easier to view source since the files can be so large. e.g. v1 SDK's service/ec2/api.go file is over 100k lines and no longer can be rendered in github. The v2 SDK's equivalent file is about 10k lines.

api_client.go <- generated API client

api_op_Foo.go <- generated Operation method, operation request/response types, and
     operation Input type.  Generated pagination behavior could probably
     go here as well.

api_types.go <- all other struct types

api_enums.go <- all API enums

@jasdel
Copy link
Contributor

jasdel commented Mar 11, 2020

It would be helpful for all generated .go files to include the following:

// Code generated toolName DO NOT EDIT.

toolName can be anything, e.g. smithy-go-codegen. Reference, for where this comes from, golang/go#13560 (comment)

General practice is to put it at the top of the file as first line for easier human discovery, but doesn't have to be.

This adds in a bunch of the basic infrastructure necessary to perform
code generation. currently all that is generated is a basic `go.mod`
file.
This adds the ability to generate structure shapes as well as most
primitive shapes. Primitives that require imports and specializations
of shapes based on traits (e.g. errors / enums) are also not yet
supported. Wholly unsupported shape types currently output `nil`
symbols.
`go mod init` will fail if the `go.mod` already exists, so this
deletes it if it's present in the output. While it's technically
possible to simply edit the file, it's easier to just start fresh.
This removes the ability to infer which service should be generated
for in a model, instead relying on the setting being explicit.
@JordonPhillips JordonPhillips requested a review from kstich March 17, 2020 20:08
Comment on lines +41 to +48
public String toString() {
String contents = super.toString();
String[] packageParts = fullPackageName.split("/");
String header = String.format(
"// Code generated by smithy-go-codegen DO NOT EDIT.%npackage %s%n%n",
packageParts[packageParts.length - 1]);
// TODO: add imports
return header + contents;
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't have to do it ATM but wanted to call out, that it would be good to consider how we'll generate summary/overview documentation API packages.

The package doc string should only appear in one file within a package, (e.g. doc.go file) and provides the documentation/summary/overview of a package. We can determine if this should be a canned string, or incorporate documentation provided by the API model, (e.g v1 sdk's ACM)

// Code generated by smithy-go-codegen DO NOT EDIT.

// Package weather provides the client, and types for the Weather API. <- first paragraph is summary
//
// ... could be more that only appears in API docs overview, not included in package's summary.
package weather

Copy link
Member Author

Choose a reason for hiding this comment

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

Eventually we'll add a useFileWriter to GoDelegator, which we'll then use to write out such files. We should include the service shape's documentation there where available.

GoSettings settings = new GoSettings();
config.warnIfAdditionalProperties(Arrays.asList(SERVICE, MODULE_NAME, MODULE_DESCRIPTION, MODULE_VERSION));

settings.setService(config.expectStringMember(SERVICE).expectShapeId());
Copy link

Choose a reason for hiding this comment

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

Can you add a TODO for using inferService from Smithy's codebase instead of just fully removing it?

Copy link
Member Author

Choose a reason for hiding this comment

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

To be clear, the intent is to never infer the service.

Comment on lines +203 to +204
public Symbol stringShape(StringShape shape) {
// TODO: support specialized strings
Copy link
Contributor

Choose a reason for hiding this comment

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

assuming specialized strings are enums?

Copy link
Member Author

Choose a reason for hiding this comment

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

correct

Copy link
Member Author

Choose a reason for hiding this comment

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

and mediatype

@JordonPhillips JordonPhillips merged commit 55b5ac8 into aws:master Mar 18, 2020
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