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

Fix S1905 FP: Nullability context and array of anonymous types #6753

Merged
merged 23 commits into from Mar 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
56a5ca2
Add FP test cases
Tim-Pohlmann Feb 13, 2023
fa0d931
Clean up some code
Tim-Pohlmann Feb 13, 2023
2ce6132
Add check for anaonymous objects in implicit arrays and switch expres…
Tim-Pohlmann Feb 13, 2023
655120e
Improve pattern matching
Tim-Pohlmann Feb 13, 2023
12e5164
Fix a newly introduced FN
Tim-Pohlmann Feb 13, 2023
f87cadd
Change approach to check for Nullability
Tim-Pohlmann Feb 13, 2023
eaa9553
Change code to use FlowState when available
Tim-Pohlmann Feb 15, 2023
c85f588
Refactor and improve tests
Tim-Pohlmann Feb 15, 2023
cbb4a49
Clean up CheckCastExpression
Tim-Pohlmann Feb 15, 2023
92fe174
Update analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs
Tim-Pohlmann Feb 15, 2023
cdf4780
Update analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs
Tim-Pohlmann Feb 15, 2023
8a5b500
Update analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs
Tim-Pohlmann Feb 15, 2023
ab8ecd8
Fixe wrong parameter order
Tim-Pohlmann Feb 15, 2023
b3db542
Refactor some code and add tests for as expressions
Tim-Pohlmann Feb 15, 2023
05578e3
Improve code
Tim-Pohlmann Feb 15, 2023
2d51c5f
Refactor GetElementType
Tim-Pohlmann Feb 24, 2023
11d61ce
Add more tests
Tim-Pohlmann Feb 24, 2023
aae24ab
Use Is(KnownType...) instead of name check
Tim-Pohlmann Feb 28, 2023
28ddab0
Simplify GetReportLocation
Tim-Pohlmann Mar 1, 2023
943592d
Add more tests, including FNs
Tim-Pohlmann Mar 6, 2023
95278ed
Fix FP
Tim-Pohlmann Mar 6, 2023
3a1197c
Simplify code
Tim-Pohlmann Mar 6, 2023
f8abb8d
Remove CodeSmell
Tim-Pohlmann Mar 6, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
240 changes: 108 additions & 132 deletions analyzers/src/SonarAnalyzer.CSharp/Rules/RedundantCast.cs
Expand Up @@ -18,161 +18,137 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

namespace SonarAnalyzer.Rules.CSharp
{
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class RedundantCast : SonarDiagnosticAnalyzer
{
internal const string DiagnosticId = "S1905";
private const string MessageFormat = "Remove this unnecessary cast to '{0}'.";

private static readonly DiagnosticDescriptor rule =
DescriptorFactory.Create(DiagnosticId, MessageFormat);

public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(rule);
namespace SonarAnalyzer.Rules.CSharp;

private static readonly ISet<string> CastIEnumerableMethods = new HashSet<string> { "Cast", "OfType" };

protected override void Initialize(SonarAnalysisContext context)
{
context.RegisterNodeAction(
c =>
{
var castExpression = (CastExpressionSyntax)c.Node;
CheckCastExpression(c, castExpression.Expression, castExpression.Type, castExpression.Type.GetLocation());
},
SyntaxKind.CastExpression);
[DiagnosticAnalyzer(LanguageNames.CSharp)]
public sealed class RedundantCast : SonarDiagnosticAnalyzer
{
internal const string DiagnosticId = "S1905";
private const string MessageFormat = "Remove this unnecessary cast to '{0}'.";

context.RegisterNodeAction(
c =>
{
var castExpression = (BinaryExpressionSyntax)c.Node;
CheckCastExpression(c, castExpression.Left, castExpression.Right,
castExpression.OperatorToken.CreateLocation(castExpression.Right));
},
SyntaxKind.AsExpression);

context.RegisterNodeAction(
CheckExtensionMethodInvocation,
SyntaxKind.InvocationExpression);
}
private static readonly DiagnosticDescriptor Rule =
DescriptorFactory.Create(DiagnosticId, MessageFormat);

private static void CheckCastExpression(SonarSyntaxNodeReportingContext context, ExpressionSyntax expression, ExpressionSyntax type, Location location)
{
if (expression.IsKind(SyntaxKindEx.DefaultLiteralExpression))
{
return;
}
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);

var expressionType = context.SemanticModel.GetTypeInfo(expression).Type;
if (expressionType == null)
{
return;
}
private static readonly ISet<string> CastIEnumerableMethods = new HashSet<string> { "Cast", "OfType" };

var castType = context.SemanticModel.GetTypeInfo(type).Type;
if (castType == null)
protected override void Initialize(SonarAnalysisContext context)
{
context.RegisterNodeAction(
c =>
{
return;
}
var castExpression = (CastExpressionSyntax)c.Node;
CheckCastExpression(c, castExpression.Expression, castExpression.Type, castExpression.Type.GetLocation());
},
SyntaxKind.CastExpression);

if (expressionType.Equals(castType))
context.RegisterNodeAction(
c =>
{
context.ReportIssue(Diagnostic.Create(rule, location,
castType.ToMinimalDisplayString(context.SemanticModel, expression.SpanStart)));
}
}
var castExpression = (BinaryExpressionSyntax)c.Node;
CheckCastExpression(c, castExpression.Left, castExpression.Right,
castExpression.OperatorToken.CreateLocation(castExpression.Right));
},
SyntaxKind.AsExpression);

context.RegisterNodeAction(
CheckExtensionMethodInvocation,
SyntaxKind.InvocationExpression);
}

