Skip to content

feat: add stringDefaultComparison for type-level default string comparer#948

Open
sirphilliptubell wants to merge 5 commits into
SteveDunn:mainfrom
sirphilliptubell:feature/string-comparer-default
Open

feat: add stringDefaultComparison for type-level default string comparer#948
sirphilliptubell wants to merge 5 commits into
SteveDunn:mainfrom
sirphilliptubell:feature/string-comparer-default

Conversation

@sirphilliptubell

Copy link
Copy Markdown

PR for feature request #947

Adds a new StringComparisonDefault enum and stringDefaultComparison parameter to ValueObjectAttribute, ValueObjectAttribute<T>, and VogenDefaultsAttribute. When set, the specified StringComparer iswired into all five equality/hashing entry points on string-backed value objects: Equals(T), Equals(string), operator == (both directions), and GetHashCode(). The Comparers nested class is generated implicitly when a default comparison is specified.

Two new diagnostics:

  • VOG039 (warning): stringDefaultComparison set on a non-string type
  • VOG040 (error): stringComparers: Omit explicitly conflicts with stringDefaultComparison

Also fixes a bug in the existing Comparers class where GetHashCode called _comparer.GetHashCode() (the comparer's own object hash) instead of _comparer.GetHashCode(obj._value), causing all values to hash identically and degrading dictionary lookups to O(n).

Fyi, this includes the Fix in #946 (but not the tests in it)

Adds a new `StringComparisonDefault` enum and `stringDefaultComparison` parameter to `ValueObjectAttribute`, `ValueObjectAttribute<T>`, and `VogenDefaultsAttribute`. When set, the specified `StringComparer` iswired into all five equality/hashing entry points on string-backed value objects: `Equals(T)`, `Equals(string)`, `operator ==` (both directions), and `GetHashCode()`. The `Comparers` nested class is generated implicitly when a default comparison is specified.

Two new diagnostics:
- VOG039 (warning): `stringDefaultComparison` set on a non-string type
- VOG040 (error): `stringComparers: Omit` explicitly conflicts with `stringDefaultComparison`

Also fixes a bug in the existing `Comparers` class where `GetHashCode` called `_comparer.GetHashCode()` (the comparer's own object hash) instead of `_comparer.GetHashCode(obj._value)`, causing all values to hash identically and degrading dictionary lookups to O(n).
… previous ones to maintain backwards compatibility
Previously, setting stringDefaultComparison (without stringComparers) caused the Comparers nested class to be generated as an unintended side-effect. The two settings are orthogonal: stringComparers controls whether the Comparers utility class is generated; stringDefaultComparison controls the VO's own Equals/GetHashCode/operator behavior.

The Comparers class now only generates when stringComparers = Generate is explicitly set. VOG040 (which blocked stringComparers = Omit alongside stringDefaultComparison) has been removed as its premise no longer holds.

Added a snapshot test covering both settings used together.
@sirphilliptubell

Copy link
Copy Markdown
Author

Did some cleanup, the existing Comparer generation is now separate from the string equality feature

Conversions.Unspecified = -1 fails the IsValidFlags() >= 0 check, causing VOG011 on the four string default comparison test types.
Thorough CI builds run all three frameworks; the OrdinalIgnoreCase and combined-settings tests were missing their v4.8 and v8.0 verified snapshots.
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.

2 participants