[rdeago]

Checklist of related issues / discussions

• Closes Remove the null-forgiving operators from src/ #408
• Part of #
• Related discussion(s): #

Proposed changes

A null-forgiving operator (!) tells the compiler that a value is not null, and the compiler stops checking it. The fact behind the operator then lives in no type. #408 asks to remove the operators under src/, and to add a rule against them to csharp-style-guide.md. #412 added Nullable.Extended.Analyzer to Buildvana SDK, and Common.props turned it off until the operators were gone.

This PR removes every operator, under src/, tests/, and .buildvana/hooks/. It then removes UseNullableExtendedAnalyzer=false from Common.props, so the analyzer reports any new operator.

src/ and the hook

The fixes follow the order the new style guide section prescribes:

• A type states the fact.
  • PinWriting.Moving returns each moving pin as a PinMove, with its non-null target.
  • AppDirectiveEditor.RewriteVersions passes the version text to its callback as a non-null argument.
  • PackagePinDumps yields each pin dump with its non-null ProjectAssetsFile, for OrphanDetector and OverrideProject.
  • TryGetKeyedElementType returns the JsonKeyedObjectAttribute with the element type.
• A check that flow analysis follows. Patterns replace casts and bool locals, in SelfVersionService, GitignorePattern, ConstantValueParser, SidecarReader, and the JSON code.
• Debug.Assert, as a last resort. 22 assertions in src/, and one in post-release.cs. Each message says why the value cannot be null. Seven of them, and the one in the hook, assert that Path.GetDirectoryName of a full file path is not null.

No operator is left under src/. #408 admitted operators with a comment stating the invariant. Debug.Assert states the invariant instead, and checks it in every Debug build.

tests/

#408 left open whether tests/ follows the rule. It does, with one exception:

• await Assert.That(value).IsNotNull() returns the value, non-null, where a test used value!.
• A comparison through ?. fails on a null, as in node["a"]?["b"]?.GetValue<string>().
• WithMessage or WithMessageContaining chained onto Throws<T>() replaces exception!.Message.
• AssemblyExtensions.GetMetadata, in Buildvana.Core.Testing, replaces 22 lookups of AssemblyMetadataAttribute.Value!.
• A synchronous helper throws InvalidOperationException when the null comes from the code under test.

The exception is a null! or default! that a test passes on purpose, as the input under test. 29 operators keep the !, each under the comment // ! Null on purpose: testing null detection.

OrphanDetectorTests and OverrideLifecycleTests passed new Lazy<SolutionContext>(static () => null!). OrphanDetector reads the value, so the lazy now returns an empty SolutionContext.

The style guide

csharp-style-guide.md has a new section, "The null-forgiving operator". It bans the operator, and lists the fixes above in order of preference. It does not allow the // ! comment, except for the test inputs above.

Additional changes

• GitService.GetLatestVersions threw ArgumentException when two version tags named the same commit. ToDictionary keyed the tags by commit. The fix keeps every version tag of a commit. Latest is the highest version on the first tagged commit, walking from HEAD. LatestStable is the highest stable version on the first commit with a stable tag. GitServiceTests reproduces the throw. IGitService and docs/tool-commands/version.md describe the rule, and CHANGELOG.md has a bullet.
• Line length in tests. The test files this PR touches had lines over 140 characters, and declarations over 120. They now comply, in a separate commit.

Types of changes

This pull request introduces the following types of changes:

• Bug fix
• New feature
• Test addition / update (no changes to non-test code)
• Refactor (no changes in public API syntax or semantics)
• Performance improvement (no changes in public API syntax or semantics)
• Documentation update (not including changelog)
• Dependency addition / update
• Changes to toolchain configuration
• Changes to CI (workflows, bot / app configurations)
• Changes to repository files (.gitattributes, .gitignore)
• Other

Breaking changes

This pull request introduces breaking changes:

• Yes
• No

Checklist

• For all types of changes:
  • I have read the Code of Conduct and agree to be bound by it
  • I have read the Contributor Guide and agree to follow it
• For code changes only:
  • The project builds on my machine, via the provided build script, with zero warnings
  • I have added tests that prove my feature works / my fix is effective
  • I have added / modified XML documentation according to changes in code
  • I have checked that all the links I added or modified in XML documentation point to their intended destination
• For documentation changes only:
  • I have checked and proofread documentation locally
  • I have checked that all the links I added or modified point to their intended destination

🤖 Generated with Claude Code

[codecov]

Codecov Report

❌ Patch coverage is 92.36111% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.28%. Comparing base (83627c0) to head (c5f4235).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
...a.Core.Configuration/BuildvanaJsonConfigExample.cs	40.00%	1 Missing and 2 partials ⚠️
src/Buildvana.Core.Testing/AssemblyExtensions.cs	60.00%	0 Missing and 2 partials ⚠️
....Core.Configuration/BuildvanaJsonConfigProvider.cs	50.00%	0 Missing and 1 partial ⚠️
...ana.Core.IO/Gitignore/GitignorePattern-matching.cs	75.00%	0 Missing and 1 partial ⚠️
...e.Json/JsonKeyedObjectConverter.ListConverter`1.cs	90.00%	0 Missing and 1 partial ⚠️
.../Buildvana.Core.Json/Schema/JsonSchemaValidator.cs	0.00%	0 Missing and 1 partial ⚠️
src/Buildvana.Tool/Build/BuildPipeline.cs	0.00%	1 Missing ⚠️
...dvana.Tool/Services/Dependencies/ToolPinUpdater.cs	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #413   +/-   ##
=======================================
  Coverage   87.28%   87.28%           
=======================================
  Files         321      323    +2     
  Lines        9514     9555   +41     
  Branches     1793     1805   +12     
=======================================
+ Hits         8304     8340   +36     
- Misses        911      913    +2     
- Partials      299      302    +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[rdeago]

Review by Claude. Self-review.

⚠️ Changes requested.

---

One finding, on the tests of GitService.GetLatestVersions.

[rdeago]

Review by Claude. Self-review.

👍 Approved.

---

No blocking findings. GetLatestVersions_WithSeveralVersionTagsOnOneCommit_ReadsTheHighest addresses the previous finding.