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(smithy): Union variant naming #3415

Merged
merged 2 commits into from
Jul 13, 2023
Merged

Conversation

dnys1
Copy link
Contributor

@dnys1 dnys1 commented Jul 13, 2023

This was discovered as a bug when developing the config object. It's very easy to accidentally creating conflicting types due to how we name union variants. Consider the following Smithy model:

union AWSCategory {
  auth: AWSCategoryAuth
}

structure AWSCategoryAuth {}

This generates the following Dart code:

aws_category.dart (the AWSCategory union)

sealed class AWSCategory {
  const factory AWSCategory.auth() = AWSCategoryAuth;
}

final class AWSCategoryAuth {
  const AWSCategoryAuth();
}

aws_category_auth.dart (the AWSCategoryAuth structure)

abstract class AWSCategoryAuth {}

Two types with name AWSCategoryAuth are available in the global namespace which conflict. This is because the heuristic for union variant classes is '{unionName}{variantName}'.pascalCase. This is guaranteed to be unique amongst union variants but will conflict with a struct named the same.

Since this is a Dart-only limitation, it would not be appropriate to throw an error or not allow this. The $ symbol is frequently used to indicate generated code, so this is used to suffix all variant names so that they 1) never conflict with other symbols; and 2) make clear that these classes are Dart abstractions (they do not directly represent anything in the Smithy schema).

So the union file becomes:

sealed class AWSCategory {
  const factory AWSCategory.auth() = AWSCategoryAuth$;
}

final class AWSCategoryAuth$ {
  const AWSCategoryAuth();
}

@dnys1 dnys1 requested a review from a team as a code owner July 13, 2023 17:44
@dnys1 dnys1 force-pushed the fix/smithy/union-variant-naming branch from 5a6bf3f to 4cafd0e Compare July 13, 2023 17:45
@dnys1 dnys1 changed the title fix(smithy)!: Union variant naming fix(smithy): Union variant naming Jul 13, 2023
@dnys1 dnys1 merged commit 3058dae into main Jul 13, 2023
@dnys1 dnys1 deleted the fix/smithy/union-variant-naming branch July 13, 2023 19:24
dnys1 added a commit that referenced this pull request Jul 27, 2023
* fix(smithy)!: Union variant naming

* chore(repo): Regenerate SDK
dnys1 pushed a commit that referenced this pull request Jul 27, 2023
### Features
- feat(aft): Generate AWS config ([#3424](#3424))
- feat(auth): Enable ASF
- feat(authenticator): default dial code ([#3354](#3354))
- feat(datastore): Adds DataStoreHubEventType to DataStoreHubEvents ([#3454](#3454))
- feat(logging): logger can register one plugin per type in a logger hierarchy ([#3173](#3173))
- feat(storage): example app ([#3359](#3359))

### Fixes
- fix(codegen): Mark Cognito map as sparse ([#3386](#3386))
- fix(smithy): Union variant naming ([#3415](#3415))

### Chores
- chore(deps): Bump Amplify Android to 2.11.0

Updated-Components: amplify_lints, Amplify Flutter, Amplify Dart, Amplify UI, DB Common, Secure Storage, AWS Common, Smithy, Worker Bee
dnys1 pushed a commit that referenced this pull request Jul 27, 2023
### Features
- feat(aft): Generate AWS config ([#3424](#3424))
- feat(auth): Enable ASF
- feat(authenticator): default dial code ([#3354](#3354))
- feat(datastore): Adds DataStoreHubEventType to DataStoreHubEvents ([#3454](#3454))
- feat(logging): logger can register one plugin per type in a logger hierarchy ([#3173](#3173))
- feat(storage): example app ([#3359](#3359))

### Fixes
- fix(codegen): Mark Cognito map as sparse ([#3386](#3386))
- fix(smithy): Union variant naming ([#3415](#3415))

### Chores
- chore(deps): Bump Amplify Android to 2.11.0

Updated-Components: amplify_lints, Amplify Flutter, Amplify Dart, Amplify UI, DB Common, Secure Storage, AWS Common, Smithy, Worker Bee
dnys1 added a commit that referenced this pull request Jul 28, 2023
* fix(smithy)!: Union variant naming

* chore(repo): Regenerate SDK
dnys1 pushed a commit that referenced this pull request Jul 28, 2023
### Features
- feat(aft): Generate AWS config ([#3424](#3424))
- feat(auth): Enable ASF
- feat(authenticator): default dial code ([#3354](#3354))
- feat(datastore): Adds DataStoreHubEventType to DataStoreHubEvents ([#3454](#3454))
- feat(logging): logger can register one plugin per type in a logger hierarchy ([#3173](#3173))
- feat(storage): example app ([#3359](#3359))

### Fixes
- fix(codegen): Mark Cognito map as sparse ([#3386](#3386))
- fix(smithy): Union variant naming ([#3415](#3415))

### Chores
- chore(deps): Bump Amplify Android to 2.11.0

Updated-Components: amplify_lints, Amplify Flutter, Amplify Dart, Amplify UI, DB Common, Secure Storage, AWS Common, Smithy, Worker Bee
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