Skip to content

WeakString can only be set through Create [major] - #493

Open
matt-edmondson wants to merge 1 commit into
mainfrom
fix/310-weakstring-init
Open

matt-edmondson wants to merge 1 commit into
mainfrom
fix/310-weakstring-init

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #310

What changed

  • SemanticString<T>.WeakString is now { get; protected init; } instead of { get; init; }. Code outside the type hierarchy can no longer write uuid with { WeakString = "not a uuid" } or new Uuid { WeakString = "garbage" }, so the only way to get a value in is through Create, which canonicalizes and validates it.
  • ISemanticString.WeakString is now get-only.
  • The cached WeakStringSetter<TDest> delegate that Create uses (SemanticString.Create repeats type reflection on every call (~500 B and ~1.6 µs per value before any validator runs); cache per-type metadata #444) now looks up the non-public accessor (GetSetMethod(nonPublic: true)). A derived type can still set the value from its own constructor, which is what the existing SemanticStringExtensionsTests.TestSemanticString does.
  • Stale path caches: AbsoluteDirectoryPath, AbsoluteFilePath and RelativeDirectoryPath are sealed, and nothing inside them uses with { WeakString = ... }. With the accessor closed, a with copy always has the same value as its source, so the copied Parent and related caches stay correct. The types need no copy constructor.

Breaking change

Removing a public accessor is a breaking API change, as the issue and its triage note, so the commit carries [major]. A caller that set WeakString in an object initializer or a with expression must call Create / As<T>() instead. dotnet pack with package validation on Semantics.Strings passes with no new suppressions, because the change is the same across every target framework.

Tests

  • New WeakStringAccessTests:
    • WeakString has no public setter on MySemanticString, Uuid or the three path types;
    • ISemanticString.WeakString is read-only;
    • Create still sets the value for a Uuid, a plain semantic string and an AbsoluteDirectoryPath, including its Parent.
  • With the Semantics.Strings change reverted, the two accessor tests fail and the two Create tests pass. With the change they all pass.
  • dotnet build -c Release finished with 0 warnings and 0 errors. dotnet test -c Release had 0 failures (1993 passed, the rest skipped).

Generated by Claude Code

SemanticString.WeakString had a public init accessor, so
`uuid with { WeakString = "not a uuid" }` or `new Uuid { WeakString = "x" }`
built a value that never passed canonicalization or validation. For the
path types, `with` also carried over cached Parent and related values for
the old path.

The accessor is now protected init, and ISemanticString.WeakString is
get-only. Create sets the value through the cached setter delegate, which
now looks up the non-public accessor. A derived type can still set the
value from its own constructor. The path types are sealed, so with the
accessor closed nothing outside them can change a path's value through
`with` and leave the caches stale.

Breaking: callers that set WeakString in an object initializer or a `with`
expression must call Create instead.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW2sHrLz9iC1cJzLLZy
@sonarqubecloud

Copy link
Copy Markdown

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.

Public init on SemanticString.WeakString bypasses canonicalization and validation: uuid with { WeakString = "not a uuid" } compiles and runs

2 participants