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

New Analyzer: Prevent behavioral change in built-in operators for IntPtr and UIntPtr #6153

Merged
merged 8 commits into from
Sep 21, 2022
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the MIT license. See License.txt in the project root for license information.

using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.NetCore.Analyzers.Runtime;

namespace Microsoft.NetCore.CSharp.Analyzers.Runtime
{
/// <summary>
/// CA2020: Detects Behavioral Changes introduced by new Numeric IntPtr UIntPtr feature
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class CSharpPreventNumericIntPtrUIntPtrBehavioralChanges : PreventNumericIntPtrUIntPtrBehavioralChanges
{
private const string IntPtr = nameof(IntPtr);
private const string UIntPtr = nameof(UIntPtr);

protected override bool IsWithinCheckedContext(IOperation operation)
{
var parent = operation.Parent.Syntax;
while (parent != null)
{
switch (parent)
{
case CheckedExpressionSyntax expression:
return expression.IsKind(SyntaxKind.CheckedExpression);
case CheckedStatementSyntax statement:
return statement.IsKind(SyntaxKind.CheckedStatement);
case MethodDeclarationSyntax:
return false;
}

parent = parent.Parent;
}

return false;
}

protected override bool IsAliasUsed(ISymbol? symbol)
{
if (symbol != null)
{
foreach (SyntaxReference reference in symbol.DeclaringSyntaxReferences)
{
SyntaxNode definition = reference.GetSyntax();

while (definition is VariableDeclaratorSyntax)
{
definition = definition.Parent;
}

var type = GetType(definition);

if (IdentifierNameIsIntPtrOrUIntPtr(type))
{
return false;
}
}
}

return true;
}

private static bool IdentifierNameIsIntPtrOrUIntPtr(ExpressionSyntax? syntax) =>
syntax is IdentifierNameSyntax identifierName &&
identifierName.Identifier.Text is IntPtr or UIntPtr;

protected override bool IsAliasUsed(SyntaxNode syntax)
{
if (syntax is CastExpressionSyntax castSyntax)
{
if (IdentifierNameIsIntPtrOrUIntPtr(castSyntax.Expression) ||
IdentifierNameIsIntPtrOrUIntPtr(castSyntax.Type))
{
return false;
}
}

return true;
}

private static TypeSyntax? GetType(SyntaxNode syntax) =>
syntax switch
{
VariableDeclarationSyntax fieldDeclaration => fieldDeclaration.Type,
ParameterSyntax parameter => parameter.Type,
CastExpressionSyntax cast => cast.Type,
_ => null,
};
}
}
1 change: 1 addition & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ CA1853 | Performance | Info | DoNotGuardDictionaryRemoveByContainsKey, [Document
CA1854 | Performance | Info | PreferDictionaryTryGetValueAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1854)
CA1855 | Performance | Info | UseSpanClearInsteadOfFillAnalyzer, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1855)
CA2019 | Reliability | Info | UseThreadStaticCorrectly, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2019)
CA2020 | Reliability | Info | PreventNumericIntPtrUIntPtrBehavioralChanges, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2020)
CA2259 | Usage | Warning | UseThreadStaticCorrectly, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2259)
CA5404 | Security | Disabled | DoNotDisableTokenValidationChecks, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca5404)
CA5405 | Security | Disabled | DoNotAlwaysSkipTokenValidationInDelegates, [Documentation](https://docs.microsoft.com/visualstudio/code-quality/ca5405)
Original file line number Diff line number Diff line change
Expand Up @@ -1971,4 +1971,19 @@
<data name="UseSpanClearInsteadOfFillTitle" xml:space="preserve">
<value>Prefer 'Clear' over 'Fill'</value>
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesDescription" xml:space="preserve">
<value>Some built in operators added in .NET 7 behave differently than the user defined operatorsi in .NET 6 and below. Some operators that used to throw in unchecked context while overflowing will not throw anymore unless wrapped within checked context, and some operators that not used to throw in checked context now would throw unless wrapped within unchecked context.</value>
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesConversionNotThrowMessage" xml:space="preserve">
<value>Starting with .NET 7 the explicit conversion '{0}' will not throw when overflowing in an unchecked context. Wrap the expression with a 'checked' statement to restore the .NET 6 behavior.</value>
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesOperatorThrowsMessage" xml:space="preserve">
<value>Starting with .NET 7 the operator '{0}' will throw when overflowing in a checked context. Wrap the expression with an 'unchecked' statement to restore the .NET 6 behavior.</value>
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesTitle" xml:space="preserve">
<value>Prevent from behavioral change</value>
buyaa-n marked this conversation as resolved.
Show resolved Hide resolved
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesConversionThrowsMessage" xml:space="preserve">
<value>Starting with .NET 7 the explicit conversion '{0}' will throw when overflowing in a checked context. Wrap the expression with an 'unchecked' statement to restore the .NET 6 behavior.</value>
</data>
</root>
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
// 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 Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.Operations;

namespace Microsoft.NetCore.Analyzers.Runtime
{
using static MicrosoftNetCoreAnalyzersResources;
/// <summary>
/// CA2020: Detects Behavioral Changes introduced by new Numeric IntPtr UIntPtr feature
/// </summary>
public abstract class PreventNumericIntPtrUIntPtrBehavioralChanges : DiagnosticAnalyzer
{
internal const string RuleId = "CA2020";
private static readonly LocalizableResourceString s_titleResource = CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesTitle));
private static readonly LocalizableResourceString s_descriptionResource = CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesDescription));

