Skip to content

Commit

Permalink
Merge pull request #4726 from NewellClark/issue-33789
Browse files Browse the repository at this point in the history
(2/4) Override Stream ReadAsync/WriteAsync Analyzer
  • Loading branch information
mavasani authored May 18, 2021
2 parents 0f43809 + 4d7967b commit 4bda814
Show file tree
Hide file tree
Showing 20 changed files with 1,940 additions and 0 deletions.
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ CA1840 | Performance | Info | UseEnvironmentMembers, [Documentation](https://doc
CA1841 | Performance | Info | PreferDictionaryContainsMethods, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1841)
CA1842 | Performance | Info | DoNotUseWhenAllOrWaitAllWithSingleArgument, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1842)
CA1843 | Performance | Info | DoNotUseWhenAllOrWaitAllWithSingleArgument, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1843)
CA1844 | Performance | Info | ProvideStreamMemoryBasedAsyncOverrides, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1844)
CA1845 | Performance | Info | UseSpanBasedStringConcat, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1845)

### Removed Rules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1534,6 +1534,16 @@
<value>and all other platforms</value>
<comment>This call site is reachable on: 'windows' 10.0.2000 and later, and all other platforms</comment>
</data>
<data name="ProvideStreamMemoryBasedAsyncOverridesDescription" xml:space="preserve">
<value>To improve performance, override the memory-based async methods when subclassing 'Stream'. Then implement the array-based methods in terms of the memory-based methods.</value>
</data>
<data name="ProvideStreamMemoryBasedAsyncOverridesMessage" xml:space="preserve">
<value>'{0}' overrides array-based '{1}' but does not override memory-based '{2}'. Consider overriding memory-based '{2}' to improve performance.</value>
<comment>0 = type that subclasses Stream directly, 1 = array-based method, 2 = memory-based method</comment>
</data>
<data name="ProvideStreamMemoryBasedAsyncOverridesTitle" xml:space="preserve">
<value>Provide memory-based overrides of async methods when subclassing 'Stream'</value>
</data>
<data name="UseSpanBasedStringConcatDescription" xml:space="preserve">
<value>It is more efficient to use 'AsSpan' and 'string.Concat', instead of 'Substring' and a concatenation operator.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Composition;
using System.Linq;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;

using Resx = Microsoft.NetCore.Analyzers.MicrosoftNetCoreAnalyzersResources;

