Both narrow re-reviews returned CLOSED. This closeout takes the two cheap
re-review residuals before CC3 takes references to the shared model:
R2: every array handed into the typed chargen model is now wrapped in
Array.AsReadOnly at the projection seam — a T[] behind IReadOnlyList<T>
was still downcast-mutable, and ChargenOptions is a process-shared
singleton graph.
R3: the no-Chorizite-leak guard now also walks public fields; every
current type uses properties, but a public field would have slipped
through the property-only walk.
Ledger: CC1 fix-round sha corrected to cb4703e8 (the cell previously
cited the pre-amend 459a87f2), CC1/CC2 rows flipped to REVIEW-CLOSED
with the re-review outcomes, R1 (retail refunds +1 credit on a
both-tier cost miss; port charges 0 — unreachable via retail's own
listbox, noted for CC3) and the Olthoi-locked-to-template-0 decomp fact
recorded for CC3/CC4.
Core.Tests 4736/1 skip, Content.Tests 145/0, Release.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
159 lines
6.2 KiB
C#
159 lines
6.2 KiB
C#
using System.Reflection;
|
|
using AcDream.Core.CharGen;
|
|
|
|
namespace AcDream.Core.Tests.CharGen;
|
|
|
|
/// <summary>
|
|
/// Pins the CC1 no-leak contract with a real assertion rather than a doc
|
|
/// comment. AcDream.Core references Chorizite.DatReaderWriter (for
|
|
/// <c>TextureHelpers</c>), so "no Chorizite type on any public CharGen
|
|
/// member" was convention only until this guard exists — nothing stopped a
|
|
/// future edit from putting e.g. a <c>DatReaderWriter.Enums.SkillId</c>
|
|
/// directly on a public property. This test walks every public type in the
|
|
/// <c>AcDream.Core.CharGen</c> namespace and asserts that no public
|
|
/// property, field, indexer, constructor parameter, or method
|
|
/// return/parameter type — nor any of their generic type arguments,
|
|
/// recursively — comes from the <c>DatReaderWriter</c> assembly or any
|
|
/// assembly whose name starts with <c>Chorizite</c>. (The field walk
|
|
/// closes the CC1 re-review's R3 residual: every current type uses
|
|
/// properties, but <c>public SkillId Foo;</c> would otherwise slip
|
|
/// through.)
|
|
/// </summary>
|
|
public sealed class ChargenNoChoriziteLeakTests
|
|
{
|
|
[Fact]
|
|
public void PublicCharGenSurface_NeverExposesChoriziteOrDatReaderWriterTypes()
|
|
{
|
|
Assembly coreAssembly = typeof(ChargenOptions).Assembly;
|
|
Type[] publicCharGenTypes = coreAssembly.GetTypes()
|
|
.Where(t => t.IsPublic && t.Namespace == "AcDream.Core.CharGen")
|
|
.ToArray();
|
|
|
|
// Guards the guard: if the namespace ever ends up empty (e.g. a
|
|
// rename), this test must fail loudly rather than vacuously pass.
|
|
Assert.True(
|
|
publicCharGenTypes.Length > 5,
|
|
$"Expected multiple public types in AcDream.Core.CharGen, found {publicCharGenTypes.Length}. " +
|
|
"Did the namespace get renamed or moved?");
|
|
|
|
var offenders = new List<string>();
|
|
|
|
foreach (Type type in publicCharGenTypes)
|
|
{
|
|
foreach (PropertyInfo property in type.GetProperties(
|
|
BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static))
|
|
{
|
|
CheckSite(property.PropertyType, $"{type.FullName}.{property.Name} (property)", offenders);
|
|
|
|
foreach (ParameterInfo indexParam in property.GetIndexParameters())
|
|
{
|
|
CheckSite(
|
|
indexParam.ParameterType,
|
|
$"{type.FullName}.{property.Name}[{indexParam.Name}] (indexer parameter)",
|
|
offenders);
|
|
}
|
|
}
|
|
|
|
foreach (FieldInfo field in type.GetFields(
|
|
BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static))
|
|
{
|
|
CheckSite(field.FieldType, $"{type.FullName}.{field.Name} (field)", offenders);
|
|
}
|
|
|
|
foreach (ConstructorInfo ctor in type.GetConstructors(
|
|
BindingFlags.Public | BindingFlags.Instance))
|
|
{
|
|
foreach (ParameterInfo param in ctor.GetParameters())
|
|
{
|
|
CheckSite(
|
|
param.ParameterType,
|
|
$"{type.FullName}..ctor({param.Name}) (constructor parameter)",
|
|
offenders);
|
|
}
|
|
}
|
|
|
|
foreach (MethodInfo method in type.GetMethods(
|
|
BindingFlags.Public | BindingFlags.Instance | BindingFlags.Static))
|
|
{
|
|
// Property accessors, operators, and other compiler-emitted
|
|
// members are IsSpecialName; the property/indexer loop above
|
|
// already covers accessor types directly.
|
|
if (method.IsSpecialName)
|
|
continue;
|
|
|
|
CheckSite(method.ReturnType, $"{type.FullName}.{method.Name} (return type)", offenders);
|
|
|
|
foreach (ParameterInfo param in method.GetParameters())
|
|
{
|
|
CheckSite(
|
|
param.ParameterType,
|
|
$"{type.FullName}.{method.Name}({param.Name}) (method parameter)",
|
|
offenders);
|
|
}
|
|
}
|
|
}
|
|
|
|
Assert.True(
|
|
offenders.Count == 0,
|
|
"A Chorizite/DatReaderWriter type leaked onto a public AcDream.Core.CharGen member:\n"
|
|
+ string.Join('\n', offenders));
|
|
}
|
|
|
|
private static void CheckSite(Type type, string site, List<string> offenders)
|
|
{
|
|
foreach (Type candidate in FlattenTypeArguments(type))
|
|
{
|
|
string? assemblyName = candidate.Assembly.GetName().Name;
|
|
if (assemblyName is null)
|
|
continue;
|
|
|
|
bool isForbidden =
|
|
assemblyName.Equals("DatReaderWriter", StringComparison.OrdinalIgnoreCase)
|
|
|| assemblyName.StartsWith("Chorizite", StringComparison.OrdinalIgnoreCase);
|
|
|
|
if (isForbidden)
|
|
offenders.Add($"{site}: {candidate.FullName} (assembly '{assemblyName}')");
|
|
}
|
|
}
|
|
|
|
/// <summary>Yields <paramref name="type"/> itself plus every generic
|
|
/// type argument, array element type, and by-ref (out/ref parameter)
|
|
/// element type, recursively — so e.g. <c>out IReadOnlyDictionary
|
|
/// <uint, ChargenSkillCost></c> is checked against
|
|
/// <c>ChargenSkillCost</c>, not just the outer dictionary type.</summary>
|
|
private static IEnumerable<Type> FlattenTypeArguments(Type type)
|
|
{
|
|
yield return type;
|
|
|
|
if (type.IsByRef || type.IsPointer)
|
|
{
|
|
Type? element = type.GetElementType();
|
|
if (element is not null)
|
|
{
|
|
foreach (Type inner in FlattenTypeArguments(element))
|
|
yield return inner;
|
|
}
|
|
yield break;
|
|
}
|
|
|
|
if (type.IsArray)
|
|
{
|
|
Type? element = type.GetElementType();
|
|
if (element is not null)
|
|
{
|
|
foreach (Type inner in FlattenTypeArguments(element))
|
|
yield return inner;
|
|
}
|
|
yield break;
|
|
}
|
|
|
|
if (type.IsGenericType)
|
|
{
|
|
foreach (Type argument in type.GetGenericArguments())
|
|
{
|
|
foreach (Type inner in FlattenTypeArguments(argument))
|
|
yield return inner;
|
|
}
|
|
}
|
|
}
|
|
}
|