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

basetypes: fix equality for values with nil elementType #961

Merged
merged 4 commits into from
Apr 17, 2024
Merged

basetypes: fix equality for values with nil elementType #961

merged 4 commits into from
Apr 17, 2024

Conversation

jar-b
Copy link
Member

@jar-b jar-b commented Mar 18, 2024

A MapValue, ListValue, or SetValue with a nil elementType is an invalid state, and therefore cannot verify equality with another value. These changes to the Equal method now returns false if either the receiver or argument have a nil elementType.

Closes #959

@jar-b jar-b requested a review from a team as a code owner March 18, 2024 18:48
@jar-b jar-b changed the title basetypes: fix equality for MapValue with nil elementType basetypes: fix equality for values with nil elementType Mar 18, 2024
Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

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

LGTM

Perhaps a bug fix change log entry is warranted?

@bflad bflad added the bug Something isn't working label Mar 19, 2024
@bflad bflad added this to the v1.7.0 milestone Mar 19, 2024
@jar-b
Copy link
Member Author

jar-b commented Mar 19, 2024

Thanks @bendbennett - added separate changelog entries for each type. Let me know if you'd prefer a single entry instead!

@bflad
Copy link
Contributor

bflad commented Apr 16, 2024

@bendbennett / @jar-b anything else need to happen here? Implementation change looks good to me 😄

Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

jar-b added 4 commits April 16, 2024 09:13
A `MapValue` with a nil `elementType` is an invalid state, and therefore cannot verify equality with another value. This change to the `MapValue.Equal` method now returns `false` if either the receiver or argument have a nil `elementType`.
@jar-b
Copy link
Member Author

jar-b commented Apr 16, 2024

Apologies, I missed that this needed rebased. Should be ready now!

Copy link
Member

@austinvalle austinvalle left a comment

Choose a reason for hiding this comment

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

Thanks for this @jar-b! 🚀

@austinvalle austinvalle merged commit de32b2c into hashicorp:main Apr 17, 2024
6 checks passed
@jar-b jar-b deleted the b-nil-map-equality branch April 17, 2024 17:06
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zero value MapValue causes panic in Equal method
4 participants