From 27d7cc0708b8ed45a333473c0aa8aec6efa33db3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ricardo=20Gro=C3=9F?= Date: Sat, 31 Oct 2026 08:29:54 +0100 Subject: [PATCH] fix: CMTK0005 reports Array.CreateInstance, CreateInstanceFromArrayType and ArrayPool.Rent AnalyzeInvocation looked only at generic calls with one type argument, GC.AllocateArray, GC.AllocateUninitializedArray and Array.Resize. The README defines CMTK0005 as an array of a value object created with a length, which Array.CreateInstance(typeof(Plain), 3) in each overload, CreateInstanceFromArrayType(typeof(Plain[]), 3) and ArrayPool.Shared.Rent(3) are too; none was reported. The two factories are reported when a typeof names the element (the element of the array type for the second); a Type in a variable could be anything and stays silent. Rent is recognised on ArrayPool or an override of it, its T the element. The lengths are read from every int or long parameter and the elements of a params or written-out array of them, never the lower bounds, so #227's rule holds: a constant zero in any dimension stays silent. Input class (AIS): an expression that creates an array of a no-default struct, by the API that creates it. Every Array.CreateInstance and CreateInstanceFromArrayType overload that takes a length, Rent on the shared pool, on ArrayPool.Create() and on a derived pool, each with a zero-length control; a Type variable, GetType(), typeof(Plain?) and an array of another element silent. Rent's T and the typeof element run across TypeArgumentKinds (79 kinds; dynamic and string? take Rent alone, typeof refusing them): each reports exactly where GC.AllocateArray does. No sibling beyond #229's CollectionsMarshal.SetCount, which 2.4.0 takes. Reverting the source fails the forms row and the three kinds that report. Seeding the zero check away fails the three zero rows, reading the lower bounds as lengths fails the forms row, and skipping an array of lengths fails the silent row. Closes #228 Defect-Class: AIS Found-By: RS Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 1 + .../DefaultFilledCollectionAnalyzer.cs | 92 ++++++++-- src/CodoMetis.TypeKit.Analyzers/README.md | 4 +- .../DefaultFilledCollectionAnalyzerTests.cs | 158 ++++++++++++++++++ test/oracles/claims.json | 14 ++ 5 files changed, 251 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aecf40b..b071c6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ section is the GitHub release's notes. - Generators: an `Equals` a value object declares for one of its base records' types, such as `Equals(DeepBase?)`, now fails the build (CMTK1011), so your build can fail when you upgrade. A `HashSet` compared by it while `==` compared the underlying value; normalise values in `Create` instead ([Equality](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Generators/README.md#equality)). - Generators: a `CompareTo(TSelf)`, `CompareTo(object)`, `TryFrom` or `Revalidate` a value object inherits from a base record now fails the build (CMTK1008, CMTK1011), so your build can fail when you upgrade. It stood in for the generated member: an inherited `CompareTo` made `<` overflow the stack and an EF Core query order by the column, and an inherited `TryFrom` need not run `Create`. Declare it on the value object ([Write your own members](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Generators/README.md#write-your-own-members)). - Generators: `IValueObject`, `IValueObjectMaterializer` or `IPlainValueObject` declared through an interface of yours, such as an `IEntityId : IValueObject`, or by a base record now fails with CMTK1011 naming that interface, where it failed with a Metalama error (LAMA0512) that named nothing you wrote. To constrain generic code, use `IValueObject` itself ([Write your own members](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Generators/README.md#write-your-own-members)). +- Analyzers: CMTK0005 now also warns at `Array.CreateInstance(typeof(OrderId), n)`, `Array.CreateInstanceFromArrayType(typeof(OrderId[]), n)` and `ArrayPool.Rent(n)`, which create default slots as `new OrderId[n]` does. In a build that treats warnings as errors, that can report code that built before ([CMTK0005](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0005)). ### Fixed diff --git a/src/CodoMetis.TypeKit.Analyzers/DefaultFilledCollectionAnalyzer.cs b/src/CodoMetis.TypeKit.Analyzers/DefaultFilledCollectionAnalyzer.cs index c984a14..1707b0b 100644 --- a/src/CodoMetis.TypeKit.Analyzers/DefaultFilledCollectionAnalyzer.cs +++ b/src/CodoMetis.TypeKit.Analyzers/DefaultFilledCollectionAnalyzer.cs @@ -1,3 +1,4 @@ +using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis; @@ -16,7 +17,11 @@ namespace CodoMetis.TypeKit.Analyzers; /// CMTK0001 sees default written out, and none of these forms writes it: new OrderId[n], /// stackalloc OrderId[n], GC.AllocateUninitializedArray<OrderId>(n), /// GC.AllocateArray and Array.Resize (measured 2026-09-28), which adds default slots -/// whenever the new size is larger, and the rule cannot tell whether it is. An array with initial +/// whenever the new size is larger, and the rule cannot tell whether it is. Also +/// Array.CreateInstance(typeof(OrderId), n), Array.CreateInstanceFromArrayType(typeof(OrderId[]), n) +/// and ArrayPool<OrderId>.Rent(n) (#228): the first two only where a typeof names the element, +/// since a Type variable could be anything, and a rented array is reported as a fresh one, all +/// default, though a reused one holds what it was returned with. An array with initial /// elements, a constant length or size of zero in any one dimension and a collection expression start /// with no default element and are not reported. A warning, since filling such an array in a loop /// straight after is correct code the rule cannot tell apart. @@ -52,9 +57,10 @@ public override void Initialize(AnalysisContext context) var gc = start.Compilation.GetTypeByMetadataName("System.GC"); var array = start.Compilation.GetTypeByMetadataName("System.Array"); + var pool = start.Compilation.GetTypeByMetadataName("System.Buffers.ArrayPool`1"); start.RegisterOperationAction(operation => AnalyzeArrayCreation(operation, symbols), OperationKind.ArrayCreation); - start.RegisterOperationAction(operation => AnalyzeInvocation(operation, symbols, gc, array), OperationKind.Invocation); + start.RegisterOperationAction(operation => AnalyzeInvocation(operation, symbols, gc, array, pool), OperationKind.Invocation); // IOperation has no public node for stackalloc; the syntax is read instead. start.RegisterSyntaxNodeAction(node => AnalyzeStackAlloc(node, symbols), SyntaxKind.StackAllocArrayCreationExpression); @@ -72,33 +78,85 @@ private static void AnalyzeArrayCreation(OperationAnalysisContext context, TypeK if (Diagnose(context.ContainingSymbol, creation.Syntax, "An array created with a length", element, symbols) is { } diagnostic) context.Report(diagnostic); } - private static void AnalyzeInvocation(OperationAnalysisContext context, TypeKitSymbols symbols, INamedTypeSymbol? gc, INamedTypeSymbol? array) + private static void AnalyzeInvocation(OperationAnalysisContext context, TypeKitSymbols symbols, INamedTypeSymbol? gc, INamedTypeSymbol? array, INamedTypeSymbol? arrayPool) { var invocation = (IInvocationOperation)context.Operation; var method = invocation.TargetMethod; - if (method.TypeArguments.Length != 1) return; + string? form = null; + ITypeSymbol? element = null; - string? form = null; - var size = -1; - - if (SymbolEqualityComparer.Default.Equals(method.ContainingType, gc) && AllocatingMethods.Contains(method.Name)) + if (method.TypeArguments.Length == 1 && SymbolEqualityComparer.Default.Equals(method.ContainingType, gc) && AllocatingMethods.Contains(method.Name)) + { + // (int length, bool pinned = false) + form = $"GC.{method.Name}"; + element = method.TypeArguments[0]; + } + else if (method.TypeArguments.Length == 1 && SymbolEqualityComparer.Default.Equals(method.ContainingType, array) && method.Name == "Resize") + { + // (ref T[]? array, int newSize) + form = "Array.Resize"; + element = method.TypeArguments[0]; + } + else if (SymbolEqualityComparer.Default.Equals(method.ContainingType, array) && method.Name is "CreateInstance" or "CreateInstanceFromArrayType") { - form = $"GC.{method.Name}"; - size = 0; // (int length, bool pinned = false) + // (Type elementType or arrayType, the lengths: one to three ints, int[] or long[], then int[] lowerBounds or nothing). + // Only a typeof names the element; a Type held in a variable could be anything. + var type = invocation.Arguments.FirstOrDefault(argument => argument.Parameter?.Ordinal == 0)?.Value; + while (type is IConversionOperation conversion) type = conversion.Operand; + + form = $"Array.{method.Name}"; + element = (type as ITypeOfOperation)?.TypeOperand; + if (method.Name == "CreateInstanceFromArrayType") element = (element as IArrayTypeSymbol)?.ElementType; } - else if (SymbolEqualityComparer.Default.Equals(method.ContainingType, array) && method.Name == "Resize") + else if (method.Name == "Rent" && PoolOf(method, arrayPool) is { } pool) { - form = "Array.Resize"; - size = 1; // (ref T[]? array, int newSize) + // (int minimumLength): a fresh array from the pool holds nothing but default slots. + form = "ArrayPool.Rent"; + element = pool.TypeArguments[0]; } - if (form is null) return; + if (form is null || element is null) return; + + // A length of zero in any dimension creates no slot, as new T[0] and new T[0, n] do not. + if (Lengths(invocation).Any(length => IsZero(length.ConstantValue))) return; + + if (Diagnose(context.ContainingSymbol, invocation.Syntax, form, element, symbols) is { } diagnostic) context.Report(diagnostic); + } + + /// The ArrayPool<T> whose Rent is or overrides. + private static INamedTypeSymbol? PoolOf(IMethodSymbol method, INamedTypeSymbol? arrayPool) + { + for (var current = method; current is not null; current = current.OverriddenMethod) + { + if (SymbolEqualityComparer.Default.Equals(current.ContainingType.OriginalDefinition, arrayPool)) return current.ContainingType; + } - // A length of zero creates no slot, as new T[0] does not. - if (invocation.Arguments.FirstOrDefault(argument => argument.Parameter?.Ordinal == size) is { } length && IsZero(length.Value.ConstantValue)) return; + return null; + } - if (Diagnose(context.ContainingSymbol, invocation.Syntax, form, method.TypeArguments[0], symbols) is { } diagnostic) context.Report(diagnostic); + /// + /// The length arguments of an allocating call: every int or long after the element, and the + /// elements of a params or written-out array of lengths, but not the lower bounds. + /// + private static IEnumerable Lengths(IInvocationOperation invocation) + { + foreach (var argument in invocation.Arguments) + { + if (argument.Parameter is not { } parameter || parameter.Name == "lowerBounds" || parameter.RefKind != RefKind.None) continue; + + switch (parameter.Type) + { + case { SpecialType: SpecialType.System_Int32 or SpecialType.System_Int64 }: + yield return argument.Value; + break; + + case IArrayTypeSymbol { ElementType.SpecialType: SpecialType.System_Int32 or SpecialType.System_Int64 } + when argument.Value is IArrayCreationOperation { Initializer: { } lengths }: + foreach (var length in lengths.ElementValues) yield return length; + break; + } + } } private static void AnalyzeStackAlloc(SyntaxNodeAnalysisContext context, TypeKitSymbols symbols) diff --git a/src/CodoMetis.TypeKit.Analyzers/README.md b/src/CodoMetis.TypeKit.Analyzers/README.md index dc51499..7d445b3 100644 --- a/src/CodoMetis.TypeKit.Analyzers/README.md +++ b/src/CodoMetis.TypeKit.Analyzers/README.md @@ -302,7 +302,9 @@ OrderId[] kept = [.. rows.Select(r => OrderId.From(r.Id))]; // built from valu ``` `stackalloc`, `GC.AllocateUninitializedArray`, `GC.AllocateArray` and `Array.Resize` are reported -too. `Array.Resize` adds default slots whenever the new size is larger, which the rule can't tell. A +too, and so are `Array.CreateInstance` and `Array.CreateInstanceFromArrayType` when a `typeof` names +the element, and `ArrayPool.Rent`. A `Type` held in a variable could be anything, so it isn't +reported. `Array.Resize` adds default slots whenever the new size is larger, which the rule can't tell. A constant length or size of zero creates no slot, as `new OrderId[0]` doesn't, and isn't reported. That holds for any one dimension: `new OrderId[0, count]` holds no element either. diff --git a/test/CodoMetis.TypeKit.Analyzers.Tests/DefaultFilledCollectionAnalyzerTests.cs b/test/CodoMetis.TypeKit.Analyzers.Tests/DefaultFilledCollectionAnalyzerTests.cs index f9c91eb..7807f1b 100644 --- a/test/CodoMetis.TypeKit.Analyzers.Tests/DefaultFilledCollectionAnalyzerTests.cs +++ b/test/CodoMetis.TypeKit.Analyzers.Tests/DefaultFilledCollectionAnalyzerTests.cs @@ -1,4 +1,7 @@ +using CodoMetis.TypeKit.Analyzers.Tests.Variants; using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; +using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Testing; namespace CodoMetis.TypeKit.Analyzers.Tests; @@ -9,6 +12,7 @@ public sealed class DefaultFilledCollectionAnalyzerTests private const string Subjects = """ using System; + using System.Buffers; using CodoMetis.TypeKit; using CodoMetis.TypeKit.ValueObjects; @@ -139,6 +143,160 @@ public static void Run(int n) Cmtk0005(1, "An array created with a length", "Plain")) .RunAsync(TestContext.Current.CancellationToken); + /// + /// The non-generic array factories and the pool create default slots as new Plain[n] does, and + /// none was reported (#228). Across the input class, an expression that creates an array of a value + /// object, by the API that creates it: each overload of Array.CreateInstance and + /// CreateInstanceFromArrayType, Rent on the shared pool, a created one and a derived one. + /// + [Fact] + public Task The_array_factories_and_the_pool_report() => + Test( + """ + public sealed class Pool : ArrayPool + { + public override Plain[] Rent(int minimumLength) => []; + public override void Return(Plain[] array, bool clearArray = false) { } + } + + public static class C + { + public static void Run(int n, int[] lengths, Pool pool) + { + var one = {|#0:Array.CreateInstance(typeof(Plain), n)|}; + var two = {|#1:Array.CreateInstance(typeof(Plain), 2, 2)|}; + var three = {|#2:Array.CreateInstance(typeof(Plain), 2, n, 2)|}; + var four = {|#3:Array.CreateInstance(typeof(Plain), 2, 2, 2, 2)|}; + var held = {|#4:Array.CreateInstance(typeof(Plain), lengths)|}; + var bounded = {|#5:Array.CreateInstance(typeof(Plain), new[] { 2 }, new[] { 0 })|}; + var wide = {|#6:Array.CreateInstance(typeof(Plain), 2L, 2L)|}; + var fromArray = {|#7:Array.CreateInstanceFromArrayType(typeof(Plain[]), n)|}; + var fromGrid = {|#8:Array.CreateInstanceFromArrayType(typeof(Plain[,]), 2, 2)|}; + var fromBounded = {|#9:Array.CreateInstanceFromArrayType(typeof(Plain[]), [2], [1])|}; + var shared = {|#10:ArrayPool.Shared.Rent(n)|}; + var created = {|#11:ArrayPool.Create().Rent(3)|}; + var derived = {|#12:pool.Rent(3)|}; + var options = {|#13:Array.CreateInstance(typeof(Option), n)|}; + } + + public static Array Generic(int n) where T : struct, IValue => {|#14:Array.CreateInstance(typeof(T), n)|}; + } + """, + Cmtk0005(0, "Array.CreateInstance", "Plain"), + Cmtk0005(1, "Array.CreateInstance", "Plain"), + Cmtk0005(2, "Array.CreateInstance", "Plain"), + Cmtk0005(3, "Array.CreateInstance", "Plain"), + Cmtk0005(4, "Array.CreateInstance", "Plain"), + Cmtk0005(5, "Array.CreateInstance", "Plain"), + Cmtk0005(6, "Array.CreateInstance", "Plain"), + Cmtk0005(7, "Array.CreateInstanceFromArrayType", "Plain"), + Cmtk0005(8, "Array.CreateInstanceFromArrayType", "Plain"), + Cmtk0005(9, "Array.CreateInstanceFromArrayType", "Plain"), + Cmtk0005(10, "ArrayPool.Rent", "Plain"), + Cmtk0005(11, "ArrayPool.Rent", "Plain"), + Cmtk0005(12, "ArrayPool.Rent", "Plain"), + Cmtk0005(13, "Array.CreateInstance", "Option"), + Cmtk0005(14, "Array.CreateInstance", "T")) + .RunAsync(TestContext.Current.CancellationToken); + + /// + /// A constant zero length in any dimension creates no slot, and a Type the rule cannot read names + /// no element: a variable, GetType(), a nullable value object, an array of another type. + /// + [Fact] + public Task The_array_factories_with_a_zero_length_or_an_unknown_type_stay_silent() => + Test( + """ + public static class C + { + public static void Run(int n, Type type, Plain a) + { + var one = Array.CreateInstance(typeof(Plain), 0); + var first = Array.CreateInstance(typeof(Plain), 0, n); + var second = Array.CreateInstance(typeof(Plain), n, 0); + var third = Array.CreateInstance(typeof(Plain), n, 2, 0); + var listed = Array.CreateInstance(typeof(Plain), 2, 0, 2, 2); + var bounded = Array.CreateInstance(typeof(Plain), new[] { 0 }, new[] { 5 }); + var wide = Array.CreateInstance(typeof(Plain), 2L, 0L); + var fromArray = Array.CreateInstanceFromArrayType(typeof(Plain[]), 0); + var fromGrid = Array.CreateInstanceFromArrayType(typeof(Plain[,]), n, 0); + var shared = ArrayPool.Shared.Rent(0); + var created = ArrayPool.Create().Rent(minimumLength: 0); + + var variable = Array.CreateInstance(type, n); + var runtime = Array.CreateInstance(a.GetType(), n); + var nullable = Array.CreateInstance(typeof(Plain?), n); + var numbers = Array.CreateInstance(typeof(int), n); + var jagged = Array.CreateInstance(typeof(Plain[]), n); + var other = Array.CreateInstanceFromArrayType(typeof(int[]), n); + var unknown = Array.CreateInstanceFromArrayType(type, n); + var pooled = ArrayPool.Shared.Rent(n); + var instance = Activator.CreateInstance(typeof(Plain)); + } + } + """) + .RunAsync(TestContext.Current.CancellationToken); + + /// + /// ArrayPool<T>.Rent and the typeof factories judge their element as + /// GC.AllocateArray<T> does, across every kind of : each + /// reports exactly where the generic allocator does. typeof takes neither dynamic (CS1962) nor a nullable reference type + /// (CS8639), so those two kinds run Rent alone. + /// + [Theory] + [MemberData(nameof(Kinds))] + public async Task The_factories_judge_each_kind_as_the_generic_allocator_does(string kindName) + { + var kind = TypeArgumentKinds.All.Single(candidate => candidate.Name == kindName); + var typeOf = kind.Argument is not ("dynamic" or "string?"); + var source = + $$""" + #nullable enable + using System.Buffers; + + public static class Use{{kind.Parameters}} {{kind.Where}} + { + public static void M(int n) + { + _ = GC.AllocateArray<{{kind.Argument}}>(n); + _ = ArrayPool<{{kind.Argument}}>.Shared.Rent(n); + {{(typeOf ? $"_ = Array.CreateInstance(typeof({kind.Argument}), n);" : "")}} + {{(typeOf ? $"_ = Array.CreateInstance(typeof({kind.Argument}), n, n);" : "")}} + {{(typeOf ? $"_ = Array.CreateInstanceFromArrayType(typeof({kind.Argument}[]), n);" : "")}} + } + } + """; + + var forms = await ReportedForms(source); + + var reported = forms.Contains("GC.AllocateArray"); + if (kind is KindOfType { Is: var type } && type.HasFlag(Is.Struct) && type.HasFlag(Is.ValueObjectMarker)) reported.ShouldBeTrue("a value object struct is the positive control"); + + string[] factories = typeOf ? ["ArrayPool.Rent", "Array.CreateInstance", "Array.CreateInstance", "Array.CreateInstanceFromArrayType"] : ["ArrayPool.Rent"]; + forms.Where(form => form != "GC.AllocateArray").Order().ShouldBe(reported ? factories.Order() : []); + } + + public static TheoryData Kinds() => [.. TypeArgumentKinds.All.Where(kind => kind.Mentions.All(Declared)).Select(kind => kind.Name)]; + + private static bool Declared(string attribute) => RealTypeKit.Assembly.GetType($"CodoMetis.TypeKit.{attribute}Attribute") is not null; + + /// The form of each CMTK0005 the analyzer reports in , which must compile. + private static async Task ReportedForms(string source) + { + var cancellation = TestContext.Current.CancellationToken; + var references = await ReferenceAssemblies.Net.Net100.ResolveAsync(LanguageNames.CSharp, cancellation); + var compilation = CSharpCompilation.Create( + "Kinds", + [CSharpSyntaxTree.ParseText(source, cancellationToken: cancellation), CSharpSyntaxTree.ParseText(TypeArgumentKinds.Candidates, cancellationToken: cancellation)], + [.. references, RealTypeKit.Reference], + new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)); + + compilation.GetDiagnostics(cancellation).Where(diagnostic => diagnostic.Severity == DiagnosticSeverity.Error).ShouldBeEmpty(); + + var diagnostics = await compilation.WithAnalyzers([new DefaultFilledCollectionAnalyzer()]).GetAnalyzerDiagnosticsAsync(cancellation); + return [.. diagnostics.Select(diagnostic => diagnostic.GetMessage().Split(" fills ")[0])]; + } + /// /// A zero of any integral type creates no slot: a dimension may be a long, uint or /// ulong, and a stackalloc size's constant is read before its conversion to int, diff --git a/test/oracles/claims.json b/test/oracles/claims.json index d8aa8c0..ca79ecb 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -2432,6 +2432,20 @@ "ForbiddenDefaultInitializationAnalyzerTests" ] }, + { + "id": "K05-a-factory-given-a-typeof", + "source": "src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0005", + "quote": "and so are `Array.CreateInstance` and `Array.CreateInstanceFromArrayType` when a `typeof` names the element, and `ArrayPool.Rent`. A `Type` held in a variable could be anything, so it isn't reported.", + "claim": "CMTK0005 reports Array.CreateInstance and Array.CreateInstanceFromArrayType given a typeof of a no-default type, and ArrayPool.Rent of one, and not a Type held in a variable.", + "oracle": "claim", + "surfaces": [ + "IValueObject.*", + "IValidatedValue.*" + ], + "heldBy": [ + "DefaultFilledCollectionAnalyzerTests" + ] + }, { "id": "K05-a-zero-dimension-is-silent", "source": "src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0005",