Add Buildvana.Core.Versioning shared library (Versioning Phase 4)

[rdeago]

Closes #271. Part of #267.

Summary

Stands up a new host-agnostic Buildvana.Core.Versioning library holding the pure version logic that native versioning (Phases 5–6) will build on. The library is added and unit-tested but not yet wired into the live version-computation path — bv still computes versions via nbgv, so there is no behavior change in this phase.

Changes

• New src/Buildvana.Core.Versioning/ ($(StandardTfm); depends only on NuGet.Versioning + CommunityToolkit.Diagnostics):
  • VersionSpec and VersionSpecChange moved verbatim from Buildvana.Tool (recorded as renames; namespace → Buildvana.Core.Versioning, made public).
  • VersionFileData — immutable model of current-version.json ({ major, minor, prerelease }) with a strict Parse(string) rejecting malformed input (FormatException). Parse-only; "fail loudly if the file is absent" is documented as a caller contract.
  • VersionCalculator.Compute(...) — builds M.m.{height}[-tag] from version-file data + git height + branch; IsPublicRelease = branch matches any public-release pattern.
  • CalculatedVersion — result record (SemanticVersion, CurrentStr, IsPublicRelease, IsPrerelease).
• Buildvana.Tool references the new library; usings updated where the two types moved. No logic change.
• New tests/Buildvana.Core.Versioning.Tests/ (TUnit, 24 tests): version-string construction, regex matching (empty list, detached HEAD), current-version.json parsing (valid/malformed), plus VersionSpec (previously untested).

Notes / decisions

• Named Buildvana.Core.Versioning (Core-tier convention) rather than the issue's Buildvana.Versioning.
• VersionSpecChange moved alongside VersionSpec (ApplyChange depends on it).
• Compute takes pre-compiled IReadOnlyList<Regex>; callers own compilation/timeout/error-translation (mirrors ReleaseSettings).
• Guard added: a prerelease VersionFileData with an empty prereleaseTag throws, so a prerelease can't silently degrade to a stable version.

Acceptance criteria

• Library builds cleanly; VersionSpec no longer exists under Buildvana.Tool.
• Unit tests for VersionCalculator and VersionFileData pass.
• bv behavior unchanged (NBGV still in use).
• CHANGELOG: internal-only — no public entry.

Verification

• dotnet bv pack → success (Release artifacts produced).
• dotnet build -c Release → 0 warnings / 0 errors.
• ReSharper inspectcode --severity=WARNING → 0 results.
• Tests: Versioning 24/24, Tool 66/66.

🤖 Generated with Claude Code

[rdeago]

@claude please review this PR.

[claude]

Claude finished @rdeago's task in 3m 12s —— View job

---

PR Review

• Gather context and read changed files
• Review new Buildvana.Core.Versioning library
• Review test files
• Review Buildvana.Tool modifications
• Check architecture/style compliance
• Post review findings

---

Overall this is a clean, well-structured lift-and-shift phase. The library is host-agnostic, the guards are consistent with the rest of the codebase, and the test suite covers the main paths. A few items below — one design observation worth discussing, two test coverage gaps, and one minor inconsistency.

Design observation: CalculatedVersion has two derivable fields

VersionCalculator.cs:64-66:

var currentStr = semanticVersion.ToNormalizedString();
var isPublicRelease = publicReleaseBranches.Any(pattern => pattern.IsMatch(branchName));
return new CalculatedVersion(semanticVersion, currentStr, isPublicRelease, semanticVersion.IsPrerelease);

Both CurrentStr and IsPrerelease are always equal to semanticVersion.ToNormalizedString() and semanticVersion.IsPrerelease respectively. As primary constructor parameters on a sealed record, they participate in equality — so two CalculatedVersion instances with the same SemanticVersion but a mismatched CurrentStr would be considered unequal, which is a latent footgun (e.g., in future tests constructing CalculatedVersion directly).

Making them computed properties instead would prevent drift and clean up equality:

public string CurrentStr => SemanticVersion.ToNormalizedString();
public bool IsPrerelease => SemanticVersion.IsPrerelease;

The record would then become:

public sealed record CalculatedVersion(SemanticVersion SemanticVersion, bool IsPublicRelease)
{
    public string CurrentStr => SemanticVersion.ToNormalizedString();
    public bool IsPrerelease => SemanticVersion.IsPrerelease;
}

Not a correctness bug today since Compute is the only creation site, but worth considering while the API surface is still fresh.

Fix this →

Minor: two test coverage gaps in VersionFileDataTests

Missing null input test — Parse starts with Guard.IsNotNull(json), but there's no corresponding test:

[Test]
public async Task Parse_NullJson_Throws()
{
    await Assert.That(() => VersionFileData.Parse(null!)).Throws<ArgumentNullException>();
}

Missing minor symmetry — Parse_MissingMajor_Throws has no sibling Parse_MissingMinor_Throws. Given GetNonNegativeInt32 is shared between the two, the coverage matters.

Fix this →

Nit: test regexes don't follow the documented caller guidance

VersionCalculatorTests.cs:43,51,67 construct Regex instances without RegexOptions.CultureInvariant or a match timeout:

IReadOnlyList<Regex> branches = [new Regex("^main$"), new Regex(@"^v\d+\.\d+$")];

The Compute XML doc explicitly says callers should compile patterns with CultureInvariant and a match timeout. For pure-ASCII patterns the difference is nil, but following the documented convention in tests is more consistent and avoids confusing future readers who look to the tests as usage examples.

---

Everything else looks good: the architecture placement is correct (Buildvana.Core.* tier, no host references, no explicit IsPackable needed since Common.props covers it), the namespace root strips the .Versioning suffix correctly, VersionFileData.Parse error messages consistently name the file and the offending property, the using (document) pattern is correct for the try/catch separation, and the VersionSpec XML doc still correctly references version.json (the NBGV file, not current-version.json).

[rdeago]

I'm retiring this PR because of fundamental flaws in the specs. Issues #271, #272, and #273 are going to be replaced by a better-thought-out comprehensive issue.