Skip to content

Fix HasValueX incorrectly returning true#676

Merged
Turnerj merged 4 commits intoRehanSaeed:mainfrom
SmithPlatts:fix-empty-values-cast-hasvaluex
Dec 17, 2023
Merged

Fix HasValueX incorrectly returning true#676
Turnerj merged 4 commits intoRehanSaeed:mainfrom
SmithPlatts:fix-empty-values-cast-hasvaluex

Conversation

@SmithPlatts
Copy link
Copy Markdown

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
Copy Markdown
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
Copy Markdown
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
@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