namespace Microsoft.NetCore.Analyzers.Runtime
{
/// <summary>
/// CA1840: Reports a diagnostic if a class that directly subclasses <see cref="System.IO.Stream"/> overrides
/// <see cref="System.IO.Stream.ReadAsync(byte[], int, int)"/> and/or <see cref="System.IO.Stream.WriteAsync(byte[], int, int)"/>,
/// and does not override the corrasponding memory-based version.
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
public sealed class ProvideStreamMemoryBasedAsyncOverrides : DiagnosticAnalyzer
{
internal const string RuleId = "CA1844";

private static readonly LocalizableString s_localizableTitle = new LocalizableResourceString(nameof(Resx.ProvideStreamMemoryBasedAsyncOverridesTitle), Resx.ResourceManager, typeof(Resx));
private static readonly LocalizableString s_localizableMessage = new LocalizableResourceString(nameof(Resx.ProvideStreamMemoryBasedAsyncOverridesMessage), Resx.ResourceManager, typeof(Resx));
private static readonly LocalizableString s_localizableDescription = new LocalizableResourceString(nameof(Resx.ProvideStreamMemoryBasedAsyncOverridesDescription), Resx.ResourceManager, typeof(Resx));

internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(
RuleId,
s_localizableTitle,
s_localizableMessage,
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
s_localizableDescription,
isPortedFxCopRule: false,
isDataflowRule: false);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(Rule);

private const string ReadAsyncName = nameof(System.IO.Stream.ReadAsync);
private const string WriteAsyncName = nameof(System.IO.Stream.WriteAsync);

public override void Initialize(AnalysisContext context)
{
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.EnableConcurrentExecution();
context.RegisterCompilationStartAction(OnCompilationStart);
}

private static void OnCompilationStart(CompilationStartAnalysisContext context)
{
var compilation = context.Compilation;

if (!TryGetRequiredSymbols(compilation, out RequiredSymbols symbols))
return;

context.RegisterSymbolAction(AnalyzeNamedType, SymbolKind.NamedType);

return;

// Local functions.
void AnalyzeNamedType(SymbolAnalysisContext context)
{
var type = (INamedTypeSymbol)context.Symbol;

// We only report a diagnostic if the type directly subclasses stream. We don't report diagnostics
// if there are any bases in the middle.

if (!symbols.StreamType.Equals(type.BaseType, SymbolEqualityComparer.Default))
return;

// We use 'FirstOrDefault' because there could be multiple overrides for the same method if
// there are compiler errors.

IMethodSymbol? readAsyncArrayOverride = GetOverridingMethodSymbols(type, symbols.ReadAsyncArrayMethod).FirstOrDefault();
IMethodSymbol? readAsyncMemoryOverride = GetOverridingMethodSymbols(type, symbols.ReadAsyncMemoryMethod).FirstOrDefault();
IMethodSymbol? writeAsyncArrayOverride = GetOverridingMethodSymbols(type, symbols.WriteAsyncArrayMethod).FirstOrDefault();
IMethodSymbol? writeAsyncMemoryOverride = GetOverridingMethodSymbols(type, symbols.WriteAsyncMemoryMethod).FirstOrDefault();

// For both ReadAsync and WriteAsync, if the array-based form is overridden and the memory-based
// form is not, we report a diagnostic. We report separate diagnostics for ReadAsync and WriteAsync.

if (readAsyncArrayOverride is not null && readAsyncMemoryOverride is null)
{
var diagnostic = CreateDiagnostic(type, readAsyncArrayOverride, symbols.ReadAsyncMemoryMethod);
context.ReportDiagnostic(diagnostic);
}

if (writeAsyncArrayOverride is not null && writeAsyncMemoryOverride is null)
{
var diagnostic = CreateDiagnostic(type, writeAsyncArrayOverride, symbols.WriteAsyncMemoryMethod);
context.ReportDiagnostic(diagnostic);
}
}

static Diagnostic CreateDiagnostic(INamedTypeSymbol violatingType, IMethodSymbol arrayBasedOverride, IMethodSymbol memoryBasedMethod)
{
RoslynDebug.Assert(arrayBasedOverride.OverriddenMethod is not null);

// We want to underline the name of the violating type in the class declaration. If the violating type
// is a partial class, we underline all partial declarations.

return violatingType.CreateDiagnostic(
Rule,
violatingType.Name,
arrayBasedOverride.Name,
memoryBasedMethod.Name);
}
}

private static bool TryGetRequiredSymbols(Compilation compilation, out RequiredSymbols requiredSymbols)
{
var int32Type = compilation.GetSpecialType(SpecialType.System_Int32);
var byteType = compilation.GetSpecialType(SpecialType.System_Byte);

if (int32Type is null || byteType is null)
{
requiredSymbols = default;
return false;
}

var byteArrayType = compilation.CreateArrayTypeSymbol(byteType);
var memoryOfByteType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemMemory1)?.Construct(byteType);
var readOnlyMemoryOfByteType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemReadOnlyMemory1)?.Construct(byteType);
var streamType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemIOStream);
var cancellationTokenType = compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemThreadingCancellationToken);

if (memoryOfByteType is null || readOnlyMemoryOfByteType is null || streamType is null || cancellationTokenType is null)
{
requiredSymbols = default;
return false;
}

// Even though framework types should never contain compiler errors, we still use 'FirstOrDefault' to avoid having the
// analyzer crash if it runs against the source code for 'System.Private.CoreLib', which could be malformed.

var readAsyncArrayMethod = GetOverloads(streamType, ReadAsyncName, byteArrayType, int32Type, int32Type, cancellationTokenType).FirstOrDefault();
var readAsyncMemoryMethod = GetOverloads(streamType, ReadAsyncName, memoryOfByteType, cancellationTokenType).FirstOrDefault();
var writeAsyncArrayMethod = GetOverloads(streamType, WriteAsyncName, byteArrayType, int32Type, int32Type, cancellationTokenType).FirstOrDefault();
var writeAsyncMemoryMethod = GetOverloads(streamType, WriteAsyncName, readOnlyMemoryOfByteType, cancellationTokenType).FirstOrDefault();

