Skip to content

Commit

Permalink
Add analyzer for flagging single-use of local JsonSerializerOptions (#…
Browse files Browse the repository at this point in the history
…6850)

* Add analyzer for flagging single-use of local JsonSerializerOptions

* Add changes made by msbuild /t:pack /v:m

* Amend id CA1865 -> CA1869

* Address feedback

* Update rule description, message and title

* Fix formatting and apply changes from msbuild /t:pack /v:m
  • Loading branch information
jozkee authored Aug 14, 2023
1 parent 755a4f8 commit 2af4cd8
Show file tree
Hide file tree
Showing 22 changed files with 1,106 additions and 2 deletions.
3 changes: 2 additions & 1 deletion src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ Rule ID | Category | Severity | Notes
CA1865 | Performance | Info | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1865)
CA1866 | Performance | Info | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1866)
CA1867 | Performance | Disabled | UseStringMethodCharOverloadWithSingleCharacters, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1867)
CA1868 | Performance | Info | DoNotGuardSetAddOrRemoveByContains, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1865)
CA1868 | Performance | Info | DoNotGuardSetAddOrRemoveByContains, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1868)
CA1869 | Performance | Info | AvoidSingleUseOfLocalJsonSerializerOptions, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/CA1869)
CA2261 | Usage | Warning | DoNotUseConfigureAwaitWithSuppressThrowing, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2250)
CA1510 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1510)
CA1511 | Maintainability | Info | UseExceptionThrowHelpers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1511)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2129,4 +2129,13 @@ Widening and user defined conversions are not supported with generic types.</val
<data name="UseStringMethodCharOverloadWithSingleCharactersTitle" xml:space="preserve">
<value>Use char overload</value>
</data>
<data name="AvoidSingleUseOfLocalJsonSerializerOptionsMessage" xml:space="preserve">
<value>Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead.</value>
</data>
<data name="AvoidSingleUseOfLocalJsonSerializerOptionsDescription" xml:space="preserve">
<value>Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application.</value>
</data>
<data name="AvoidSingleUseOfLocalJsonSerializerOptionsTitle" xml:space="preserve">
<value>Cache and reuse 'JsonSerializerOptions' instances</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,328 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis;
using System.Collections.Immutable;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis.Operations;
using System.Diagnostics.CodeAnalysis;
using System;
using System.Collections.Generic;

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

