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.

Sign up for GitHub

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

Jump to bottom

go: no runtime indication that a required field in a struct is not supplied #2672

Closed
1 of 5 tasks
eladb opened this issue Mar 10, 2021 · 1 comment · Fixed by #3486
Closed
1 of 5 tasks

go: no runtime indication that a required field in a struct is not supplied #2672

eladb opened this issue Mar 10, 2021 · 1 comment · Fixed by #3486
Assignees
@RomainMuller
Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort language/go Regarding GoLang bindings p2

Comments

@eladb
Copy link
Contributor

eladb commented Mar 10, 2021

🐛 Bug Report

Affected Languages

  • GoLang
  • TypeScript or Javascript
  • Python
  • Java
  • .NET (C#, F#, ...)

General Information

  • JSII Version: 1.42.0
  • Platform: any

What is the problem?

Consider a jsii struct with a required field:

interface MyStruct {
  readonly required: string;
}

In go it is possible to initialize it without specifying a value for required:

my := MyStruct{}

It is impossible to tell which fields are required #2536, but at least we should get a panic if this object is passed to JS without the required fields set:

obj.CallMethod(my) // <-- should panic

See compliance test TestStructs_containsNullChecks.
@eladb eladb added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Mar 10, 2021
@eladb eladb changed the title go: no indication that a required field in a struct is not supplied go: no runtime indication that a required field in a struct is not supplied Mar 11, 2021
@MrArnoldPalmer MrArnoldPalmer removed their assignment Mar 30, 2021
@MrArnoldPalmer MrArnoldPalmer added language/go Regarding GoLang bindings effort/small Small work item – less than a day of effort p2 and removed needs-triage This issue or PR still needs to be triaged. labels Mar 30, 2021
@MrArnoldPalmer MrArnoldPalmer mentioned this issue Dec 21, 2021
Closed
5 tasks
@polothy polothy mentioned this issue Mar 22, 2022
Closed
7 tasks
@RomainMuller RomainMuller mentioned this issue Apr 11, 2022
Merged
RomainMuller added a commit that referenced this issue Apr 11, 2022
@RomainMuller
Add runtime check for missing required value.
cb2602e 
Fixes #2672
RomainMuller added a commit that referenced this issue Apr 11, 2022
@RomainMuller
Add runtime check for missing required value.
9533cb3 
Fixes #2672
@RomainMuller RomainMuller self-assigned this Apr 11, 2022
@RomainMuller RomainMuller moved this from Todo to In Review in CDK Go General Availability Apr 11, 2022
RomainMuller added a commit that referenced this issue Apr 25, 2022
@RomainMuller
feat(go): add tag to denote required/optional properties (#3486)
3d7c310 
Adds a `field:"required"` or `field:"optional"` tag on struct fields to
denote whether they are required or optional (obviously). This is a
cheap-ass way to provide documentation visibility to users.

This isn't perfect as ideally, one would want compile-time enforcement
of these, however this isn't currently posisble with Go (we explored
several options, including via the use of generics, but it did not end
up with any particularly improved solution).

This does however include runtime enforcement with a dedicated error
message.

Fixes #2536
Fixes #2672
Repository owner moved this from In Review to Done in CDK Go General Availability Apr 25, 2022
@github-actions
Copy link
Contributor

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@RomainMuller RomainMuller

Labels
bug This issue is a bug. effort/small Small work item – less than a day of effort language/go Regarding GoLang bindings p2
Projects
CDK Go General Availability
Status: Done
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(go): add tag to denote required/optional properties aws/jsii
3 participants
@RomainMuller @eladb @MrArnoldPalmer