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

Call Unalias() in IsNilable() to support gotypesalias=1 #3304

Merged
merged 3 commits into from
Sep 27, 2024

Conversation

noamcohen97
Copy link
Contributor

Fixed false IsNilable() value for any (or any other aliased type) when gotypesalias=1 (default since Go 1.23.0)
Fixes #3301

I have:

  • Added tests covering the bug / feature (see testing)
  • Updated any relevant documentation (see docs)

Co-authored-by: Gilad Maymon <gilad.maymon@wiz.io>
@coveralls
Copy link

coveralls commented Sep 26, 2024

Coverage Status

coverage: 58.882% (+0.05%) from 58.829%
when pulling fbeb739 on noamcohen97:fix_isnilable
into 3a445f0 on 99designs:master.

@@ -520,6 +520,7 @@ func (b *Binder) CopyModifiersFromAst(t *ast.Type, base types.Type) types.Type {
}

func IsNilable(t types.Type) bool {
t = code.Unalias(t)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe we should call types.Unalias(t) so we'll Unalias even in Go 1.22?
Currently in Go 1.22 calling this with a nilable alias type returns false.
@StevenACoffman, @a8m WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We really need to continue to support Go 1.22, and I don't think we can just assume people have gotypesalias=1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So I'll call types.Unalias() here, that way we'll make sure IsNilable() is reliable no matter what.
Also - we don't need the pointer base type unaliasing from code.Unalias() here.

@StevenACoffman StevenACoffman merged commit 4cdeaa2 into 99designs:master Sep 27, 2024
16 of 17 checks passed
@StevenACoffman
Copy link
Collaborator

Thanks! I really appreciate your fix here! Looking forward to your next contribution!

@noamcohen97 noamcohen97 deleted the fix_isnilable branch September 27, 2024 18:49
@noamcohen97
Copy link
Contributor Author

Thanks @StevenACoffman!
When can we expect a new release?

@StevenACoffman
Copy link
Collaborator

Right now! Just released!

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.

Bug found when generating schema including arrays of type Any
3 participants