internal static readonly DiagnosticDescriptor OperatorThrowsRule = DiagnosticDescriptorHelper.Create(
RuleId,
s_titleResource,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesOperatorThrowsMessage)),
DiagnosticCategory.Reliability,
RuleLevel.IdeSuggestion,
s_descriptionResource,
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor ConversionThrowsRule = DiagnosticDescriptorHelper.Create(
RuleId,
s_titleResource,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesConversionThrowsMessage)),
DiagnosticCategory.Reliability,
RuleLevel.IdeSuggestion,
s_descriptionResource,
isPortedFxCopRule: false,
isDataflowRule: false);

internal static readonly DiagnosticDescriptor ConversionNotThrowRule = DiagnosticDescriptorHelper.Create(
RuleId,
s_titleResource,
CreateLocalizableResourceString(nameof(PreventNumericIntPtrUIntPtrBehavioralChangesConversionNotThrowMessage)),
DiagnosticCategory.Reliability,
RuleLevel.IdeSuggestion,
s_descriptionResource,
isPortedFxCopRule: false,
isDataflowRule: false);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(OperatorThrowsRule, ConversionThrowsRule, ConversionNotThrowRule);

protected abstract bool IsWithinCheckedContext(IOperation operation);

protected abstract bool IsAliasUsed(ISymbol? syntaxReferences);

protected abstract bool IsAliasUsed(SyntaxNode syntax);

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(context =>
{
if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemRuntimeCompilerServicesRuntimeFeature, out var runtimeFeatureType) ||
!runtimeFeatureType.GetMembers("NumericIntPtr").OfType<IFieldSymbol>().Any())
{
// Numeric IntPtr feature not available
return;
}

