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

node/bindnode: panic if a schema.Type is passed as a Go ptrType #382

Open
mvdan opened this issue Mar 8, 2022 · 0 comments
Open

node/bindnode: panic if a schema.Type is passed as a Go ptrType #382

mvdan opened this issue Mar 8, 2022 · 0 comments
Labels
good first issue Good issue for new contributors P3 Low: Not priority right now

Comments

@mvdan
Copy link
Contributor

mvdan commented Mar 8, 2022

The following code compiles, and will panic in a confusing way:

proto := bindnode.Prototype(schemaType, nil)

The right way to supply a schema type and infer the Go type is:

proto := bindnode.Prototype(nil, schemaType)

The Go type is passed as a ptrType interface{}, causing this footgun. Messing up the other way, e.g. proto := bindnode.Prototype(nil, (*MyType)(nil)) wouldn't compile, because the second parameter expects a schema.Type interface.

We can avoid this footgun by making the bindnode APIs panic if their "Go type" parameter implements the schema.Type interface. I don't see why anyone would ever implement it on purpose for a Go type used to back a bindnode node. The most likely explanation is that they got the order of the parameters wrong, like I just did.

@mvdan mvdan added the good first issue Good issue for new contributors label Mar 8, 2022
@BigLep BigLep added the P3 Low: Not priority right now label May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good issue for new contributors P3 Low: Not priority right now
Projects
None yet
Development

No branches or pull requests

2 participants