private static void CheckExtensionMethodInvocation(SonarSyntaxNodeReportingContext context)
private static void CheckCastExpression(SonarSyntaxNodeReportingContext context, ExpressionSyntax expression, ExpressionSyntax type, Location location)
{
if (!expression.IsKind(SyntaxKindEx.DefaultLiteralExpression)
&& context.SemanticModel.GetTypeInfo(expression) is { Type: { } expressionType } expressionTypeInfo
&& context.SemanticModel.GetTypeInfo(type) is { Type: { } castType }
&& expressionType.Equals(castType)
&& FlowStateEquals(expressionTypeInfo, type))
{
var invocation = (InvocationExpressionSyntax)context.Node;
if (GetEnumerableExtensionSymbol(invocation, context.SemanticModel) is { } methodSymbol)
{
var returnType = methodSymbol.ReturnType;
if (GetGenericTypeArgument(returnType) is { } castType)
{
if (methodSymbol.Name == "OfType" && CanHaveNullValue(castType))
{
// OfType() filters 'null' values from enumerables
return;
}

var elementType = GetElementType(invocation, methodSymbol, context.SemanticModel);
// Generic types {T} and {T?} are equal and there is no way to access NullableAnnotation field right now
// See https://github.com/SonarSource/sonar-dotnet/issues/3273
if (elementType != null && elementType.Equals(castType) && string.Equals(elementType.ToString(), castType.ToString(), System.StringComparison.Ordinal))
{
var methodCalledAsStatic = methodSymbol.MethodKind == MethodKind.Ordinary;
context.ReportIssue(Diagnostic.Create(rule, GetReportLocation(invocation, methodCalledAsStatic),
returnType.ToMinimalDisplayString(context.SemanticModel, invocation.SpanStart)));
}
}
}
ReportIssue(context, expression, castType, location);
}
}

/// If the invocation one of the <see cref="CastIEnumerableMethods"/> extensions, returns the method symbol.
private static IMethodSymbol GetEnumerableExtensionSymbol(InvocationExpressionSyntax invocation, SemanticModel semanticModel) =>
invocation.GetMethodCallIdentifier() is { } methodName
&& CastIEnumerableMethods.Contains(methodName.ValueText)
&& semanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol
&& methodSymbol.IsExtensionOn(KnownType.System_Collections_IEnumerable)
? methodSymbol
: null;

private static ITypeSymbol GetGenericTypeArgument(ITypeSymbol type) =>
type is INamedTypeSymbol returnType && returnType.Is(KnownType.System_Collections_Generic_IEnumerable_T)
? returnType.TypeArguments.Single()
: null;

private static bool CanHaveNullValue(ITypeSymbol type) => type.IsReferenceType || type.Name == "Nullable";

private static Location GetReportLocation(InvocationExpressionSyntax invocation, bool methodCalledAsStatic)
private static bool FlowStateEquals(TypeInfo expressionTypeInfo, ExpressionSyntax type)
{
var castingToNullable = type.IsKind(SyntaxKind.NullableType);
return expressionTypeInfo.Nullability().FlowState switch
{
if (!(invocation.Expression is MemberAccessExpressionSyntax memberAccess))
{
return invocation.Expression.GetLocation();
}

return methodCalledAsStatic
? memberAccess.GetLocation()
: memberAccess.OperatorToken.CreateLocation(invocation);
}
NullableFlowState.None => true,
NullableFlowState.MaybeNull => castingToNullable,
NullableFlowState.NotNull => !castingToNullable,
_ => true,
};
}

