Skip to content

Commit

Permalink
Improve perf for S2291, S2971, S3346 and S3256 (#3777)
Browse files Browse the repository at this point in the history
  • Loading branch information
@andrei-epure-sonarsource
andrei-epure-sonarsource authored Nov 20, 2020
1 parent 027464e commit 0ab8ff1
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 74 deletions.
2 changes: 1 addition & 1 deletion sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/BeginInvokePairedWithEndInvoke.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ protected override void Initialize(SonarAnalysisContext context) =>
{
var invocation = (InvocationExpressionSyntax)c.Node;
const string BeginInvoke = "BeginInvoke";
if (invocation.ToStringContains(BeginInvoke) &&
if (invocation.Expression.ToStringContains(BeginInvoke) &&
GetCallbackArg(invocation) is { } callbackArg &&
GetMethodSymbol(invocation, c.SemanticModel) is { } methodSymbol &&
methodSymbol.Name == BeginInvoke &&
Expand Down
1 change: 1 addition & 0 deletions sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/CollectionQuerySimplification.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ private static void CheckCountCall(SyntaxNodeAnalysisContext context)
var invocation = (InvocationExpressionSyntax)context.Node;
if (invocation.ArgumentList?.Arguments.Count == 0 &&
invocation.Expression is MemberAccessExpressionSyntax memberAccess &&
memberAccess.Name.Identifier.ValueText == CountName &&
context.SemanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol &&
methodSymbol.Name == CountName &&
methodSymbol.IsExtensionOn(KnownType.System_Collections_Generic_IEnumerable_T) &&
Expand Down
31 changes: 13 additions & 18 deletions sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/DebugAssertHasNoSideEffects.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,20 +47,18 @@ public sealed class DebugAssertHasNoSideEffects : SonarDiagnosticAnalyzer
"INSERT", "PUSH", "APPEND", "CLEAR", "DEQUEUE", "ENQUEUE", "DISPOSE"
};

protected override void Initialize(SonarAnalysisContext context)
{
protected override void Initialize(SonarAnalysisContext context) =>
context.RegisterSyntaxNodeActionInNonGenerated(c =>
{
var invokedMethodSyntax = c.Node as InvocationExpressionSyntax;

if (ContainsCallsWithSideEffects(invokedMethodSyntax) &&
IsDebugAssert(invokedMethodSyntax, c))
if (IsDebugAssert(invokedMethodSyntax, c)
&& ContainsCallsWithSideEffects(invokedMethodSyntax))
{
c.ReportDiagnosticWhenActive(Diagnostic.Create(rule, invokedMethodSyntax.ArgumentList.GetLocation()));
}
},
SyntaxKind.InvocationExpression);
}

private static string GetIdentifierName(InvocationExpressionSyntax invocation)
{
Expand All @@ -74,21 +72,18 @@ private static string GetIdentifierName(InvocationExpressionSyntax invocation)
return memberBinding?.Name?.Identifier.ValueText;
}

private bool IsDebugAssert(InvocationExpressionSyntax invocation, SyntaxNodeAnalysisContext context)
{
var invokedMethodSymbol = context.SemanticModel.GetSymbolInfo(invocation).Symbol as IMethodSymbol;
return invokedMethodSymbol.IsDebugAssert();
}
private static bool IsDebugAssert(InvocationExpressionSyntax invocation, SyntaxNodeAnalysisContext context) =>
invocation.Expression is MemberAccessExpressionSyntax memberAccess
&& memberAccess.Name.Identifier.ValueText == nameof(System.Diagnostics.Debug.Assert)
&& context.SemanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol symbol
&& symbol.IsDebugAssert();

private bool ContainsCallsWithSideEffects(InvocationExpressionSyntax invocation)
{
return invocation
.DescendantNodes()
private static bool ContainsCallsWithSideEffects(InvocationExpressionSyntax invocation) =>
invocation.DescendantNodes()
.OfType<InvocationExpressionSyntax>()
.Select(GetIdentifierName)
.Any(name => !string.IsNullOrEmpty(name) &&
name != "SetEquals" &&
sideEffectWords.Contains(name.SplitCamelCaseToWords().First()));
}
.Any(name => !string.IsNullOrEmpty(name)
&& name != "SetEquals"
&& sideEffectWords.Contains(name.SplitCamelCaseToWords().First()));
}
}
1 change: 1 addition & 0 deletions sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/EnumerableSumInUnchecked.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ protected override void Initialize(SonarAnalysisContext context)
var invocation = (InvocationExpressionSyntax)c.Node;
var expression = invocation.Expression;
if (expression is MemberAccessExpressionSyntax memberAccess &&
memberAccess.Name.Identifier.ValueText == "Sum" &&
IsSumInsideUnchecked(invocation) &&
c.SemanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol &&
IsSumOnInteger(methodSymbol))
Expand Down
3 changes: 2 additions & 1 deletion sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/GetTypeWithIsAssignableFrom.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ protected override void Initialize(SonarAnalysisContext context)
return;
}

if (invocation.ToStringContainsEitherOr("IsInstanceOfType", "IsAssignableFrom") &&
var methodName = memberAccess.Name.Identifier.ValueText;
if ((methodName == "IsInstanceOfType" || methodName == "IsAssignableFrom") &&
c.SemanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol &&
methodSymbol.IsInType(KnownType.System_Type))
{
Expand Down
14 changes: 7 additions & 7 deletions sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/RedundantToCharArrayCall.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,13 @@ protected override void Initialize(SonarAnalysisContext context)
{
var invocation = (InvocationExpressionSyntax)c.Node;

if ((invocation.Parent is ElementAccessExpressionSyntax || invocation.Parent is ForEachStatementSyntax) &&
invocation.ToStringContains("ToCharArray") &&
invocation.Expression is MemberAccessExpressionSyntax memberAccess &&
c.SemanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol &&
methodSymbol.Name == "ToCharArray" &&
methodSymbol.IsInType(KnownType.System_String) &&
methodSymbol.Parameters.Length == 0)
if ((invocation.Parent is ElementAccessExpressionSyntax || invocation.Parent is ForEachStatementSyntax)
&& invocation.Expression is MemberAccessExpressionSyntax memberAccess
&& memberAccess.Name.Identifier.ValueText == "ToCharArray"
&& c.SemanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol
&& methodSymbol.Name == "ToCharArray"
&& methodSymbol.IsInType(KnownType.System_String)
&& methodSymbol.Parameters.Length == 0)
{
c.ReportDiagnosticWhenActive(Diagnostic.Create(rule, memberAccess.Name.GetLocation()));
}
Expand Down
2 changes: 1 addition & 1 deletion sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/TypeExaminationOnSystemType.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ protected override void Initialize(SonarAnalysisContext context)
{
var invocation = (InvocationExpressionSyntax)c.Node;

if (invocation.ToStringContainsEitherOr("IsInstanceOfType", "GetType") &&
if (invocation.Expression.ToStringContainsEitherOr("IsInstanceOfType", "GetType") &&
c.SemanticModel.GetSymbolInfo(invocation).Symbol is IMethodSymbol methodSymbol)
{
CheckGetTypeCallOnType(invocation, methodSymbol, c);
Expand Down
72 changes: 27 additions & 45 deletions sonaranalyzer-dotnet/src/SonarAnalyzer.CSharp/Rules/UseStringIsNullOrEmpty.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
* Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
*/

using System.Collections.Generic;
using System.Collections.Immutable;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
Expand All @@ -42,7 +41,7 @@ public sealed class UseStringIsNullOrEmpty : SonarDiagnosticAnalyzer

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

private static readonly ISet<string> ReservedMethods = new HashSet<string> { nameof(string.Equals) };
private const string EqualsName = nameof(string.Equals);

protected override void Initialize(SonarAnalysisContext context)
{
Expand All @@ -51,34 +50,25 @@ protected override void Initialize(SonarAnalysisContext context)
{
var invocationExpression = (InvocationExpressionSyntax)c.Node;

if (!(invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression))
if (invocationExpression.Expression is MemberAccessExpressionSyntax memberAccessExpression
&& memberAccessExpression.Name.Identifier.ValueText == EqualsName
&& TryGetFirstArgument(invocationExpression, out var firstArgument)
&& IsStringEqualsMethod(memberAccessExpression, c.SemanticModel))
{
return;
}

if (!TryGetFirstArgument(invocationExpression, out var firstArgument))
{
return;
}

if (!IsStringEqualsMethod(memberAccessExpression, c.SemanticModel))
{
return;
}

// x.Equals(value), where x is string.Empty, "" or const "", and value is some string
if (IsStringIdentifier(firstArgument.Expression, c.SemanticModel) &&
IsConstantEmptyString(memberAccessExpression.Expression, c.SemanticModel))
{
c.ReportDiagnosticWhenActive(Diagnostic.Create(rule, invocationExpression.GetLocation(), MessageFormat));
return;
}

// value.Equals(x), where x is string.Empty, "" or const "", and value is some string
if (IsStringIdentifier(memberAccessExpression.Expression, c.SemanticModel) &&
IsConstantEmptyString(firstArgument.Expression, c.SemanticModel))
{
c.ReportDiagnosticWhenActive(Diagnostic.Create(rule, invocationExpression.GetLocation(), MessageFormat));
// x.Equals(value), where x is string.Empty, "" or const "", and value is some string
if (IsStringIdentifier(firstArgument.Expression, c.SemanticModel)
&& IsConstantEmptyString(memberAccessExpression.Expression, c.SemanticModel))
{
c.ReportDiagnosticWhenActive(Diagnostic.Create(rule, invocationExpression.GetLocation(), MessageFormat));
return;
}

// value.Equals(x), where x is string.Empty, "" or const "", and value is some string
if (IsStringIdentifier(memberAccessExpression.Expression, c.SemanticModel)
&& IsConstantEmptyString(firstArgument.Expression, c.SemanticModel))
{
c.ReportDiagnosticWhenActive(Diagnostic.Create(rule, invocationExpression.GetLocation(), MessageFormat));
}
}
},
SyntaxKind.InvocationExpression);
Expand All @@ -95,13 +85,12 @@ private static bool IsStringEqualsMethod(MemberAccessExpressionSyntax memberAcce
{
var methodName = semanticModel.GetSymbolInfo(memberAccessExpression.Name);

return methodName.Symbol.IsInType(KnownType.System_String) &&
ReservedMethods.Contains(methodName.Symbol.Name);
return methodName.Symbol.IsInType(KnownType.System_String)
&& methodName.Symbol.Name == EqualsName;
}

private static bool IsStringIdentifier(ExpressionSyntax expression, SemanticModel semanticModel)
{

if (!(expression is IdentifierNameSyntax identifierNameExpression))
{
return false;
Expand All @@ -112,23 +101,16 @@ private static bool IsStringIdentifier(ExpressionSyntax expression, SemanticMode
return expressionType != null && expressionType.Is(KnownType.System_String);
}

private static bool IsConstantEmptyString(ExpressionSyntax expression, SemanticModel semanticModel)
{
return IsStringEmptyLiteral(expression) ||
IsStringEmptyConst(expression, semanticModel) ||
expression.IsStringEmpty(semanticModel);
}
private static bool IsConstantEmptyString(ExpressionSyntax expression, SemanticModel semanticModel) =>
IsStringEmptyLiteral(expression)
|| IsStringEmptyConst(expression, semanticModel)
|| expression.IsStringEmpty(semanticModel);

private static bool IsStringEmptyConst(ExpressionSyntax expression, SemanticModel semanticModel)
{
var constValue = semanticModel.GetConstantValue(expression);
if (!constValue.HasValue)
{
return false;
}


return constValue.Value is string stringConstValue && stringConstValue == string.Empty;
return constValue.HasValue
&& constValue.Value is string stringConstValue && stringConstValue == string.Empty;
}

private static bool IsStringEmptyLiteral(ExpressionSyntax expression)
Expand Down
14 changes: 13 additions & 1 deletion sonaranalyzer-dotnet/tests/SonarAnalyzer.UnitTest/TestCases/EnumerableSumInUnchecked.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ namespace Tests.Diagnostics
{
public class EnumerableSumInUnchecked
{
public void Test(List<int> list)
public void Test(List<int> list, List<float> list2)
{
int a = list.Sum(); // Compliant

Expand All @@ -20,6 +20,8 @@ public void Test(List<int> list)

e = Enumerable.Sum(list); // Noncompliant
// ^^^

float floatSum = list2.Sum(); // Compliant
}

checked
Expand Down Expand Up @@ -50,6 +52,16 @@ public void Test(List<int> list)
{
var y = l2.Sum(ll => ll); // Noncompliant
}

// coverage
list.Count();
MySum();
unchecked
{
MySum();
}
}

void MySum() { }
}
}

0 comments on commit 0ab8ff1

Please sign in to comment.