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

crypto: use the named aliases crypto.PrivateKey and crypto.PublicKey as return types in sub-package APIs rather than bare interface{} #19328

Closed
andrewmbenton opened this issue Feb 28, 2017 · 4 comments

Comments

@andrewmbenton
Copy link
Contributor

@andrewmbenton andrewmbenton commented Feb 28, 2017

For clarity, it would be nice to have funcs like x509.ParsePKIXPublicKey(derBytes []byte) (pub interface{}, err error) and x509.ParsePKCS8PrivateKey(der []byte) (key interface{}, err error) return the named aliases crypto.PublicKey and crypto.PrivateKey rather than just interface{}.

There are other spots in the crypto packages that could probably benefit from this as well. I'm wondering if there's a reason not to do this?

I have a simple patch that updates these APIs with all tests passing that I'd be happy to submit.

@cespare
Copy link
Contributor

@cespare cespare commented Feb 28, 2017

Unfortunately, this is a backwards-incompatible change that's forbidden by the Go 1 compatibility promise.

@cespare cespare closed this Feb 28, 2017
@andrewmbenton
Copy link
Contributor Author

@andrewmbenton andrewmbenton commented Feb 28, 2017

I read that document before submitting which almost prevented me from suggesting this. But it isn't exactly clear to me.

Is it really breaking an API compatibility rule to replace an empty interface return type with an alias to an empty interface? I couldn't see a specific way that this change would break any existing Go programs. But maybe I just wasn't thinking hard enough?

@cespare
Copy link
Contributor

@cespare cespare commented Feb 28, 2017

For example this program won't compile after your change:

https://play.golang.org/p/zcDwTjQkPx

@andrewmbenton
Copy link
Contributor Author

@andrewmbenton andrewmbenton commented Feb 28, 2017

Good point. Thanks for the guidance. Something to keep in mind for a possible go2 API refactor.

@cespare cespare added the Go2 label Feb 28, 2017
@golang golang locked and limited conversation to collaborators Feb 28, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.