[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class AvoidSingleUseOfLocalJsonSerializerOptions : DiagnosticAnalyzer
{
internal static readonly DiagnosticDescriptor s_Rule = DiagnosticDescriptorHelper.Create(
id: "CA1869",
title: CreateLocalizableResourceString(nameof(AvoidSingleUseOfLocalJsonSerializerOptionsTitle)),
messageFormat: CreateLocalizableResourceString(nameof(AvoidSingleUseOfLocalJsonSerializerOptionsMessage)),
category: DiagnosticCategory.Performance,
ruleLevel: RuleLevel.IdeSuggestion,
description: CreateLocalizableResourceString(nameof(AvoidSingleUseOfLocalJsonSerializerOptionsDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

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

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

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

compilation.TryGetOrCreateTypeByMetadataName(
WellKnownTypeNames.SystemTextJsonJsonSerializerOptions, out INamedTypeSymbol? jsonSerializerOptionsSymbol);

compilation.TryGetOrCreateTypeByMetadataName(
WellKnownTypeNames.SystemTextJsonJsonSerializer, out INamedTypeSymbol? jsonSerializerSymbol);

if (jsonSerializerOptionsSymbol is null || jsonSerializerSymbol is null)
{
return;
}

context.RegisterOperationAction(
context =>
{
var operation = (IObjectCreationOperation)context.Operation;
INamedTypeSymbol? typeSymbol = operation.Constructor?.ContainingType;
if (SymbolEqualityComparer.Default.Equals(typeSymbol, jsonSerializerOptionsSymbol))
{
if (IsCtorUsedAsArgumentForJsonSerializer(operation, jsonSerializerSymbol) ||
IsLocalUsedAsArgumentForJsonSerializerOnly(operation, jsonSerializerSymbol))
{
context.ReportDiagnostic(operation.CreateDiagnostic(s_Rule));
}
}
},
OperationKind.ObjectCreation);
}

private static bool IsCtorUsedAsArgumentForJsonSerializer(IObjectCreationOperation objCreationOperation, INamedTypeSymbol jsonSerializerSymbol)
{
IOperation operation = WalkUpConditional(objCreationOperation);

return operation.Parent is IArgumentOperation argumentOperation &&
IsArgumentForJsonSerializer(argumentOperation, jsonSerializerSymbol);
}

private static bool IsArgumentForJsonSerializer(IArgumentOperation argumentOperation, INamedTypeSymbol jsonSerializerSymbol)
{
return argumentOperation.Parent is IInvocationOperation invocationOperation &&
SymbolEqualityComparer.Default.Equals(invocationOperation.TargetMethod.ContainingType, jsonSerializerSymbol);
}

private static bool IsLocalUsedAsArgumentForJsonSerializerOnly(IObjectCreationOperation objCreation, INamedTypeSymbol jsonSerializerSymbol)
{
IOperation operation = WalkUpConditional(objCreation);

if (!IsLocalAssignment(operation, out List<ILocalSymbol>? localSymbols))
{
return false;
}

IBlockOperation? localBlock = objCreation.GetFirstParentBlock();
bool isSingleUseJsonSerializerInvocation = false;

foreach (IOperation descendant in localBlock.Descendants())
{
if (descendant is not ILocalReferenceOperation localRefOperation ||
!localSymbols.Contains(localRefOperation.Local))
{
continue;
}

// Avoid cases that would potentially make the local escape current block scope.
if (IsArgumentOfJsonSerializer(descendant, jsonSerializerSymbol, out bool isArgumentOfInvocation))
{
// Case: used more than once i.e: not single-use.
if (isSingleUseJsonSerializerInvocation)
{
return false;
}

isSingleUseJsonSerializerInvocation = true;
}

// Case: passed as argument of a non-JsonSerializer method.
else if (isArgumentOfInvocation)
{
return false;
}

if (IsFieldOrPropertyAssignment(descendant))
{
return false;
}

// Case: deconstruction assignment.
if (IsTupleForDeconstructionTargetingFieldOrProperty(descendant))
{
return false;
}

// Case: local goes into closure.
if (IsClosureOnLambdaOrLocalFunction(descendant, localBlock!))
{
return false;
}
}

return isSingleUseJsonSerializerInvocation;
}

[return: NotNullIfNotNull(nameof(operation))]
private static IOperation? WalkUpConditional(IOperation? operation)
{
if (operation is null)
return null;

while (operation.Parent is IConditionalOperation conditionalOperation)
{
operation = conditionalOperation;
}

return operation;
}

private static bool IsArgumentOfJsonSerializer(IOperation operation, INamedTypeSymbol jsonSerializerSymbol, out bool isArgumentOfInvocation)
{
if (operation.Parent is IArgumentOperation arg && arg.Parent is IInvocationOperation inv)
{
isArgumentOfInvocation = true;
return SymbolEqualityComparer.Default.Equals(inv.TargetMethod.ContainingType, jsonSerializerSymbol);
}

isArgumentOfInvocation = false;
return false;
}

private static bool IsFieldOrPropertyAssignment(IOperation operation)
{
IOperation? current = operation.Parent;

while (current is IAssignmentOperation assignment)
{
if (assignment.Target is IFieldReferenceOperation or IPropertyReferenceOperation)
{
return true;
}

current = current.Parent;
}

return false;
}

private static bool IsTupleForDeconstructionTargetingFieldOrProperty(IOperation operation)
{
IOperation? current = operation.Parent;

if (current is not ITupleOperation tuple)
{
return false;
}

Stack<int> depth = new Stack<int>();
depth.Push(tuple.Elements.IndexOf(operation));

// walk-up right-hand nested tuples.
while (tuple.Parent is ITupleOperation parent)
{
depth.Push(parent.Elements.IndexOf(tuple));
tuple = parent;
}

current = tuple.WalkUpConversion().Parent;
if (current is not IDeconstructionAssignmentOperation deconstruction)
{
return false;
}

// walk-down left-hand nested tuples and see if it targets a field or property.
if (deconstruction.Target is not ITupleOperation deconstructionTarget)
{
return false;
}

tuple = deconstructionTarget;

IOperation? target = null;
while (depth.Count > 0)
{
int idx = depth.Pop();
target = tuple.Elements[idx];

if (target is ITupleOperation targetAsTuple)
{
tuple = targetAsTuple;
}
else if (depth.Count > 0)
{
return false;
}
}

return target is IFieldReferenceOperation or IPropertyReferenceOperation;
}

private static bool IsClosureOnLambdaOrLocalFunction(IOperation operation, IBlockOperation localBlock)
{
if (!operation.IsWithinLambdaOrLocalFunction(out IOperation? lambdaOrLocalFunc))
{
return false;
}

IBlockOperation? block = lambdaOrLocalFunc switch
{
IAnonymousFunctionOperation lambda => lambda.Body,
ILocalFunctionOperation localFunc => localFunc.Body,
_ => throw new InvalidOperationException()
};

return block != localBlock;
}

private static bool IsLocalAssignment(IOperation operation, [NotNullWhen(true)] out List<ILocalSymbol>? localSymbols)
{
localSymbols = null;
IOperation? currentOperation = operation.Parent;

while (currentOperation is not null)
{
// for cases like:
// var options;
// options = new JsonSerializerOptions();
if (currentOperation is IExpressionStatementOperation)
{
IOperation? tmpOperation = operation.Parent;
while (tmpOperation is IAssignmentOperation assignment)
{
if (assignment.Target is IFieldReferenceOperation or IPropertyReferenceOperation)
{
return false;
}
else if (assignment.Target is ILocalReferenceOperation localRef)
{
localSymbols ??= new List<ILocalSymbol>();
localSymbols.Add(localRef.Local);
}

tmpOperation = assignment.Parent;
}

return localSymbols != null;
}
// For cases like:
// var options = new JsonSerializerOptions();
else if (currentOperation is IVariableDeclarationOperation declaration)
{
if (operation.Parent is IAssignmentOperation assignment)
{
foreach (IOperation children in assignment.Children)
{
if (children is IFieldReferenceOperation or IPropertyReferenceOperation)
{
return false;
}
}
}

var local = GetLocalSymbolFromDeclaration(declaration);
if (local != null)
{
localSymbols = new List<ILocalSymbol> { local };
}

return local != null;
}

currentOperation = currentOperation.Parent;
}

return false;
}

private static ILocalSymbol? GetLocalSymbolFromDeclaration(IVariableDeclarationOperation declaration)
{
if (declaration.Declarators.Length != 1)
{
return null;
}

IVariableDeclaratorOperation declarator = declaration.Declarators[0];
return declarator.Symbol;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,21 @@
<target state="translated">Vyhněte se konstantním polím jako argumentům</target>
<note />
</trans-unit>
<trans-unit id="AvoidSingleUseOfLocalJsonSerializerOptionsDescription">
<source>Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application.</source>
<target state="new">Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application.</target>
<note />
</trans-unit>
<trans-unit id="AvoidSingleUseOfLocalJsonSerializerOptionsMessage">
<source>Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead.</source>
<target state="new">Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead.</target>
<note />
</trans-unit>
<trans-unit id="AvoidSingleUseOfLocalJsonSerializerOptionsTitle">
<source>Cache and reuse 'JsonSerializerOptions' instances</source>
<target state="new">Cache and reuse 'JsonSerializerOptions' instances</target>
<note />
</trans-unit>
<trans-unit id="AvoidStringBuilderPInvokeParametersDescription">
<source>Marshalling of 'StringBuilder' always creates a native buffer copy, resulting in multiple allocations for one marshalling operation.</source>
<target state="translated">Zařazením parametru StringBuilder se vždy vytvoří kopie nativní vyrovnávací paměti, která bude mít za následek vícenásobné přidělení pro jednu operaci zařazování.</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,21 @@
<target state="translated">Konstantenmatrizen als Argumente vermeiden</target>
<note />
</trans-unit>
<trans-unit id="AvoidSingleUseOfLocalJsonSerializerOptionsDescription">
<source>Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application.</source>
<target state="new">Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead. Single use 'JsonSerializerOptions' instances can substantially degrade the performance of your application.</target>
<note />
</trans-unit>
<trans-unit id="AvoidSingleUseOfLocalJsonSerializerOptionsMessage">
<source>Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead.</source>
<target state="new">Avoid creating a new 'JsonSerializerOptions' instance for every serialization operation. Cache and reuse instances instead.</target>
<note />
</trans-unit>
<trans-unit id="AvoidSingleUseOfLocalJsonSerializerOptionsTitle">
<source>Cache and reuse 'JsonSerializerOptions' instances</source>
<target state="new">Cache and reuse 'JsonSerializerOptions' instances</target>
<note />
</trans-unit>
<trans-unit id="AvoidStringBuilderPInvokeParametersDescription">
<source>Marshalling of 'StringBuilder' always creates a native buffer copy, resulting in multiple allocations for one marshalling operation.</source>
<target state="translated">Beim Marshalling von "StringBuilder" wird immer eine native Pufferkopie erstellt, sodass mehrere Zuordnungen für einen Marshallingvorgang vorhanden sind.</target>
Expand Down
Loading

0 comments on commit 2af4cd8

Please sign in to comment.