if (readAsyncArrayMethod is null || readAsyncMemoryMethod is null || writeAsyncArrayMethod is null || writeAsyncMemoryMethod is null)
{
requiredSymbols = default;
return false;
}

requiredSymbols = new RequiredSymbols(
streamType, memoryOfByteType, readOnlyMemoryOfByteType,
readAsyncArrayMethod, readAsyncMemoryMethod,
writeAsyncArrayMethod, writeAsyncMemoryMethod);
return true;
}

// There could be more than one overload with an exact match if there are compiler errors.
private static ImmutableArray<IMethodSymbol> GetOverloads(ITypeSymbol containingType, string methodName, params ITypeSymbol[] argumentTypes)
{
return containingType.GetMembers(methodName)
.OfType<IMethodSymbol>()
.WhereAsArray(m => IsMatch(m, argumentTypes));

static bool IsMatch(IMethodSymbol method, ITypeSymbol[] argumentTypes)
{
if (method.Parameters.Length != argumentTypes.Length)
return false;

for (int index = 0; index < argumentTypes.Length; ++index)
{
if (!argumentTypes[index].Equals(method.Parameters[index].Type, SymbolEqualityComparer.Default))
return false;
}

return true;
}
}

/// <summary>
/// If <paramref name="derivedType"/> overrides <paramref name="overriddenMethod"/> on its immediate base class, returns the <see cref="IMethodSymbol"/>
/// for the overriding method. Returns null if <paramref name="derivedType"/> does not override <paramref name="overriddenMethod"/>.
/// </summary>
/// <param name="derivedType">The type that may have overridden a method on its immediate base-class.</param>
/// <param name="overriddenMethod"></param>
/// <returns>The <see cref="IMethodSymbol"/> for the method that overrides <paramref name="overriddenMethod"/>, or null if
/// <paramref name="overriddenMethod"/> is not overridden.</returns>
private static ImmutableArray<IMethodSymbol> GetOverridingMethodSymbols(ITypeSymbol derivedType, IMethodSymbol overriddenMethod)
{
RoslynDebug.Assert(derivedType.BaseType.Equals(overriddenMethod.ContainingType, SymbolEqualityComparer.Default));

return derivedType.GetMembers(overriddenMethod.Name)
.OfType<IMethodSymbol>()
.WhereAsArray(m => m.IsOverride && overriddenMethod.Equals(m.GetOverriddenMember(), SymbolEqualityComparer.Default));
}

