diff --git a/CHANGELOG.md b/CHANGELOG.md index a29a56e..bbc4e26 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,14 +24,12 @@ section is the GitHub release's notes. - EF Core: `Min`, `Max`, `Order`, `OrderDescending` or `OrderBy(x => x)` over a value object with its own `CompareTo`, cast to `object` first, as in `Select(o => (object)o.Priority).Max()`, or read back through `??`, a member of a projection, a grouping's key, a join or a subquery's `First()`, as in `Select(o => new { Key = (object)o.Priority }).OrderBy(a => a.Key)`, now throws as the same query without the cast does. EF dropped the cast and answered the column's order, the opposite of memory's for a descending `CompareTo` ([Queries](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.EntityFrameworkCore/README.md#queries)). - Analyzers: CMTK0006 now warns when a constructor sets a member only on another object it creates, `new Order(...) { Id = id }` or `other with { Id = id }`, which it took as setting its own. In a build that treats warnings as errors, that can report code that built before ([CMTK0006](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0006)). - EF Core: a query filter that orders values your context holds without a lambda, as in `HasQueryFilter(o => o.Priority == Limits.Max())`, builds again; EF works it out in memory by your `CompareTo`. With a lambda, EF orders those values in the database, and the filter still fails ([Queries](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.EntityFrameworkCore/README.md#queries)). +- 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 - Analyzers: CMTK0005 no longer reports an array with a constant zero in any one dimension, such as `new OrderId[0, count]`, or a zero written as `0L` or, in a `stackalloc`, `(byte)0`: neither holds an element ([CMTK0005](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0005)). - Analyzers: CMTK0006 no longer reports an explicit interface implementation that every constructor assigns through a cast, `((IHasId)this).Id = id` ([CMTK0006](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/src/CodoMetis.TypeKit.Analyzers/README.md#cmtk0006)). - -### Fixed - - OpenAPI: each element of a list, an array or a dictionary of `Patch` is described as a patch property is, the argument's schema admitting null, where it admitted anything. A nullable `Patch?` is its argument's schema admitting null, where it was an empty required component; System.Text.Json reads a null there as a `null` patch, as it reads a property left out ([Use it in an endpoint and in the document](https://github.com/CaffeinatedCoder/CodoMetis.TypeKit/blob/main/docs/merge-patches.md#use-it-in-an-endpoint-and-in-the-document)). ## 2.3.0 — 2026-10-29 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 9f0cc73..e692396 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 2a4f013..ded2e7f 100644 --- a/test/oracles/claims.json +++ b/test/oracles/claims.json @@ -2486,6 +2486,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",