private static ITypeSymbol GetElementType(InvocationExpressionSyntax invocation, IMethodSymbol methodSymbol,
SemanticModel semanticModel)
private static void CheckExtensionMethodInvocation(SonarSyntaxNodeReportingContext context)
{
var invocation = (InvocationExpressionSyntax)context.Node;
if (GetEnumerableExtensionSymbol(invocation, context.SemanticModel) is { } methodSymbol)
{
ExpressionSyntax collection;
if (methodSymbol.MethodKind == MethodKind.Ordinary)
var returnType = methodSymbol.ReturnType;
if (GetGenericTypeArgument(returnType) is { } castType)
{
if (!invocation.ArgumentList.Arguments.Any())
if (methodSymbol.Name == "OfType" && CanHaveNullValue(castType))
{
return null;
// OfType() filters 'null' values from enumerables
return;
}
collection = invocation.ArgumentList.Arguments.First().Expression;
}
else
{
if (!(invocation.Expression is MemberAccessExpressionSyntax memberAccess))

var elementType = GetElementType(invocation, methodSymbol, context.SemanticModel);
// Generic types {T} and {T?} are equal and there is no way to access NullableAnnotation field right now
Tim-Pohlmann marked this conversation as resolved.
Show resolved Hide resolved
// See https://github.com/SonarSource/sonar-dotnet/issues/3273
if (elementType != null && elementType.Equals(castType) && string.Equals(elementType.ToString(), castType.ToString(), StringComparison.Ordinal))
{
return null;
var methodCalledAsStatic = methodSymbol.MethodKind == MethodKind.Ordinary;
ReportIssue(context, invocation, returnType, GetReportLocation(invocation, methodCalledAsStatic));
}
collection = memberAccess.Expression;
}
}
}

var typeInfo = semanticModel.GetTypeInfo(collection);
if (typeInfo.Type is INamedTypeSymbol collectionType &&
collectionType.TypeArguments.Length == 1)
{
return collectionType.TypeArguments.First();
}
private static void ReportIssue(SonarSyntaxNodeReportingContext context, ExpressionSyntax expression, ITypeSymbol castType, Location location) =>
context.ReportIssue(Diagnostic.Create(Rule, location, castType.ToMinimalDisplayString(context.SemanticModel, expression.SpanStart)));

if (typeInfo.Type is IArrayTypeSymbol arrayType &&
arrayType.Rank == 1) // casting is necessary for multidimensional arrays
{
return arrayType.ElementType;
}
/// If the invocation one of the <see cref="CastIEnumerableMethods"/> extensions, returns the method symbol.
private static IMethodSymbol GetEnumerableExtensionSymbol(InvocationExpressionSyntax invocation, SemanticModel semanticModel) =>
invocation.GetMethodCallIdentifier() is { } methodName
&& CastIEnumerableMethods.Contains(methodName.ValueText)
&& semanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol
&& methodSymbol.IsExtensionOn(KnownType.System_Collections_IEnumerable)
? methodSymbol
: null;

return null;
}
private static ITypeSymbol GetGenericTypeArgument(ITypeSymbol type) =>
type is INamedTypeSymbol returnType && returnType.Is(KnownType.System_Collections_Generic_IEnumerable_T)
? returnType.TypeArguments.Single()
: null;

private static bool CanHaveNullValue(ITypeSymbol type) =>
type.IsReferenceType || type.Is(KnownType.System_Nullable_T);

private static Location GetReportLocation(InvocationExpressionSyntax invocation, bool methodCalledAsStatic) =>
methodCalledAsStatic is false && invocation.Expression is MemberAccessExpressionSyntax memberAccess
? memberAccess.OperatorToken.CreateLocation(invocation)
: invocation.Expression.GetLocation();

private static ITypeSymbol GetElementType(InvocationExpressionSyntax invocation, IMethodSymbol methodSymbol, SemanticModel semanticModel)
{
return semanticModel.GetTypeInfo(CollectionExpression(invocation, methodSymbol)).Type switch
{
INamedTypeSymbol { TypeArguments: { Length: 1 } typeArguments } => typeArguments.First(),
IArrayTypeSymbol { Rank: 1 } arrayType => arrayType.ElementType, // casting is necessary for multidimensional arrays
_ => null
};

static ExpressionSyntax CollectionExpression(InvocationExpressionSyntax invocation, IMethodSymbol methodSymbol) =>
methodSymbol.MethodKind is MethodKind.ReducedExtension
? ReducedExtensionExpression(invocation)
: invocation.ArgumentList.Arguments.FirstOrDefault()?.Expression;

static ExpressionSyntax ReducedExtensionExpression(InvocationExpressionSyntax invocation) =>
invocation.Expression is MemberAccessExpressionSyntax { Expression: { } memberAccessExpression }
? memberAccessExpression
: invocation.GetParentConditionalAccessExpression()?.Expression;
}
}