// We use a struct instead of a record-type to save on allocations.
// There is no trade-off for doing so because this type is never passed by-value.
// We never do equality operations on instances.
#pragma warning disable CA1815 // Override equals and operator equals on value types
private readonly struct RequiredSymbols
#pragma warning restore CA1815 // Override equals and operator equals on value types
{
public RequiredSymbols(
ITypeSymbol streamType, ITypeSymbol memoryOfByteType, ITypeSymbol readOnlyMemoryOfByteType,
IMethodSymbol readAsyncArrayMethod, IMethodSymbol readAsyncMemoryMethod,
IMethodSymbol writeAsyncArrayMethod, IMethodSymbol writeAsyncMemoryMethod)
{
StreamType = streamType;
MemoryOfByteType = memoryOfByteType;
ReadOnlyMemoryOfByteType = readOnlyMemoryOfByteType;
ReadAsyncArrayMethod = readAsyncArrayMethod;
ReadAsyncMemoryMethod = readAsyncMemoryMethod;
WriteAsyncArrayMethod = writeAsyncArrayMethod;
WriteAsyncMemoryMethod = writeAsyncMemoryMethod;
}

public ITypeSymbol StreamType { get; }
public ITypeSymbol MemoryOfByteType { get; }
public ITypeSymbol ReadOnlyMemoryOfByteType { get; }
public IMethodSymbol ReadAsyncArrayMethod { get; }
public IMethodSymbol ReadAsyncMemoryMethod { get; }
public IMethodSymbol WriteAsyncArrayMethod { get; }
public IMethodSymbol WriteAsyncMemoryMethod { get; }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1787,6 +1787,21 @@
<target state="translated">Poskytujte metody deserializace pro volitelné pole</target>
<note />
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesDescription">
<source>To improve performance, override the memory-based async methods when subclassing 'Stream'. Then implement the array-based methods in terms of the memory-based methods.</source>
<target state="new">To improve performance, override the memory-based async methods when subclassing 'Stream'. Then implement the array-based methods in terms of the memory-based methods.</target>
<note />
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesMessage">
<source>'{0}' overrides array-based '{1}' but does not override memory-based '{2}'. Consider overriding memory-based '{2}' to improve performance.</source>
<target state="new">'{0}' overrides array-based '{1}' but does not override memory-based '{2}'. Consider overriding memory-based '{2}' to improve performance.</target>
<note>0 = type that subclasses Stream directly, 1 = array-based method, 2 = memory-based method</note>
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesTitle">
<source>Provide memory-based overrides of async methods when subclassing 'Stream'</source>
<target state="new">Provide memory-based overrides of async methods when subclassing 'Stream'</target>
<note />
</trans-unit>
<trans-unit id="RemoveRedundantCall">
<source>Remove redundant call</source>
<target state="translated">Odebrat redundantní volání</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1787,6 +1787,21 @@
<target state="translated">Deserialisierungsmethoden für optionale Felder angeben</target>
<note />
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesDescription">
<source>To improve performance, override the memory-based async methods when subclassing 'Stream'. Then implement the array-based methods in terms of the memory-based methods.</source>
<target state="new">To improve performance, override the memory-based async methods when subclassing 'Stream'. Then implement the array-based methods in terms of the memory-based methods.</target>
<note />
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesMessage">
<source>'{0}' overrides array-based '{1}' but does not override memory-based '{2}'. Consider overriding memory-based '{2}' to improve performance.</source>
<target state="new">'{0}' overrides array-based '{1}' but does not override memory-based '{2}'. Consider overriding memory-based '{2}' to improve performance.</target>
<note>0 = type that subclasses Stream directly, 1 = array-based method, 2 = memory-based method</note>
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesTitle">
<source>Provide memory-based overrides of async methods when subclassing 'Stream'</source>
<target state="new">Provide memory-based overrides of async methods when subclassing 'Stream'</target>
<note />
</trans-unit>
<trans-unit id="RemoveRedundantCall">
<source>Remove redundant call</source>
<target state="translated">Redundanten Aufruf entfernen</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1787,6 +1787,21 @@
<target state="translated">Proporcionar métodos de deserialización para campos opcionales</target>
<note />
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesDescription">
<source>To improve performance, override the memory-based async methods when subclassing 'Stream'. Then implement the array-based methods in terms of the memory-based methods.</source>
<target state="new">To improve performance, override the memory-based async methods when subclassing 'Stream'. Then implement the array-based methods in terms of the memory-based methods.</target>
<note />
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesMessage">
<source>'{0}' overrides array-based '{1}' but does not override memory-based '{2}'. Consider overriding memory-based '{2}' to improve performance.</source>
<target state="new">'{0}' overrides array-based '{1}' but does not override memory-based '{2}'. Consider overriding memory-based '{2}' to improve performance.</target>
<note>0 = type that subclasses Stream directly, 1 = array-based method, 2 = memory-based method</note>
</trans-unit>
<trans-unit id="ProvideStreamMemoryBasedAsyncOverridesTitle">
<source>Provide memory-based overrides of async methods when subclassing 'Stream'</source>
<target state="new">Provide memory-based overrides of async methods when subclassing 'Stream'</target>
<note />
</trans-unit>
<trans-unit id="RemoveRedundantCall">
<source>Remove redundant call</source>
<target state="translated">Quitar llamada redundante</target>
Expand Down
Loading

0 comments on commit 4bda814

Please sign in to comment.