refactor(mocks): extract MockTypeModel.Visibility helper#5443
Conversation
Replaces the duplicated 'IsPublic ? "public" : "internal"' ternary across five builders with a single Visibility property on MockTypeModel. Also restores the optional defaults on spanReturnElementType/spanReturnType in GenerateVoidUnifiedClass by moving the new visibility parameter earlier in the signature.
There was a problem hiding this comment.
Code Review
Clean, well-scoped refactor. No issues found.
What this does well
DRY principle applied correctly — the duplicated model.IsPublic ? "public" : "internal" ternary appeared in 5 separate builders, all computing the same value from the same source. Extracting it to MockTypeModel.Visibility is the right place for this logic: the model owns the data, so the derived string belongs there too.
Signature fix in GenerateVoidUnifiedClass — moving visibility to an earlier position in the parameter list was necessary to restore the optional defaults on spanReturnElementType / spanReturnType. C# requires that parameters with defaults trail all required parameters, so the previous ordering was forcing callers to always pass nulls explicitly. The reordering is correct and improves call-site ergonomics.
Minimal footprint — the PR touches exactly the right files and nothing more. The XML doc comment on Visibility is concise and useful.
Minor notes (no action needed)
IsPublicis still present on the model (correctly, as the source of truth).Visibilityis a purely derived helper — that separation is appropriate.- If visibility ever needs to support more than two states (e.g.,
protected internal), this is the one place to update — a good consolidation point.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
TIP This summary will be updated as you push new changes. Give us feedback
Summary
Visibilityproperty onMockTypeModelto replace the duplicatedIsPublic ? "public" : "internal"ternary across five builders.spanReturnElementType/spanReturnTypeinGenerateVoidUnifiedClassby moving thevisibilityparameter earlier in the signature (it had been demoted to required to make room for the new param).Test plan
TUnit.Mocks.Testsbuilds cleanIssue5426Tests(3/3) pass