-
Notifications
You must be signed in to change notification settings - Fork 463
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
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
470 changes: 470 additions & 0 deletions
470
...alyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.Collector.cs
Large diffs are not rendered by default.
Oops, something went wrong.
216 changes: 216 additions & 0 deletions
216
src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Performance/UseConcreteTypeAnalyzer.cs
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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); | ||||||
} | ||||||
|
||||||
/// <summary> | ||||||
/// Given all the accumulated analysis state, generate the diagnostics. | ||||||
/// </summary> | ||||||
private static void Report(SymbolAnalysisContext context, Collector coll) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
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!); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
||||||
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); | ||||||
} | ||||||
} | ||||||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.