context.RegisterOperationAction(context =>
{
if (context.Operation is IBinaryOperation binaryOperation &&
binaryOperation.IsAdditionOrSubstractionOperation(out var binaryOperator) &&
binaryOperation.IsChecked)
{
if ((binaryOperation.LeftOperand.Type?.SpecialType == SpecialType.System_IntPtr ||
binaryOperation.LeftOperand.Type?.SpecialType == SpecialType.System_UIntPtr) &&
IsConversionFromInt32(binaryOperation.RightOperand) &&
!IsAliasUsed(GetSymbol(binaryOperation.LeftOperand)))
{
context.ReportDiagnostic(binaryOperation.CreateDiagnostic(OperatorThrowsRule, binaryOperator));
}
}
else if (context.Operation is IConversionOperation conversionOperation)
{
var operation = conversionOperation.WalkDownConversion(c => c.IsImplicit); // get innermost conversion
if (operation is IConversionOperation explicitConversion &&
explicitConversion.OperatorMethod == null) // Built in conversion
{
if (explicitConversion.IsChecked ||
IsWithinCheckedContext(explicitConversion))
{
if (IsIntPtrToOrFromPtrConversion(explicitConversion.Type, explicitConversion.Operand.Type) &&
!IsAliasUsed(GetSymbol(explicitConversion.Operand)))
{
context.ReportDiagnostic(explicitConversion.CreateDiagnostic(ConversionThrowsRule,
PopulateConversionString(explicitConversion.Type, explicitConversion.Operand.Type)));
}
else if (IsIntPtrToOrFromPtrConversion(explicitConversion.Operand.Type, explicitConversion.Type) &&
!IsAliasUsed(explicitConversion.Syntax))
{
context.ReportDiagnostic(explicitConversion.CreateDiagnostic(ConversionThrowsRule,
PopulateConversionString(explicitConversion.Type, explicitConversion.Operand.Type)));
}
}
else // unchecked context
{
if ((IsLongToIntPtrConversion(explicitConversion.Type, explicitConversion.Operand.Type) ||
IsULongToUIntPtrConversion(explicitConversion.Type, explicitConversion.Operand.Type)) &&
!IsAliasUsed(explicitConversion.Syntax))
{
context.ReportDiagnostic(explicitConversion.CreateDiagnostic(ConversionNotThrowRule,
PopulateConversionString(explicitConversion.Type, explicitConversion.Operand.Type)));
}
else if ((IsIntPtrToIntConversion(explicitConversion.Type, explicitConversion.Operand.Type) ||
IsUIntPtrToUIntConversion(explicitConversion.Type, explicitConversion.Operand.Type)) &&
!IsAliasUsed(GetSymbol(explicitConversion.Operand)))
{
context.ReportDiagnostic(explicitConversion.CreateDiagnostic(ConversionNotThrowRule,
PopulateConversionString(explicitConversion.Type, explicitConversion.Operand.Type)));
}
}
}
}
},
OperationKind.Binary, OperationKind.Conversion);
});

static string PopulateConversionString(ITypeSymbol type, ITypeSymbol operand)
{
string typeName = type.Name;
string operandName = operand.Name;

if (type is IPointerTypeSymbol)
{
typeName = type.ToString();
}

if (operand is IPointerTypeSymbol)
{
operandName = operand.ToString();
}

return $"({typeName}){operandName}";
}

static ISymbol? GetSymbol(IOperation operation) =>
operation switch
{
IFieldReferenceOperation fieldReference => fieldReference.Field,
IParameterReferenceOperation parameter => parameter.Parameter,
ILocalReferenceOperation local => local.Local,
_ => null,
};

static bool IsConversionFromInt32(IOperation operation) =>
operation is IConversionOperation conversion &&
conversion.Operand.Type.SpecialType == SpecialType.System_Int32;

static bool IsIntPtrToOrFromPtrConversion(ITypeSymbol pointerType, ITypeSymbol intPtrType) =>
intPtrType.SpecialType == SpecialType.System_IntPtr &&
pointerType is IPointerTypeSymbol pointer;

static bool IsLongToIntPtrConversion(ITypeSymbol convertingType, ITypeSymbol operandType) =>
convertingType.SpecialType == SpecialType.System_IntPtr &&
operandType.SpecialType == SpecialType.System_Int64;

static bool IsIntPtrToIntConversion(ITypeSymbol convertingType, ITypeSymbol operandType) =>
convertingType.SpecialType == SpecialType.System_Int32 &&
operandType.SpecialType == SpecialType.System_IntPtr;

static bool IsULongToUIntPtrConversion(ITypeSymbol convertingType, ITypeSymbol operandType) =>
convertingType.SpecialType == SpecialType.System_UIntPtr &&
operandType.SpecialType == SpecialType.System_UInt64;

static bool IsUIntPtrToUIntConversion(ITypeSymbol convertingType, ITypeSymbol operandType) =>
convertingType.SpecialType == SpecialType.System_UInt32 &&
operandType.SpecialType == SpecialType.System_UIntPtr;
}
}
}

Loading