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

Fix HasValueX incorrectly returning true #676

Merged
merged 4 commits into from
Dec 17, 2023

Conversation

SmithPlatts
Copy link

Overview

As per issue #675, when a whitespace string is passed to the Values(params object[] items) or Values(IEnumerable<object?> items) constructors of any of the Values<> classes, the HasValueX property will return true, where the corresponding ValueX will be empty.

Primary changes

  • Added tests to handle the known failure scenario.
  • Moved this.HasValueX evaluation and assignment to after this.ValueX assignment.

Ancillary changes

  • Updated .editorconfig to redefine new Roslyn analysers, ensuring that newer warnings don't fail the build.
  • Added a helper method to Schema.NET.Test, AssertEx.Empty<T>(OneOrMany<T> collection), which addresses the instances where the compiler was implicitly using string for Assert.Empty(OneOrMany<string>), instead of treating the input as an enumerable.

Adam Smith-Platts added 4 commits December 13, 2023 17:36
…> collection is empty, where T is a string

- Added `AssertEx` class that exposes a `Empty()` method that takes a `OneOrMany<T>`.
- Updated all `Assert.Empty(OneOrMany<string>!)` instances to use `AssertEx.Empty(OneOrMany<string>)`.

Tests now pass!
…e to Values() constructors

Presently failing, next commit addresses bug.
…o ValueX, from object collection, and correctly storing the state of HasValueX
@github-actions github-actions bot added enhancement Issues describing an enhancement or pull requests adding an enhancement. maintenance Pull requests that perform maintenance on the project but add no features or bug fixes. labels Dec 14, 2023
@Turnerj
Copy link
Collaborator

Turnerj commented Dec 16, 2023

Hey @SmithPlatts - thanks for this! I'll try and review it soon as what you described in this and corresponding issue make sense.

@Turnerj Turnerj added the patch Pull requests requiring a patch version update according to semantic versioning. label Dec 17, 2023
Copy link
Collaborator

@Turnerj Turnerj left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@Turnerj Turnerj merged commit 7f99d27 into RehanSaeed:main Dec 17, 2023
14 checks passed
@SmithPlatts SmithPlatts deleted the fix-empty-values-cast-hasvaluex branch January 15, 2024 01:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Issues describing an enhancement or pull requests adding an enhancement. maintenance Pull requests that perform maintenance on the project but add no features or bug fixes. patch Pull requests requiring a patch version update according to semantic versioning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A null or whitespace string passed into Values<T1, T2*> causes HasValueX to return true
2 participants