Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add the UseConreteType analyzer #6370

Merged
merged 2 commits into from
Jan 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ CA1513 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](http
CA1856 | Performance | Error | ConstantExpectedAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1856)
CA1857 | Performance | Warning | ConstantExpectedAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1857)
CA1858 | Performance | Info | UseStartsWithInsteadOfIndexOfComparisonWithZero, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1858)
CA1859 | Performance | Info | UseConcreteTypeAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1859)

### Removed Rules

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1980,4 +1980,22 @@
<data name="UseThrowHelperFix" xml:space="preserve">
<value>Use '{0}.{1}'</value>
</data>
<data name="UseConcreteTypeDescription" xml:space="preserve">
<value>Using concrete types avoids virtual or interface call overhead and enables inlining.</value>
</data>
<data name="UseConcreteTypeForFieldMessage" xml:space="preserve">
<value>Change type of field '{0}' from '{1}' to '{2}' for improved performance</value>
</data>
<data name="UseConcreteTypeTitle" xml:space="preserve">
<value>Use concrete types when possible for improved performance</value>
</data>
<data name="UseConcreteTypeForLocalMessage" xml:space="preserve">
<value>Change type of variable '{0}' from '{1}' to '{2}' for improved performance</value>
</data>
<data name="UseConcreteTypeForMethodReturnMessage" xml:space="preserve">
<value>Change return type of method '{0}' from '{1}' to '{2}' for improved performance</value>
</data>
<data name="UseConcreteTypeForParameterMessage" xml:space="preserve">
<value>Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</value>
</data>
</root>

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Immutable;
using System.Linq;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Analyzer.Utilities.Lightup;
using Analyzer.Utilities.PooledObjects;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Performance
{
using static MicrosoftNetCoreAnalyzersResources;

// Ideas for the future
// ====================
// Detect arrays/collections of interface types which could be replaced with arrays/collections of concrete types
// Suggest to upgrade members of tuples returned from a method
// only suggest a replacement type if it reduces the number of virtual/interface calls

/// <summary>
/// Identifies locals/fields/parameters/return types which can be switched to a concrete type to eliminate virtual/interface dispatch.
/// </summary>
/// <remarks>
/// First, we collect a bunch of state:
///
/// * For all locals/fields/parameters/returns within the named type, we create bags representing the types having been assigned to each.
/// This state will be used to know if we can 'upgrade' the element's type.
///
/// * For all locals/fields/parameters within the named type, we keep track of when they are used as 'this' for a virtual/interface call.
/// This state will be used to filter out diagnostics for those locals/fields/parameters which aren't inducing virtual/interface calls.
/// There's no sense in upgrading those elements if they aren't the source of virtual/interface calls.
///
/// * We keep track of all methods assigned to delegates so that we don't suggest changing the signature of these methods.
///
/// Once all this state has been collected, we perform the actual analysis:
///
/// * Based on the bags of types being assigned to each local/field/parameter, if there is only one type being assigned and this
/// type is more specialized than what the element's type is, then we suggest upgrading the element's type accordingly.
///
/// * Based on the bags of types being returned by each method, if there is only one type being returned and this type is more specialized
/// than what was there before, then we suggest upgrading the return type accordingly.
///
/// Several constraints are applied before we suggest modifying a method signature (either one of its parameters or its return type):
///
/// * The method cannot be implementing any interface.
///
/// * The method cannot be virtual, abstract, or be an override.
///
/// * The method must be private.
///
/// * The method must not have been assigned to a delegate.
/// </remarks>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed partial class UseConcreteTypeAnalyzer : DiagnosticAnalyzer
{
internal const string RuleId = "CA1859";

internal static readonly DiagnosticDescriptor UseConcreteTypeForMethodReturn = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(UseConcreteTypeTitle)),
CreateLocalizableResourceString(nameof(UseConcreteTypeForMethodReturnMessage)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(UseConcreteTypeDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor UseConcreteTypeForParameter = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(UseConcreteTypeTitle)),
CreateLocalizableResourceString(nameof(UseConcreteTypeForParameterMessage)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(UseConcreteTypeDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor UseConcreteTypeForLocal = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(UseConcreteTypeTitle)),
CreateLocalizableResourceString(nameof(UseConcreteTypeForLocalMessage)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(UseConcreteTypeDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor UseConcreteTypeForField = DiagnosticDescriptorHelper.Create(
RuleId,
CreateLocalizableResourceString(nameof(UseConcreteTypeTitle)),
CreateLocalizableResourceString(nameof(UseConcreteTypeForFieldMessage)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(UseConcreteTypeDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics => ImmutableArray.Create(
UseConcreteTypeForField,
UseConcreteTypeForLocal,
UseConcreteTypeForMethodReturn,
UseConcreteTypeForParameter);

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

context.RegisterSymbolStartAction(context =>
{
var coll = Collector.GetInstance(context.Compilation);

context.RegisterOperationAction(context => coll.HandleInvocation((IInvocationOperation)context.Operation), OperationKind.Invocation);
context.RegisterOperationAction(context => coll.HandleSimpleAssignment((ISimpleAssignmentOperation)context.Operation), OperationKind.SimpleAssignment);
context.RegisterOperationAction(context => coll.HandleCoalesceAssignment((ICoalesceAssignmentOperation)context.Operation), OperationKind.CoalesceAssignment);
context.RegisterOperationAction(context => coll.HandleDeconstructionAssignment((IDeconstructionAssignmentOperation)context.Operation), OperationKind.DeconstructionAssignment);
context.RegisterOperationAction(context => coll.HandleFieldInitializer((IFieldInitializerOperation)context.Operation), OperationKind.FieldInitializer);
context.RegisterOperationAction(context => coll.HandleVariableDeclarator((IVariableDeclaratorOperation)context.Operation), OperationKind.VariableDeclarator);
context.RegisterOperationAction(context => coll.HandleDeclarationExpression((IDeclarationExpressionOperation)context.Operation), OperationKind.DeclarationExpression);
context.RegisterOperationAction(context => coll.HandleReturn((IReturnOperation)context.Operation), OperationKind.Return);

context.RegisterSymbolEndAction(context =>
{
Report(context, coll);
Collector.ReturnInstance(coll, context.CancellationToken);
});
}, SymbolKind.NamedType);
Comment on lines +112 to +130
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
context.RegisterSymbolStartAction(context =>
{
var coll = Collector.GetInstance(context.Compilation);
context.RegisterOperationAction(context => coll.HandleInvocation((IInvocationOperation)context.Operation), OperationKind.Invocation);
context.RegisterOperationAction(context => coll.HandleSimpleAssignment((ISimpleAssignmentOperation)context.Operation), OperationKind.SimpleAssignment);
context.RegisterOperationAction(context => coll.HandleCoalesceAssignment((ICoalesceAssignmentOperation)context.Operation), OperationKind.CoalesceAssignment);
context.RegisterOperationAction(context => coll.HandleDeconstructionAssignment((IDeconstructionAssignmentOperation)context.Operation), OperationKind.DeconstructionAssignment);
context.RegisterOperationAction(context => coll.HandleFieldInitializer((IFieldInitializerOperation)context.Operation), OperationKind.FieldInitializer);
context.RegisterOperationAction(context => coll.HandleVariableDeclarator((IVariableDeclaratorOperation)context.Operation), OperationKind.VariableDeclarator);
context.RegisterOperationAction(context => coll.HandleDeclarationExpression((IDeclarationExpressionOperation)context.Operation), OperationKind.DeclarationExpression);
context.RegisterOperationAction(context => coll.HandleReturn((IReturnOperation)context.Operation), OperationKind.Return);
context.RegisterSymbolEndAction(context =>
{
Report(context, coll);
Collector.ReturnInstance(coll, context.CancellationToken);
});
}, SymbolKind.NamedType);
context.RegisterCompilationStartAction(context =>
{
var voidType = context.Compilation.GetSpecialType(SpecialType.System_Void);
context.RegisterSymbolStartAction(context =>
{
var coll = Collector.GetInstance(context.Compilation);
context.RegisterOperationAction(context => coll.HandleInvocation((IInvocationOperation)context.Operation), OperationKind.Invocation);
context.RegisterOperationAction(context => coll.HandleSimpleAssignment((ISimpleAssignmentOperation)context.Operation), OperationKind.SimpleAssignment);
context.RegisterOperationAction(context => coll.HandleCoalesceAssignment((ICoalesceAssignmentOperation)context.Operation), OperationKind.CoalesceAssignment);
context.RegisterOperationAction(context => coll.HandleDeconstructionAssignment((IDeconstructionAssignmentOperation)context.Operation), OperationKind.DeconstructionAssignment);
context.RegisterOperationAction(context => coll.HandleFieldInitializer((IFieldInitializerOperation)context.Operation), OperationKind.FieldInitializer);
context.RegisterOperationAction(context => coll.HandleVariableDeclarator((IVariableDeclaratorOperation)context.Operation), OperationKind.VariableDeclarator);
context.RegisterOperationAction(context => coll.HandleDeclarationExpression((IDeclarationExpressionOperation)context.Operation), OperationKind.DeclarationExpression);
context.RegisterOperationAction(context => coll.HandleReturn((IReturnOperation)context.Operation), OperationKind.Return);
context.RegisterSymbolEndAction(context =>
{
Report(context, coll, voidType);
Collector.ReturnInstance(coll, context.CancellationToken);
});
}, SymbolKind.NamedType);
}

}

/// <summary>
/// Given all the accumulated analysis state, generate the diagnostics.
/// </summary>
private static void Report(SymbolAnalysisContext context, Collector coll)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static void Report(SymbolAnalysisContext context, Collector coll)
private static void Report(SymbolAnalysisContext context, Collector coll, INamedTypeSymbol voidType)

{
foreach (var field in coll.VirtualDispatchFields.Keys)
{
if (coll.FieldAssignments.TryGetValue(field, out var assignments))
{
Report(field, field.Type, assignments, UseConcreteTypeForField);
}
}

foreach (var local in coll.VirtualDispatchLocals.Keys)
{
if (coll.LocalAssignments.TryGetValue(local, out var assignments))
{
Report(local, local.Type, assignments, UseConcreteTypeForLocal);
}
}

foreach (var parameter in coll.VirtualDispatchParameters.Keys)
{
if (coll.ParameterAssignments.TryGetValue(parameter, out var assignments))
{
if (parameter.ContainingSymbol is IMethodSymbol method)
{
if (CanUpgrade(method))
{
Report(parameter, parameter.Type, assignments, UseConcreteTypeForParameter);
}
}
}
}

foreach (var pair in coll.MethodReturns)
{
var method = pair.Key;
var returns = pair.Value;

if (CanUpgrade(method))
{
Report(method, method.ReturnType, returns, UseConcreteTypeForMethodReturn);
}
}

void Report(ISymbol sym, ITypeSymbol fromType, PooledConcurrentSet<ITypeSymbol> assignments, DiagnosticDescriptor desc)
{
using var types = PooledHashSet<ITypeSymbol>.GetInstance(assignments, SymbolEqualityComparer.Default);

var assignedNull = types.Remove(coll.Void!);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var assignedNull = types.Remove(coll.Void!);
var assignedNull = types.Remove(voidType);


if (types.Count == 1)
{
var toType = types.Single();
if (assignedNull)
{
toType = toType.WithNullableAnnotation(Analyzer.Utilities.Lightup.NullableAnnotation.Annotated);
}

if (!toType.DerivesFrom(fromType.OriginalDefinition))
{
return;
}

if (toType.TypeKind == TypeKind.Class
&& !SymbolEqualityComparer.Default.Equals(fromType, toType)
&& toType.SpecialType != SpecialType.System_Object
&& toType.SpecialType != SpecialType.System_Delegate)
{
var fromTypeName = GetTypeName(fromType);
var toTypeName = GetTypeName(toType);
var diagnostic = sym.CreateDiagnostic(desc, sym.Name, fromTypeName, toTypeName);
context.ReportDiagnostic(diagnostic);
}
}
}

bool CanUpgrade(IMethodSymbol methodSym) => !coll.MethodsAssignedToDelegate.ContainsKey(methodSym);

static string GetTypeName(ITypeSymbol type) => type.ToDisplayString(SymbolDisplayFormat.CSharpErrorMessageFormat);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2662,6 +2662,36 @@
<target state="translated">Použijte ThrowIfCancellationRequested</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeDescription">
<source>Using concrete types avoids virtual or interface call overhead and enables inlining.</source>
<target state="new">Using concrete types avoids virtual or interface call overhead and enables inlining.</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForFieldMessage">
<source>Change type of field '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of field '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForLocalMessage">
<source>Change type of variable '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of variable '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForMethodReturnMessage">
<source>Change return type of method '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change return type of method '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForParameterMessage">
<source>Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeTitle">
<source>Use concrete types when possible for improved performance</source>
<target state="new">Use concrete types when possible for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseContainerLevelAccessPolicy">
<source>Use Container Level Access Policy</source>
<target state="translated">Použít zásady přístupu na úrovni kontejneru</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2662,6 +2662,36 @@
<target state="translated">"ThrowIfCancellationRequested()" verwenden</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeDescription">
<source>Using concrete types avoids virtual or interface call overhead and enables inlining.</source>
<target state="new">Using concrete types avoids virtual or interface call overhead and enables inlining.</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForFieldMessage">
<source>Change type of field '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of field '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForLocalMessage">
<source>Change type of variable '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of variable '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForMethodReturnMessage">
<source>Change return type of method '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change return type of method '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForParameterMessage">
<source>Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeTitle">
<source>Use concrete types when possible for improved performance</source>
<target state="new">Use concrete types when possible for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseContainerLevelAccessPolicy">
<source>Use Container Level Access Policy</source>
<target state="translated">Zugriffsrichtlinie auf Containerebene verwenden</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2662,6 +2662,36 @@
<target state="translated">Usar "ThrowIfCancellationRequested"</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeDescription">
<source>Using concrete types avoids virtual or interface call overhead and enables inlining.</source>
<target state="new">Using concrete types avoids virtual or interface call overhead and enables inlining.</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForFieldMessage">
<source>Change type of field '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of field '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForLocalMessage">
<source>Change type of variable '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of variable '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForMethodReturnMessage">
<source>Change return type of method '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change return type of method '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeForParameterMessage">
<source>Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</source>
<target state="new">Change type of parameter '{0}' from '{1}' to '{2}' for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseConcreteTypeTitle">
<source>Use concrete types when possible for improved performance</source>
<target state="new">Use concrete types when possible for improved performance</target>
<note />
</trans-unit>
<trans-unit id="UseContainerLevelAccessPolicy">
<source>Use Container Level Access Policy</source>
<target state="translated">Usar una directiva de acceso de nivel de contenedor</target>
Expand Down
Loading