diff --git a/src/Authoring/WinRT.SourceGenerator2/AnalyzerReleases.Shipped.md b/src/Authoring/WinRT.SourceGenerator2/AnalyzerReleases.Shipped.md index e1c6e605f..cd4010d2a 100644 --- a/src/Authoring/WinRT.SourceGenerator2/AnalyzerReleases.Shipped.md +++ b/src/Authoring/WinRT.SourceGenerator2/AnalyzerReleases.Shipped.md @@ -26,4 +26,5 @@ CSWINRT2016 | WindowsRuntime.SourceGenerator | Warning | Public authored type mi CSWINRT2017 | WindowsRuntime.SourceGenerator | Warning | Public authored type mixing '[ContractVersion]' and '[Version]' CSWINRT2018 | WindowsRuntime.SourceGenerator | Warning | '[WindowsRuntimeNativeExposedType]' target type cannot be instantiated CSWINRT2019 | WindowsRuntime.SourceGenerator | Warning | '[WindowsRuntimeNativeExposedType]' target type is not a projected class -CSWINRT2020 | WindowsRuntime.SourceGenerator | Warning | Duplicate '[WindowsRuntimeNativeExposedType]' target type \ No newline at end of file +CSWINRT2020 | WindowsRuntime.SourceGenerator | Warning | Duplicate '[WindowsRuntimeNativeExposedType]' target type +CSWINRT2021 | WindowsRuntime.SourceGenerator | Warning | Reading from a write-only 'Span' parameter diff --git a/src/Authoring/WinRT.SourceGenerator2/Diagnostics/Analyzers/WriteOnlySpanParameterAnalyzer.cs b/src/Authoring/WinRT.SourceGenerator2/Diagnostics/Analyzers/WriteOnlySpanParameterAnalyzer.cs new file mode 100644 index 000000000..8d1dd9961 --- /dev/null +++ b/src/Authoring/WinRT.SourceGenerator2/Diagnostics/Analyzers/WriteOnlySpanParameterAnalyzer.cs @@ -0,0 +1,997 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Collections.Immutable; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace WindowsRuntime.SourceGenerator.Diagnostics; + +/// +/// A diagnostic analyzer that warns when the implementation of a Windows Runtime method reads from a +/// parameter, which is projected as a write-only fill array in the ABI. +/// +/// +/// In the Windows Runtime ABI, array parameters use one of three conventions: a parameter +/// is a "pass array" ([in], read-only), an out T[] parameter is a "receive array" ([out] with byref), and +/// a parameter is a "fill array" ([out] without byref). A fill array is write-only: the +/// implementation is given a buffer allocated by the caller that it is expected to fill, and reading from it is not supported. +/// This analyzer detects the most common ways a method might read from such a parameter, while avoiding false positives on +/// write-only usages. +/// +/// Reading back a value the method itself has just written is valid, as the element being read is no longer uninitialized. +/// To account for that, a read is not reported when a write covering the same location is guaranteed to run before it, +/// such as a preceding span.Clear() or span.Fill(value) call, or a preceding assignment to that same element. +/// +/// +[DiagnosticAnalyzer(LanguageNames.CSharp)] +public sealed class WriteOnlySpanParameterAnalyzer : DiagnosticAnalyzer +{ + /// + public override ImmutableArray SupportedDiagnostics { get; } = [DiagnosticDescriptors.WriteOnlySpanParameterRead]; + + /// + public override void Initialize(AnalysisContext context) + { + context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); + context.EnableConcurrentExecution(); + + context.RegisterCompilationStartAction(static context => + { + // 'Span' parameters are only projected as fill arrays when authoring a Windows Runtime component + if (!context.Options.AnalyzerConfigOptionsProvider.GlobalOptions.GetCsWinRTComponent()) + { + return; + } + + // Get the 'System.Span' symbol (the type that is projected as a write-only fill array) + if (context.Compilation.GetTypeByMetadataName("System.Span`1") is not { } spanType) + { + return; + } + + // Get the 'System.ReadOnlySpan' symbol (used to detect conversions that would allow reads) + if (context.Compilation.GetTypeByMetadataName("System.ReadOnlySpan`1") is not { } readOnlySpanType) + { + return; + } + + // Also get the '[Conditional]' symbol, which is used to detect calls that might be removed entirely + INamedTypeSymbol? conditionalAttributeType = context.Compilation.GetTypeByMetadataName("System.Diagnostics.ConditionalAttribute"); + + WellKnownSymbols symbols = new(spanType, readOnlySpanType, conditionalAttributeType); + + // The whole body of a method is analyzed at once, rather than each operation in isolation, so that + // each candidate read can also be inspected in the context of the code that necessarily runs before it + context.RegisterOperationBlockAction(context => + { + // Fill array parameters can only be declared by a method on a public, top-level class (see + // 'IsWindowsRuntimeMethod'), which is a cheap way to skip most of the code being compiled + if (context.OwningSymbol.ContainingType is not { TypeKind: TypeKind.Class, DeclaredAccessibility: Accessibility.Public, ContainingType: null }) + { + return; + } + + // For a method, the parameters are known up front, so bodies that can't possibly read from a fill + // array parameter are skipped right away. That is not the case for field and property initializers, + // which can also reference the parameters of the primary constructor of their containing type. + if (context.OwningSymbol is IMethodSymbol method && !HasSpanParameter(method, spanType)) + { + return; + } + + foreach (IOperation operationBlock in context.OperationBlocks) + { + bool allowsSuppression = AllowsSuppression(operationBlock); + + foreach (IOperation operation in operationBlock.DescendantsAndSelf()) + { + switch (operation) + { + case IPropertyReferenceOperation propertyReference: + AnalyzeElementRead(context, propertyReference, symbols, allowsSuppression); + break; + case IConversionOperation conversion: + AnalyzeSpanConversion(context, conversion, symbols, allowsSuppression); + break; + case IForEachLoopOperation forEachLoop: + AnalyzeForEachLoop(context, forEachLoop, symbols, allowsSuppression); + break; + default: + break; + } + } + } + }); + }); + } + + /// + /// Analyzes a property reference, to detect reads of an element of a write-only parameter. + /// + /// The context to report diagnostics to. + /// The property reference to analyze. + /// The well known symbols used by the analyzer. + /// Whether reads preceded by a covering write can be suppressed. + /// + /// This handles reading the value (or a readonly reference) of an element, such as: + /// + /// _ = span[i]; + /// Foo(span[i]); + /// ref readonly var x = ref span[i]; + /// Foo(in span[i]); + /// span[i]++; + /// span[i] += 1; + /// + /// + private static void AnalyzeElementRead( + OperationBlockAnalysisContext context, + IPropertyReferenceOperation operation, + WellKnownSymbols symbols, + bool allowsSuppression) + { + // We only care about the 'Span' indexer (i.e. 'span[i]'), not other properties (e.g. 'Length') + if (!operation.Property.IsIndexer) + { + return; + } + + // The indexer must be invoked directly on a write-only 'Span' parameter + if (operation.Instance is not IParameterReferenceOperation { Parameter: { } parameter } || + !IsFillArrayParameter(parameter, symbols)) + { + return; + } + + // Skip usages that only write to the element, which are valid for a fill array + if (IsWriteOnlyElementUsage(operation)) + { + return; + } + + // Skip reads of an element that the method is guaranteed to have already written to + if (allowsSuppression && + operation.Arguments is [{ Value: { } index }] && + IsPrecededByCoveringWrite(operation, ReadTarget.ForElement(parameter, symbols, index))) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.WriteOnlySpanParameterRead, + operation.Syntax.GetLocation(), + parameter.Name)); + } + + /// + /// Analyzes a conversion, to detect a write-only parameter being converted to + /// , which would allow reading all of its elements. + /// + /// The context to report diagnostics to. + /// The conversion to analyze. + /// The well known symbols used by the analyzer. + /// Whether reads preceded by a covering write can be suppressed. + /// + /// This handles converting the span to , such as: + /// + /// ReadOnlySpan<int> readOnlySpan = span; + /// Foo((ReadOnlySpan<int>)span); + /// + /// + private static void AnalyzeSpanConversion( + OperationBlockAnalysisContext context, + IConversionOperation operation, + WellKnownSymbols symbols, + bool allowsSuppression) + { + // The conversion must target 'ReadOnlySpan' (a read-only view over the span elements) + if (!SymbolEqualityComparer.Default.Equals(operation.Type?.OriginalDefinition, symbols.ReadOnlySpanType)) + { + return; + } + + // The operand must be a write-only 'Span' parameter + if (operation.Operand is not IParameterReferenceOperation { Parameter: { } parameter } || + !IsFillArrayParameter(parameter, symbols)) + { + return; + } + + // Skip conversions of a span that the method is guaranteed to have already filled entirely + if (allowsSuppression && IsPrecededByCoveringWrite(operation, ReadTarget.ForSpan(parameter, symbols))) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.WriteOnlySpanParameterRead, + operation.Syntax.GetLocation(), + parameter.Name)); + } + + /// + /// Analyzes a foreach loop, to detect iteration over a write-only parameter. + /// + /// The context to report diagnostics to. + /// The foreach loop to analyze. + /// The well known symbols used by the analyzer. + /// Whether reads preceded by a covering write can be suppressed. + /// + /// This handles iterating over the span, which reads each element, such as: + /// + /// foreach (int x in span) + /// { + /// } + /// + /// It also handles reading the current element through a writable ref loop variable, such as: + /// + /// foreach (ref int x in span) + /// { + /// int y = x; + /// } + /// + /// + private static void AnalyzeForEachLoop( + OperationBlockAnalysisContext context, + IForEachLoopOperation operation, + WellKnownSymbols symbols, + bool allowsSuppression) + { + // The iterated collection is the span parameter, possibly wrapped in an identity conversion + IOperation collection = operation.Collection is IConversionOperation { Operand: { } operand } + ? operand + : operation.Collection; + + // The iterated collection must be a write-only 'Span' parameter + if (collection is not IParameterReferenceOperation { Parameter: { } parameter } || + !IsFillArrayParameter(parameter, symbols)) + { + return; + } + + // Iterating with a writable 'ref' loop variable may be used to fill the span, so the loop + // itself is valid. In that case, only the individual reads through the loop variable (which + // aliases the current element) are reported. By-value and 'ref readonly' loop variables can + // only ever read the elements, so for those the loop itself is reported instead. + if (operation.LoopControlVariable is IVariableDeclaratorOperation { Symbol: { RefKind: RefKind.Ref } loopVariable }) + { + AnalyzeLoopVariableReads(context, operation.Body, ReadTarget.ForAlias(parameter, symbols, loopVariable), allowsSuppression); + + return; + } + + // Skip iterations over a span that the method is guaranteed to have already filled entirely + if (allowsSuppression && IsPrecededByCoveringWrite(operation.Collection, ReadTarget.ForSpan(parameter, symbols))) + { + return; + } + + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.WriteOnlySpanParameterRead, + operation.Collection.Syntax.GetLocation(), + parameter.Name)); + } + + /// + /// Analyzes the body of a foreach loop iterating over a write-only parameter + /// with a writable ref loop variable, to detect reads of the current element through that variable. + /// + /// The context to report diagnostics to. + /// The body of the foreach loop declaring the loop variable. + /// The element aliased by the writable ref loop variable of the loop. + /// Whether reads preceded by a covering write can be suppressed. + private static void AnalyzeLoopVariableReads( + OperationBlockAnalysisContext context, + IOperation loopBody, + ReadTarget target, + bool allowsSuppression) + { + foreach (IOperation operation in loopBody.DescendantsAndSelf()) + { + // We only care about references to the loop variable, as those alias an element of the span. + // The loop variable can't be captured by a lambda or ref reassigned, so all aliasing usages + // of the current element are guaranteed to appear directly in the body of the loop. + if (operation is not ILocalReferenceOperation { Local: { } local } || + !SymbolEqualityComparer.Default.Equals(local, target.Alias)) + { + continue; + } + + // Skip usages that only write to the element, which are valid for a fill array + if (IsWriteOnlyElementUsage(operation)) + { + continue; + } + + // Skip reads of an element that the method is guaranteed to have already written to + if (allowsSuppression && IsPrecededByCoveringWrite(operation, target)) + { + continue; + } + + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.WriteOnlySpanParameterRead, + operation.Syntax.GetLocation(), + target.Parameter.Name)); + } + } + + /// + /// Checks whether a given method has at least one by-value parameter. + /// + /// The method to check. + /// The for . + /// Whether has at least one by-value parameter. + private static bool HasSpanParameter(IMethodSymbol method, INamedTypeSymbol spanType) + { + foreach (IParameterSymbol parameter in method.Parameters) + { + if (IsSpanParameter(parameter, spanType)) + { + return true; + } + } + + return false; + } + + /// + /// Checks whether a given parameter is a by-value parameter. + /// + /// The parameter to check. + /// The for . + /// Whether is a by-value parameter. + private static bool IsSpanParameter(IParameterSymbol parameter, INamedTypeSymbol spanType) + { + // Only a by-value 'System.Span' is projected as a fill array (a 'ref', 'in' or 'out' + // variant is not a valid Windows Runtime parameter, and 'ReadOnlySpan' is a pass array). + return + parameter.RefKind is RefKind.None && + SymbolEqualityComparer.Default.Equals(parameter.Type.OriginalDefinition, spanType); + } + + /// + /// Checks whether a given parameter is a write-only parameter (i.e. a parameter + /// that is projected as a fill array in the Windows Runtime ABI). + /// + /// The parameter to check. + /// The well known symbols used by the analyzer. + /// Whether is a write-only fill array parameter. + private static bool IsFillArrayParameter(IParameterSymbol parameter, WellKnownSymbols symbols) + { + // The parameter must belong to a method that is projected to the Windows Runtime. This also filters + // out the parameters of nested lambdas and local functions, which are never projected (and could not + // reference the parameters of the enclosing method anyway, as a span cannot be captured by them). + return + IsSpanParameter(parameter, symbols.SpanType) && + parameter.ContainingSymbol is IMethodSymbol method && + IsWindowsRuntimeMethod(method); + } + + /// + /// Checks whether a given method is part of the Windows Runtime ABI surface of an authored component. + /// + /// The method to check. + /// Whether is projected to the Windows Runtime. + private static bool IsWindowsRuntimeMethod(IMethodSymbol method) + { + // The method must be an ordinary method, a constructor, or an explicit interface + // implementation (and not e.g. a local function, a lambda, an operator or a property accessor). + if (method.MethodKind is not (MethodKind.Ordinary or MethodKind.Constructor or MethodKind.ExplicitInterfaceImplementation)) + { + return false; + } + + // The containing type must be a public, top-level class (i.e. an authored runtime class). Other type + // kinds can't have method bodies with fill array parameters, and nested types are never projected. + if (method.ContainingType is not { TypeKind: TypeKind.Class, DeclaredAccessibility: Accessibility.Public, ContainingType: null }) + { + return false; + } + + // Public methods (including overrides and implicit interface implementations) are part of the ABI surface + if (method.DeclaredAccessibility is Accessibility.Public) + { + return true; + } + + // Explicit interface implementations are also part of the ABI surface, through the interfaces they + // implement. Only public interfaces are considered, as those are the ones projected to the Windows Runtime. + foreach (IMethodSymbol implementedMethod in method.ExplicitInterfaceImplementations) + { + if (implementedMethod.ContainingType.DeclaredAccessibility is Accessibility.Public) + { + return true; + } + } + + return false; + } + + /// + /// Checks whether a given reference to an element of a write-only parameter is + /// only used to write to that element (which is valid for a fill array), as opposed to also reading its value. + /// + /// The operation referencing the element (either an indexer access, or a ref alias). + /// Whether is a write-only usage of the element. + private static bool IsWriteOnlyElementUsage(IOperation operation) + { + // The examples below use an indexer access for the element reference, but the same reasoning + // applies verbatim to a 'ref' alias to the element (e.g. a writable 'foreach' loop variable). + return operation.Parent switch + { + // 'span[i] = value' (the element is the target of the assignment): this only writes to the element. + // Note that a compound assignment ('span[i] += value') is not handled here, as it also reads. + ISimpleAssignmentOperation simpleAssignment => ReferenceEquals(simpleAssignment.Target, operation), + + // 'Foo(out span[i])' or 'Foo(ref span[i])': the callee may write to the element, and we can't tell + // whether it also reads it, so we conservatively treat these as write-only to avoid false positives. + IArgumentOperation { Parameter.RefKind: RefKind.Out or RefKind.Ref } => true, + + // 'ref var x = ref span[i]' (a writable 'ref' alias to the element): the alias may be used to write + // to the element, so we also conservatively treat it as write-only (a 'ref readonly' alias cannot). + IVariableInitializerOperation { Parent: IVariableDeclaratorOperation { Symbol.RefKind: RefKind.Ref } } => true, + + // Any other usage reads the value (e.g. as a value, a 'ref readonly' alias, an 'in' argument, a + // compound assignment, or an increment/decrement), which is not valid for a write-only fill array. + _ => false + }; + } + + /// + /// Checks whether a given read from a write-only parameter is preceded by a write + /// that is guaranteed to run before it, and that covers the same location. Such a read only ever observes values + /// that the method itself has just written, so it is valid even though the parameter is a fill array. + /// + /// The operation performing the read. + /// The location that accesses. + /// Whether is preceded by a write covering . + /// + /// Only writes appearing in a statement that precedes (an ancestor of) in some enclosing + /// block are considered, and only when they are unconditionally reached within that statement. This is meant to + /// be conservative: it is fine to miss a write that does happen, but a write must never be reported as guaranteed + /// unless it necessarily runs first, as that would silently hide an actual read of an uninitialized element. + /// + private static bool IsPrecededByCoveringWrite(IOperation read, ReadTarget target) + { + IOperation current = read; + IOperation? parent = read.Parent; + + while (parent is not null) + { + // Never look at the enclosing method when the read is inside a lambda or a local function, as those + // can be invoked at any point. This can't normally be reached, given that a span is a 'ref struct' + // type, and as such it cannot be captured by either of them, but it is cheap to guard against. + if (parent is IAnonymousFunctionOperation or ILocalFunctionOperation) + { + return false; + } + + // Only the statements within a block are ordered with respect to one another. Any other kind of parent + // is just skipped, meaning the search resumes from the closest enclosing statement in the parent block. + if (parent is IBlockOperation block) + { + int index = block.Operations.IndexOf(current); + + for (int i = index - 1; i >= 0; i--) + { + if (IsGuaranteedCoveringWrite(block.Operations[i], target)) + { + // A preceding write only covers the read if the operands it depends on can't have changed in + // between. If they can, no earlier write can be relied upon either, as it would span an even + // wider range of statements, so there is no point in continuing the search past this one. + return AreOperandsStable(block.Operations, i, index, target); + } + } + } + + current = parent; + parent = parent.Parent; + } + + return false; + } + + /// + /// Checks whether a given operation contains a write covering a target location that is guaranteed to be reached. + /// + /// The operation to inspect. + /// The location that the write should cover. + /// Whether necessarily performs a write covering . + private static bool IsGuaranteedCoveringWrite(IOperation operation, ReadTarget target) + { + // Constructs that might not run at all can never guarantee that a write nested in them will happen + if (IsConditionallyExecuted(operation, target.Symbols)) + { + return false; + } + + if (IsCoveringWrite(operation, target)) + { + return true; + } + + foreach (IOperation child in operation.ChildOperations) + { + if (IsGuaranteedCoveringWrite(child, target)) + { + return true; + } + } + + return false; + } + + /// + /// Checks whether a given operation is a write covering a target location. + /// + /// The operation to inspect. + /// The location that the write should cover. + /// Whether writes to all of . + private static bool IsCoveringWrite(IOperation operation, ReadTarget target) + { + // 'span.Clear()' and 'span.Fill(value)' initialize every element of the span, so + // they cover any subsequent read from it, no matter which elements it looks at. + if (operation is IInvocationOperation { TargetMethod: { Name: "Clear", Parameters: [] } or { Name: "Fill", Parameters: [_] } } invocation && + SymbolEqualityComparer.Default.Equals(invocation.TargetMethod.ContainingType.OriginalDefinition, target.Symbols.SpanType) && + invocation.Instance is { } instance && + IsReferenceTo(instance, target.Parameter)) + { + return true; + } + + // 'span[i] = value' and 'Foo(out span[i])' initialize a single element, so they only + // cover reads of that same element (i.e. reads through an equivalent index expression). + if (target.Index is { } index && + operation is IPropertyReferenceOperation { Property.IsIndexer: true, Instance: { } spanInstance, Arguments: [{ Value: { } writeIndex }] } && + IsReferenceTo(spanInstance, target.Parameter) && + IsDefiniteWrite(operation) && + AreIndexesEquivalent(writeIndex, index)) + { + return true; + } + + // 'x = value' and 'Foo(out x)' through a writable 'ref' 'foreach' loop variable initialize + // the element it aliases, so they cover reads of that element through the same variable. + return + target.Alias is { } alias && + IsReferenceTo(operation, alias) && + IsDefiniteWrite(operation); + } + + /// + /// Checks whether a given operation might not be reached during the execution of its parent operation. + /// + /// The operation to check. + /// The well known symbols used by the analyzer. + /// Whether might be skipped, or run at some later point. + private static bool IsConditionallyExecuted(IOperation operation, WellKnownSymbols symbols) + { + return (operation is IInvocationOperation { TargetMethod: { } targetMethod } && IsErasableCall(targetMethod, symbols)) || + operation is + IConditionalOperation or // 'if' statements and '?:' expressions + IConditionalAccessOperation or // '?.' and '?[]' accesses + ICoalesceOperation or // '??' expressions + ICoalesceAssignmentOperation or // '??=' expressions + ISwitchOperation or // 'switch' statements + ISwitchExpressionOperation or // 'switch' expressions + ILoopOperation or // all loops, as the body might never run + ITryOperation or // 'try' blocks, as an exception might skip the rest of the body + IAnonymousFunctionOperation or // lambdas and anonymous methods + ILocalFunctionOperation or // local functions + IBinaryOperation { OperatorKind: BinaryOperatorKind.ConditionalAnd or BinaryOperatorKind.ConditionalOr } or + + // Interpolated strings using a handler that can report failure, as the holes are not evaluated then + IInterpolatedStringHandlerCreationOperation { HandlerCreationHasSuccessParameter: true } or + IInterpolatedStringHandlerCreationOperation { HandlerAppendCallsReturnBool: true }; + } + + /// + /// Checks whether calls to a given method might be removed entirely by the compiler, arguments included. + /// + /// The method to check. + /// The well known symbols used by the analyzer. + /// Whether calls to might not survive compilation. + private static bool IsErasableCall(IMethodSymbol method, WellKnownSymbols symbols) + { + // A 'partial void' method that has no implementing declaration has all of its call sites removed + if (method is { IsPartialDefinition: true, PartialImplementationPart: null }) + { + return true; + } + + if (symbols.ConditionalAttributeType is not { } conditionalAttributeType) + { + return false; + } + + // A call to a method annotated with '[Conditional]' is removed when the associated preprocessor symbol + // is not defined. An override cannot carry the attribute itself, but it does inherit the conditional + // symbols of the method it overrides, so the whole chain of overridden methods is inspected here. + for (IMethodSymbol? currentMethod = method; currentMethod is not null; currentMethod = currentMethod.OverriddenMethod) + { + if (currentMethod.HasAttributeWithType(conditionalAttributeType)) + { + return true; + } + } + + return false; + } + + /// + /// Checks whether the operands a covering write depends on (i.e. the span parameter itself, and the index + /// variable, if the target is a single element) can't have been modified between that write and a later read. + /// + /// The statements of the block containing both the write and the read. + /// The index of the statement containing the write. + /// The index of the statement containing the read. + /// The location that the write covers. + /// Whether the write still applies to when the read is reached. + private static bool AreOperandsStable(ImmutableArray statements, int writeIndex, int readIndex, ReadTarget target) + { + ISymbol? indexSymbol = target.IndexSymbol; + + // An index that is itself a 'ref' local or parameter aliases some other storage (e.g. a field, or an + // array element), so its value can change without any write to the symbol itself being visible here + if (indexSymbol is ILocalSymbol { RefKind: not RefKind.None } or IParameterSymbol { RefKind: not RefKind.None }) + { + return false; + } + + // A reference to an operand handed out anywhere in the method (e.g. 'Foo(ref i)') can outlive the call + // that receives it, so it might be used to modify that operand from a place the scan below can't see + if (AreOperandsEscaping(GetOperationBlock(statements[readIndex]), target, indexSymbol)) + { + return false; + } + + for (int i = writeIndex; i <= readIndex; i++) + { + foreach (IOperation operation in statements[i].DescendantsAndSelf()) + { + // Reassigning the span parameter would make the write apply to an entirely different buffer + if (IsPotentialWrite(operation, target.Parameter)) + { + return false; + } + + // Likewise, modifying the index variable would make the two accesses target different elements + if (indexSymbol is not null && IsPotentialWrite(operation, indexSymbol)) + { + return false; + } + } + } + + return true; + } + + /// + /// Checks whether a reference to the operands a covering write depends on is handed out anywhere in an + /// operation block (e.g. as a ref argument), which could be used to modify them at any point. + /// + /// The operation block to inspect. + /// The location the write covers. + /// The local or parameter used as the index, if there is one. + /// Whether a reference to one of the operands might have escaped. + private static bool AreOperandsEscaping(IOperation operationBlock, ReadTarget target, ISymbol? indexSymbol) + { + foreach (IOperation operation in operationBlock.DescendantsAndSelf()) + { + // A reference passed to another method can outlive the call that receives it (e.g. by being stored + // in a 'ref' field, or by being wrapped in a span), so it is not enough to only look at the calls + // between the write and the read: one made before the write can still be used to modify an operand. + if (operation is not IArgumentOperation { Parameter.RefKind: not RefKind.None, Value: { } value }) + { + continue; + } + + if (IsReferenceTo(value, target.Parameter) || + (indexSymbol is not null && IsReferenceTo(value, indexSymbol))) + { + return true; + } + } + + return false; + } + + /// + /// Gets the operation block containing a given operation. + /// + /// The operation to get the operation block for. + /// The root operation containing . + private static IOperation GetOperationBlock(IOperation operation) + { + while (operation.Parent is { } parent) + { + operation = parent; + } + + return operation; + } + + /// + /// Checks whether a given operation is a reference to a local or parameter that might modify its value. + /// + /// The operation to check. + /// The local or parameter to look for. + /// Whether might modify the value of . + private static bool IsPotentialWrite(IOperation operation, ISymbol symbol) + { + return IsReferenceTo(operation, symbol) && operation.Parent switch + { + // 'x = value', 'x += value' and 'x ??= value' + IAssignmentOperation assignment => ReferenceEquals(assignment.Target, operation), + + // 'x++' and 'x--' + IIncrementOrDecrementOperation incrementOrDecrement => ReferenceEquals(incrementOrDecrement.Target, operation), + + // 'Foo(out x)' and 'Foo(ref x)' + IArgumentOperation { Parameter.RefKind: RefKind.Out or RefKind.Ref } => true, + + // 'ref var alias = ref x', which can then be used to write to it + IVariableInitializerOperation { Parent: IVariableDeclaratorOperation { Symbol.RefKind: RefKind.Ref } } => true, + + // '(x, y) = value' (a deconstruction assignment), where the reference is nested in the target tuple + ITupleOperation tuple => IsDeconstructionTarget(tuple), + + _ => false + }; + } + + /// + /// Checks whether a given tuple is (nested in) the target of a deconstruction assignment. + /// + /// The tuple to check. + /// Whether is being assigned to by a deconstruction. + private static bool IsDeconstructionTarget(ITupleOperation tuple) + { + IOperation current = tuple; + + // Deconstructions can be nested (e.g. '((x, y), z) = value'), so walk up all the enclosing tuples + while (current.Parent is ITupleOperation parent) + { + current = parent; + } + + return current.Parent is IDeconstructionAssignmentOperation deconstruction && ReferenceEquals(deconstruction.Target, current); + } + + /// + /// Checks whether a given reference to an element of a write-only parameter is + /// guaranteed to initialize that element, as opposed to just possibly writing to it. + /// + /// The operation referencing the element (either an indexer access, or a ref alias). + /// Whether necessarily writes to the element. + private static bool IsDefiniteWrite(IOperation operation) + { + return operation.Parent switch + { + // 'span[i] = value' or 'x = value': the element is definitely assigned + ISimpleAssignmentOperation simpleAssignment => ReferenceEquals(simpleAssignment.Target, operation), + + // 'Foo(out span[i])' or 'Foo(out x)': the callee has to assign the element before returning + IArgumentOperation { Parameter.RefKind: RefKind.Out } => true, + + // Note that 'ref' arguments and writable 'ref' aliases are not definite writes: the callee (or the + // code using the alias) might never actually write to the element, so they can't be relied upon. + _ => false + }; + } + + /// + /// Checks whether a given operation is a reference to a specific local or parameter. + /// + /// The operation to check. + /// The local or parameter to look for. + /// Whether is a reference to . + private static bool IsReferenceTo(IOperation operation, ISymbol symbol) + { + return operation switch + { + ILocalReferenceOperation localReference => SymbolEqualityComparer.Default.Equals(localReference.Local, symbol), + IParameterReferenceOperation parameterReference => SymbolEqualityComparer.Default.Equals(parameterReference.Parameter, symbol), + _ => false + }; + } + + /// + /// Checks whether two index expressions are guaranteed to produce the same value. + /// + /// The first index expression to compare. + /// The second index expression to compare. + /// Whether and necessarily refer to the same element. + /// + /// Two index expressions referring to the same variable are only equivalent as long as that variable is not + /// modified in between. It is up to callers to validate that (see ). + /// + private static bool AreIndexesEquivalent(IOperation left, IOperation right) + { + // Constant indices are equivalent when they have the same value (e.g. 'span[0]' and 'span[0]') + if (left.ConstantValue is { HasValue: true, Value: { } leftValue } && + right.ConstantValue is { HasValue: true, Value: { } rightValue }) + { + return leftValue.Equals(rightValue); + } + + // Otherwise, only a reference to the same local or parameter is recognized (e.g. 'span[i]' in a loop) + return (UnwrapImplicitConversions(left), UnwrapImplicitConversions(right)) switch + { + (ILocalReferenceOperation leftLocal, ILocalReferenceOperation rightLocal) => SymbolEqualityComparer.Default.Equals(leftLocal.Local, rightLocal.Local), + (IParameterReferenceOperation leftParameter, IParameterReferenceOperation rightParameter) => SymbolEqualityComparer.Default.Equals(leftParameter.Parameter, rightParameter.Parameter), + _ => false + }; + } + + /// + /// Unwraps all compiler inserted conversions from a given operation (e.g. the widening conversion in span[byteIndex]). + /// + /// The operation to unwrap. + /// The innermost operand of that is not an implicit conversion. + private static IOperation? UnwrapImplicitConversions(IOperation? operation) + { + while (operation is IConversionOperation { IsImplicit: true } conversion) + { + operation = conversion.Operand; + } + + return operation; + } + + /// + /// Checks whether reads in a given operation block can be skipped when they are preceded by a covering write. + /// + /// The operation block to inspect. + /// Whether preceding writes can be relied upon for reads in . + /// + /// Recognizing a preceding write relies on being able to see every write to the span parameter and to the + /// index variables in source order. That is not possible when the body can jump around, or when it creates + /// an indirection that could be used to modify a variable from a place the ordered scan can't account for. + /// + private static bool AllowsSuppression(IOperation operationBlock) + { + foreach (IOperation operation in operationBlock.DescendantsAndSelf()) + { + switch (operation) + { + // A 'goto' can jump over a write that would otherwise always run before a given read + case IBranchOperation { BranchKind: BranchKind.GoTo }: + + // A lambda or a local function can capture a variable and modify it when invoked, which + // can happen at any point (a span itself can never be captured, as it is a 'ref struct') + case IAnonymousFunctionOperation or ILocalFunctionOperation: + + // A pointer can be used to modify a variable without any visible write to it + case { Type: IPointerTypeSymbol }: + + // A writable 'ref' alias, whether declared or reassigned, can be used to modify whatever + // it points at from anywhere it is in scope. Note that the loop variable of a 'foreach' + // loop is not a concern here, as its declarator does not have an initializer. + case IVariableDeclaratorOperation { Symbol.RefKind: RefKind.Ref, Initializer: not null }: + case ISimpleAssignmentOperation { IsRef: true }: + return false; + default: + break; + } + } + + return true; + } + + /// + /// The well known symbols used by the analyzer. + /// + /// The for . + /// The for . + /// The for [Conditional], if available. + private readonly struct WellKnownSymbols( + INamedTypeSymbol spanType, + INamedTypeSymbol readOnlySpanType, + INamedTypeSymbol? conditionalAttributeType) + { + /// + /// Gets the for . + /// + public INamedTypeSymbol SpanType => spanType; + + /// + /// Gets the for . + /// + public INamedTypeSymbol ReadOnlySpanType => readOnlySpanType; + + /// + /// Gets the for [Conditional], if available. + /// + public INamedTypeSymbol? ConditionalAttributeType => conditionalAttributeType; + } + + /// + /// Describes the location that a read from a write-only parameter accesses, so + /// that writes which are guaranteed to have already initialized that same location can be recognized. + /// + private readonly struct ReadTarget + { + /// + /// Creates a new instance with the specified parameters. + /// + /// The write-only parameter being read from. + /// The well known symbols used by the analyzer. + /// The index of the element being read, if the read goes through the span indexer. + /// The writable ref foreach loop variable aliasing the element being read, if any. + private ReadTarget(IParameterSymbol parameter, WellKnownSymbols symbols, IOperation? index, ILocalSymbol? alias) + { + Parameter = parameter; + Symbols = symbols; + Index = index; + Alias = alias; + } + + /// + /// Gets the write-only parameter being read from. + /// + public IParameterSymbol Parameter { get; } + + /// + /// Gets the well known symbols used by the analyzer. + /// + public WellKnownSymbols Symbols { get; } + + /// + /// Gets the index of the element being read, if the read goes through the span indexer. + /// + public IOperation? Index { get; } + + /// + /// Gets the writable ref foreach loop variable aliasing the element being read, if any. + /// + public ILocalSymbol? Alias { get; } + + /// + /// Gets the local or parameter that refers to, if it is a plain variable reference. + /// + public ISymbol? IndexSymbol => UnwrapImplicitConversions(Index) switch + { + ILocalReferenceOperation localReference => localReference.Local, + IParameterReferenceOperation parameterReference => parameterReference.Parameter, + _ => null + }; + + /// + /// Creates a for a read of all the elements of a span (e.g. a foreach loop). + /// + /// The write-only parameter being read from. + /// The well known symbols used by the analyzer. + /// The resulting value. + public static ReadTarget ForSpan(IParameterSymbol parameter, WellKnownSymbols symbols) + { + return new(parameter, symbols, index: null, alias: null); + } + + /// + /// Creates a for a read of a single element through the span indexer. + /// + /// The write-only parameter being read from. + /// The well known symbols used by the analyzer. + /// The index of the element being read. + /// The resulting value. + public static ReadTarget ForElement(IParameterSymbol parameter, WellKnownSymbols symbols, IOperation index) + { + return new(parameter, symbols, index, alias: null); + } + + /// + /// Creates a for a read of the element aliased by a writable ref loop variable. + /// + /// The write-only parameter being read from. + /// The well known symbols used by the analyzer. + /// The writable ref foreach loop variable aliasing the element being read. + /// The resulting value. + public static ReadTarget ForAlias(IParameterSymbol parameter, WellKnownSymbols symbols, ILocalSymbol alias) + { + return new(parameter, symbols, index: null, alias); + } + } +} diff --git a/src/Authoring/WinRT.SourceGenerator2/Diagnostics/DiagnosticDescriptors.cs b/src/Authoring/WinRT.SourceGenerator2/Diagnostics/DiagnosticDescriptors.cs index 84567ce6c..908a3daea 100644 --- a/src/Authoring/WinRT.SourceGenerator2/Diagnostics/DiagnosticDescriptors.cs +++ b/src/Authoring/WinRT.SourceGenerator2/Diagnostics/DiagnosticDescriptors.cs @@ -286,4 +286,17 @@ internal static partial class DiagnosticDescriptors description: "A given type should only be used as the target of a single '[WindowsRuntimeNativeExposedType]' attribute in an assembly. CCW marshalling code is only generated once per type, so additional applications of the attribute for the same type have no effect.", helpLinkUri: "https://github.com/microsoft/CsWinRT", customTags: WellKnownDiagnosticTags.CompilationEnd); + + /// + /// Gets a for reading from a write-only parameter on a Windows Runtime method. + /// + public static readonly DiagnosticDescriptor WriteOnlySpanParameterRead = new( + id: "CSWINRT2021", + title: "Reading from a write-only 'Span' parameter", + messageFormat: """The 'Span' parameter '{0}' is projected as a fill array in the Windows Runtime ABI, which is write-only, so the method implementation should only write to it and not read from it""", + category: "WindowsRuntime.SourceGenerator", + defaultSeverity: DiagnosticSeverity.Warning, + isEnabledByDefault: true, + description: "A 'Span' parameter on a Windows Runtime method is projected as a fill array in the ABI, meaning the implementation is only allowed to write to it, not read from it. Reading from such a parameter (e.g. by indexing it for a value or a readonly reference, converting it to 'ReadOnlySpan', or iterating over it) is not supported. Use 'ReadOnlySpan' for a parameter that should be read from instead.", + helpLinkUri: "https://github.com/microsoft/CsWinRT"); } \ No newline at end of file diff --git a/src/Tests/SourceGenerator2Test/Test_WriteOnlySpanParameterAnalyzer.cs b/src/Tests/SourceGenerator2Test/Test_WriteOnlySpanParameterAnalyzer.cs new file mode 100644 index 000000000..94407f511 --- /dev/null +++ b/src/Tests/SourceGenerator2Test/Test_WriteOnlySpanParameterAnalyzer.cs @@ -0,0 +1,1305 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System.Threading.Tasks; +using WindowsRuntime.SourceGenerator.Diagnostics; +using WindowsRuntime.SourceGenerator.Tests.Helpers; + +namespace WindowsRuntime.SourceGenerator.Tests; + +using VerifyCS = CSharpAnalyzerTest; + +/// +/// Tests for . +/// +[TestClass] +public sealed class Test_WriteOnlySpanParameterAnalyzer +{ + // --- Tests where the analyzer should NOT warn --- + + [TestMethod] + public async Task WriteOnlyUsages_DoNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + span[0] = 1; // Writing to an element is valid + int length = span.Length; // Reading non-indexer properties is valid + bool empty = span.IsEmpty; + Out(out span[1]); // Writing through an 'out' argument is valid + Ref(ref span[2]); // Passing by 'ref' may write, so it is allowed + ref int slot = ref span[3]; // A writable 'ref' alias may write, so it is allowed + slot = 4; + + foreach (ref int x in span) // A writable 'ref' loop variable may write + { + x = 5; + } + } + + private static void Out(out int x) => x = 0; + + private static void Ref(ref int x) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ForEachWithWritableRefVariable_DoesNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + // A writable 'ref' loop variable can be used to write to all elements sequentially + foreach (ref var x in span) + { + } + + foreach (ref int y in span) + { + y = 1; + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadOnlySpanParameter_DoesNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Read(ReadOnlySpan span) + { + int x = span[0]; + ReadOnlySpan other = span; + + foreach (int y in span) + { + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task NotComponent_DoesNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + int x = span[0]; + ReadOnlySpan other = span; + + foreach (int y in span) + { + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source); + } + + [TestMethod] + public async Task PrivateMethod_DoesNotWarn() + { + const string source = """ + using System; + + public class Sample + { + private void Fill(Span span) + { + int x = span[0]; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task InternalClass_DoesNotWarn() + { + const string source = """ + using System; + + internal class Sample + { + public void Fill(Span span) + { + int x = span[0]; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task NestedClass_DoesNotWarn() + { + const string source = """ + using System; + + public class Outer + { + public class Inner + { + public void Fill(Span span) + { + int x = span[0]; + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task Struct_DoesNotWarn() + { + const string source = """ + using System; + + public struct Sample + { + public void Fill(Span span) + { + int x = span[0]; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task LocalFunction_DoesNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Method() + { + static void Fill(Span span) + { + int x = span[0]; + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterClear_DoNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, bool condition) + { + // Every element is initialized, so any subsequent read is fine + span.Clear(); + + int value = span[0]; + In(in span[1]); + ref readonly int readOnlyReference = ref span[2]; + ReadOnlySpan readOnlySpan = span; + + Read(span); + + foreach (int x in span) + { + } + + foreach (ref readonly int y in span) + { + } + + foreach (ref int z in span) + { + int nested = z; + } + + if (condition) + { + int inBranch = span[3]; + } + } + + private static void In(in int x) + { + } + + private static void Read(ReadOnlySpan span) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterFill_DoNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + span.Fill(42); + + int value = span[0]; + ReadOnlySpan readOnlySpan = span; + + foreach (int x in span) + { + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ElementReadsAfterWriteToSameIndex_DoNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + const int Index = 3; + + span[0] = 1; + int value = span[0]; // Reading back an element that was just written is fine + In(in span[0]); + RefReadOnly(in span[0]); + ref readonly int readOnlyReference = ref span[0]; + span[0]++; + span[0] += 1; + + Out(out span[1]); // An 'out' argument is also a definite write + int other = span[1]; + + span[Index] = 2; // Constant indices are compared by value + int constant = span[3]; + + for (int i = 0; i < span.Length; i++) + { + span[i] = i; + int inLoop = span[i]; + } + } + + private static void In(in int x) + { + } + + private static void RefReadOnly(ref readonly int x) + { + } + + private static void Out(out int x) => x = 0; + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task LoopVariableReadsAfterWrite_DoNotWarn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, bool condition) + { + foreach (ref int x in span) + { + x = 0; // The element is initialized through the alias + + int value = x; + Value(x); + In(in x); + RefReadOnly(in x); + ref readonly int readOnlyReference = ref x; + x++; + x += 1; + + if (condition) + { + Value(x); // Nested reads are fine as well + } + } + + foreach (ref int y in span) + { + Out(out y); // An 'out' argument is also a definite write + + int value = y; + } + } + + private static void Value(int x) + { + } + + private static void In(in int x) + { + } + + private static void RefReadOnly(ref readonly int x) + { + } + + private static void Out(out int x) => x = 0; + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + // --- Tests where the analyzer SHOULD warn --- + [TestMethod] + public async Task IndexerReads_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + int value = {|CSWINRT2021:span[0]|}; + Value({|CSWINRT2021:span[1]|}); + ref readonly int readOnlyReference = ref {|CSWINRT2021:span[2]|}; + In(in {|CSWINRT2021:span[3]|}); + {|CSWINRT2021:span[4]|}++; + {|CSWINRT2021:span[5]|} += 1; + } + + private static void Value(int x) + { + } + + private static void In(in int x) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadOnlySpanConversions_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + ReadOnlySpan implicitConversion = {|CSWINRT2021:span|}; + ReadOnlySpan explicitConversion = {|CSWINRT2021:(ReadOnlySpan)span|}; + + Read({|CSWINRT2021:span|}); + } + + private static void Read(ReadOnlySpan span) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ForEachReads_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + foreach (int x in {|CSWINRT2021:span|}) + { + } + + foreach (ref readonly int y in {|CSWINRT2021:span|}) + { + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ForEachWritableRefVariableReads_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + foreach (ref int x in span) + { + int value = {|CSWINRT2021:x|}; + Value({|CSWINRT2021:x|}); + In(in {|CSWINRT2021:x|}); + RefReadOnly(in {|CSWINRT2021:x|}); + ref readonly int readOnlyReference = ref {|CSWINRT2021:x|}; + {|CSWINRT2021:x|}++; + {|CSWINRT2021:x|} += 1; + + if (value > 0) + { + Value({|CSWINRT2021:x|}); // Nested reads are detected as well + } + + x = 0; // Writing through the alias is valid + Out(out x); // Writing through an 'out' argument is valid + Ref(ref x); // Passing by 'ref' may write, so it is allowed + ref int slot = ref x; // A writable 'ref' alias may write, so it is allowed + slot = 1; + } + } + + private static void Value(int x) + { + } + + private static void In(in int x) + { + } + + private static void RefReadOnly(ref readonly int x) + { + } + + private static void Out(out int x) => x = 0; + + private static void Ref(ref int x) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task StaticMethod_Warns() + { + const string source = """ + using System; + + public class Sample + { + public static void Fill(Span span) + { + int x = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task Constructor_Warns() + { + const string source = """ + using System; + + public class Sample + { + public Sample(Span span) + { + int x = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ExplicitInterfaceImplementation_Warns() + { + const string source = """ + using System; + + public interface IFillable + { + void Fill(Span span); + } + + public class Sample : IFillable + { + void IFillable.Fill(Span span) + { + int x = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsBeforeWrite_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + // The write only covers reads that come after it + int value = {|CSWINRT2021:span[0]|}; + + span[0] = 1; + span.Clear(); + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ElementReadsAfterWriteToOtherIndex_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, int index) + { + span[0] = 1; + int value = {|CSWINRT2021:span[1]|}; // A different constant index + + span[index] = 2; + int other = {|CSWINRT2021:span[index + 1]|}; // Not provably the same index + + // A single element being initialized says nothing about all the other ones + ReadOnlySpan readOnlySpan = {|CSWINRT2021:span|}; + + foreach (int x in {|CSWINRT2021:span|}) + { + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ElementReadsAfterModifiedIndex_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + int i = 0; + + span[i] = 1; + i++; // The two accesses no longer target the same element + + int value = {|CSWINRT2021:span[i]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterReassignedSpan_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + span.Clear(); + span = default; // The write applies to a different buffer + + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterConditionalWrite_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, bool condition) + { + if (condition) + { + span.Clear(); + } + + int value = {|CSWINRT2021:span[0]|}; + + while (condition) + { + span.Fill(1); // A loop body might never run + } + + int other = {|CSWINRT2021:span[1]|}; + + try + { + span.Clear(); // An exception might skip the rest of the block + } + catch (Exception) + { + } + + int last = {|CSWINRT2021:span[2]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterNonDefiniteWrite_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + Ref(ref span[0]); // The callee might never write to the element + int value = {|CSWINRT2021:span[0]|}; + + ref int slot = ref span[1]; // The alias might never be written to + int other = {|CSWINRT2021:span[1]|}; + + foreach (ref int x in span) + { + Ref(ref x); + + int nested = {|CSWINRT2021:x|}; + } + } + + private static void Ref(ref int x) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterWriteToOtherSpan_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, Span other) + { + other.Clear(); + other[0] = 1; + + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterWriteSkippedByGoto_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, bool condition) + { + if (condition) + { + goto Read; + } + + span.Clear(); + + Read: + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task LoopVariableReadsBeforeWrite_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, bool condition) + { + foreach (ref int x in span) + { + int value = {|CSWINRT2021:x|}; // The write only covers later reads + + x = 0; + } + + foreach (ref int y in span) + { + if (condition) + { + y = 0; // The write might not happen + } + + int value = {|CSWINRT2021:y|}; + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterWriteInSwitchSection_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, int selector) + { + switch (selector) + { + case 0: + span.Clear(); + break; + default: + span.Fill(1); + break; + } + + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterShortCircuitedWrite_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span, bool condition) + { + // The write is only reached when the left operand is 'true' + bool result = condition && Set(out span[0]); + + int value = {|CSWINRT2021:span[0]|}; + } + + private static bool Set(out int x) + { + x = 0; + + return true; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterExtensionMethodCall_Warn() + { + const string source = """ + using System; + + public static class SpanExtensions + { + public static void Clear(this Span span, bool flag) + { + } + } + + public class Sample + { + public void Fill(Span span) + { + span.Clear(true); // Not the 'Span.Clear()' method + + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task PrimaryConstructorInitializers_Warn() + { + const string source = """ + using System; + + public class Sample(Span span) + { + private int _field = {|CSWINRT2021:span[0]|}; + + private int Property { get; } = {|CSWINRT2021:span[1]|}; + + private int _converted = Read({|CSWINRT2021:span|}); + + private static int Read(ReadOnlySpan span) => span.Length; + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterDeconstructedIndex_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + int i = 0; + + span[i] = 1; + (i, _) = (5, 0); // A deconstruction also modifies the index + + int value = {|CSWINRT2021:span[i]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsWithAliasedIndexOrSpan_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void FillWithAliasedIndex(Span span) + { + int i = 0; + ref int alias = ref i; // The alias can modify the index from anywhere + + span[i] = 1; + alias = 5; + + int value = {|CSWINRT2021:span[i]|}; + } + + public void FillWithAliasedSpan(Span span, Span other) + { + ref Span alias = ref span; // The alias can redirect the span from anywhere + + span.Clear(); + alias = other; + + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsWithCapturedIndex_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + int i = 0; + + void Advance() => i++; // The capture can modify the index when invoked + + span[i] = 1; + Advance(); + + int value = {|CSWINRT2021:span[i]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterConditionalMethodWrite_Warn() + { + const string source = """ + using System; + using System.Diagnostics; + + public class Sample + { + public void Fill(Span span) + { + // The whole call, arguments included, is removed when 'DEBUG' is not defined + Log(span[0] = 1); + + int value = {|CSWINRT2021:span[0]|}; + } + + [Conditional("DEBUG")] + private static void Log(int x) + { + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterConditionalOverrideWrite_Warn() + { + const string source = """ + using System; + using System.Diagnostics; + + public class Logger + { + [Conditional("DEBUG")] + public virtual void Log(int x) + { + } + } + + public class Sample : Logger + { + // An override inherits the conditional symbols of the method it overrides + public override void Log(int x) + { + } + + public void Fill(Span span) + { + Log(span[0] = 1); + + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsWithIndirectlyAliasedIndex_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void FillWithConditionalAlias(Span span, bool condition) + { + int i = 0; + int j = 0; + ref int alias = ref (condition ? ref i : ref j); + + span[i] = 1; + alias = 5; + + int value = {|CSWINRT2021:span[i]|}; + } + + public void FillWithReturnedAlias(Span span) + { + int i = 0; + ref int alias = ref Identity(ref i); + + span[i] = 1; + alias = 5; + + int value = {|CSWINRT2021:span[i]|}; + } + + public unsafe void FillWithPointer(Span span) + { + int i = 0; + int* pointer = &i; + + span[i] = 1; + *pointer = 5; + + int value = {|CSWINRT2021:span[i]|}; + } + + private static ref int Identity(ref int x) => ref x; + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsWithRefLocalIndex_Warn() + { + const string source = """ + using System; + + public class Sample + { + private int _index; + + public void FillWithFieldIndex(Span span) + { + // The index aliases the field, so it changes with no write to 'i' in sight + ref int i = ref _index; + + span[i] = 1; + _index = 5; + + int value = {|CSWINRT2021:span[i]|}; + } + + public void FillWithReadOnlyFieldIndex(Span span) + { + ref readonly int i = ref _index; + + span[i] = 1; + Reset(); + + int value = {|CSWINRT2021:span[i]|}; + } + + public void FillWithArrayIndex(Span span, int[] indices) + { + ref int i = ref indices[0]; + + span[i] = 1; + indices[0] = 5; + + int value = {|CSWINRT2021:span[i]|}; + } + + private void Reset() => _index = 5; + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsWithEscapingIndex_Warn() + { + const string source = """ + using System; + using System.Runtime.InteropServices; + + public ref struct Cursor + { + private ref int _index; + + public Cursor(ref int index) + { + _index = ref index; + } + + public void Advance() => _index++; + } + + public class Sample + { + public void FillWithStoredIndexReference(Span span) + { + int i = 0; + Cursor cursor = new(ref i); // The reference outlives the call + + span[i] = 1; + cursor.Advance(); + + int value = {|CSWINRT2021:span[i]|}; + } + + public void FillWithSpanOverIndex(Span span) + { + int i = 0; + Span window = MemoryMarshal.CreateSpan(ref i, 1); + + span[i] = 1; + window[0] = 5; + + int value = {|CSWINRT2021:span[i]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterUnimplementedPartialMethodWrite_Warn() + { + const string source = """ + using System; + + public partial class Sample + { + // Calls to a 'partial void' method with no implementing declaration are + // removed entirely by the compiler, the evaluation of arguments included + partial void Log(int x); + + public void Fill(Span span) + { + Log(span[0] = 1); + + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ReadsAfterInterpolatedStringHandlerWrite_Warn() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span destination, Span span) + { + // The handler can report failure, in which case the holes are never evaluated + destination.TryWrite($"{span[0] = 1}", out int written); + + int value = {|CSWINRT2021:span[0]|}; + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } + + [TestMethod] + public async Task ForEachWithWriteInBody_Warns() + { + const string source = """ + using System; + + public class Sample + { + public void Fill(Span span) + { + // The write happens after each element has already been read + foreach (int x in {|CSWINRT2021:span|}) + { + span.Clear(); + } + } + } + """; + + await VerifyCS.VerifyAnalyzerAsync(source, isCsWinRTComponent: true); + } +}