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

Add analyzers for Regex.Match(...).Success and Regex.Matches(...).Count #7547

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,7 @@
Rule ID | Category | Severity | Notes
--------|----------|----------|-------
CA1873 | Performance | Info | AvoidPotentiallyExpensiveCallWhenLoggingAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1873)
CA1874 | Performance | Info | UseRegexMembers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1874)
CA1875 | Performance | Info | UseRegexMembers, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1875)
CA2023 | Reliability | Warning | LoggerMessageDefineAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2023)
CA2024 | Reliability | Warning | DoNotUseEndOfStreamInAsyncMethods, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2024)
Original file line number Diff line number Diff line change
Expand Up @@ -1376,6 +1376,30 @@
<data name="UseEnvironmentCurrentManagedThreadIdFix" xml:space="preserve">
<value>Use 'Environment.CurrentManagedThreadId'</value>
</data>
<data name="UseRegexCountDescription" xml:space="preserve">
<value>'Regex.Count' is simpler and faster than 'Regex.Matches(...).Count'.</value>
</data>
<data name="UseRegexCountMessage" xml:space="preserve">
<value>Use 'Regex.Count' instead of 'Regex.Matches(...).Count'</value>
</data>
<data name="UseRegexCountTitle" xml:space="preserve">
<value>Use 'Regex.Count'</value>
</data>
<data name="UseRegexCountFix" xml:space="preserve">
<value>Use 'Regex.Count'</value>
</data>
<data name="UseRegexIsMatchDescription" xml:space="preserve">
<value>'Regex.IsMatch' is simpler and faster than 'Regex.Match(...).Success'.</value>
</data>
<data name="UseRegexIsMatchMessage" xml:space="preserve">
<value>Use 'Regex.IsMatch' instead of 'Regex.Match(...).Success'</value>
</data>
<data name="UseRegexIsMatchTitle" xml:space="preserve">
<value>Use 'Regex.IsMatch'</value>
</data>
<data name="UseRegexIsMatchFix" xml:space="preserve">
<value>Use 'Regex.IsMatch'</value>
</data>
<data name="ThreadStaticOnNonStaticFieldDescription" xml:space="preserve">
<value>'ThreadStatic' only affects static fields. When applied to instance fields, it has no impact on behavior.</value>
</data>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
// 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>
/// CA1874: <inheritdoc cref="UseRegexIsMatchMessage"/>
/// CA1875: <inheritdoc cref="UseRegexCountMessage"/>
/// </summary>
[DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
public sealed class UseRegexMembers : DiagnosticAnalyzer
{
internal const string RegexIsMatchRuleId = "CA1874";
internal const string RegexCountRuleId = "CA1875";

// Regex.Match(...).Success => Regex.IsMatch(...)
internal static readonly DiagnosticDescriptor UseRegexIsMatchRuleId = DiagnosticDescriptorHelper.Create(RegexIsMatchRuleId,
CreateLocalizableResourceString(nameof(UseRegexIsMatchTitle)),
CreateLocalizableResourceString(nameof(UseRegexIsMatchMessage)),
DiagnosticCategory.Performance,
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(UseRegexIsMatchDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

// Regex.Matches(...).Count => Regex.Count(...)
internal static readonly DiagnosticDescriptor UseRegexCountRuleId = DiagnosticDescriptorHelper.Create(RegexCountRuleId,
CreateLocalizableResourceString(nameof(UseRegexCountTitle)),
CreateLocalizableResourceString(nameof(UseRegexCountMessage)),
DiagnosticCategory.Performance,
RuleLevel.IdeSuggestion,
CreateLocalizableResourceString(nameof(UseRegexCountDescription)),
isPortedFxCopRule: false,
isDataflowRule: false);

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

public override void Initialize(AnalysisContext context)
{
context.EnableConcurrentExecution();
context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
context.RegisterCompilationStartAction(context =>
{
// Require that Regex and supporting types exist.
if (!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextRegularExpressionsGroup, out var groupType) ||
!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextRegularExpressionsMatchCollection, out var matchCollectionType) ||
!context.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextRegularExpressionsRegex, out var regexType))
{
return;
}

// Get the various members needed from Regex types.
var groupSuccessSymbol = groupType.GetMembers("Success").FirstOrDefault();
var matchCollectionCountSymbol = matchCollectionType.GetMembers("Count").FirstOrDefault();
var regexMatchSymbols = regexType.GetMembers("Match");
var regexMatchesSymbols = regexType.GetMembers("Matches");
var regexIsMatchSymbols = regexType.GetMembers("IsMatch");
var regexCountSymbols = regexType.GetMembers("Count");

// Ensure we have the required member symbols (Regex.Count is optional).
if (groupSuccessSymbol is null ||
matchCollectionCountSymbol is null ||
regexMatchSymbols.Length == 0 ||
regexMatchesSymbols.Length == 0 ||
regexIsMatchSymbols.Length == 0)
{
return;
}

// Everything we're looking for is a property, so find all property references.
context.RegisterOperationAction(context =>
{
var initialPropRef = (IPropertyReferenceOperation)context.Operation;

// Regex.Match(...).Success. Look for Group.Success property access.
if (SymbolEqualityComparer.Default.Equals(initialPropRef.Property, groupSuccessSymbol))
{
if (initialPropRef.Instance is IInvocationOperation regexMatchInvocation &&
stephentoub marked this conversation as resolved.
Show resolved Hide resolved
regexMatchSymbols.Contains(regexMatchInvocation.TargetMethod, SymbolEqualityComparer.Default) &&
HasMatchingOverload(regexMatchInvocation.TargetMethod, regexIsMatchSymbols))
{
context.ReportDiagnostic(initialPropRef.CreateDiagnostic(UseRegexIsMatchRuleId));
}
else
{
return;
}

return;
}

// Regex.Matches(...).Count. Look for MatchCollection.Count property access.
if (regexCountSymbols.Length != 0 &&
SymbolEqualityComparer.Default.Equals(initialPropRef.Property, matchCollectionCountSymbol))
{
if (initialPropRef.Instance is IInvocationOperation regexMatchesInvocation &&
regexMatchesSymbols.Contains(regexMatchesInvocation.TargetMethod, SymbolEqualityComparer.Default) &&
HasMatchingOverload(regexMatchesInvocation.TargetMethod, regexCountSymbols))
{
context.ReportDiagnostic(initialPropRef.CreateDiagnostic(UseRegexCountRuleId));
}

return;
}

// Look in overloads to see whether any of the methods there have exactly the same parameters
// by type as does the target method.
static bool HasMatchingOverload(ISymbol target, ImmutableArray<ISymbol> overloads)
{
ImmutableArray<IParameterSymbol> targetParameters = target.GetParameters();
foreach (ISymbol overload in overloads)
{
if (ParameterTypesMatch(targetParameters, overload.GetParameters()))
{
return true;
}
}

return false;
}

// Checks whether the two lists of parameters have the same types in the same order.
static bool ParameterTypesMatch(ImmutableArray<IParameterSymbol> left, ImmutableArray<IParameterSymbol> right)
{
if (left.Length != right.Length)
{
return false;
}

for (int i = 0; i < left.Length; i++)
{
if (!SymbolEqualityComparer.Default.Equals(left[i].Type, right[i].Type))
{
return false;
}
}

return true;
}
}, OperationKind.PropertyReference);
});
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// 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.Composition;
using System.Linq;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.Editing;
using Microsoft.CodeAnalysis.Operations;

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

/// <summary>
/// CA1874: <inheritdoc cref="UseRegexIsMatchMessage"/>
/// CA1875: <inheritdoc cref="UseRegexCountMessage"/>
/// </summary>
[ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
public sealed class UseRegexMembersFixer : CodeFixProvider
{
public sealed override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(
UseRegexMembers.RegexIsMatchRuleId,
UseRegexMembers.RegexCountRuleId);

public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;

public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
Document doc = context.Document;
SemanticModel model = await doc.GetRequiredSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
SyntaxNode root = await doc.GetRequiredSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);

if (root.FindNode(context.Span, getInnermostNodeForTie: true) is SyntaxNode node &&
model.Compilation.TryGetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemTextRegularExpressionsRegex, out var regexType) &&
model.GetOperation(node, context.CancellationToken) is IPropertyReferenceOperation operation &&
operation.Instance is IInvocationOperation regexCall)
{
string title, memberName;
switch (context.Diagnostics[0].Id)
{
case UseRegexMembers.RegexIsMatchRuleId:
title = UseRegexIsMatchFix;
memberName = "IsMatch";
break;

case UseRegexMembers.RegexCountRuleId:
title = UseRegexCountFix;
memberName = "Count";
break;

default:
RoslynDebug.Assert(false, $"Unknown id {context.Diagnostics[0].Id}");
return;
}

context.RegisterCodeFix(
CodeAction.Create(title,
async cancellationToken =>
{
DocumentEditor editor = await DocumentEditor.CreateAsync(doc, cancellationToken).ConfigureAwait(false);

var replacement = editor.Generator.InvocationExpression( // swap in new method name, dropping the subsequent parameter access
editor.Generator.MemberAccessExpression(
regexCall.Instance?.Syntax ?? editor.Generator.TypeExpressionForStaticMemberAccess(regexType),
memberName),
regexCall.Arguments.Select(arg => arg.Syntax)); // use the exact same arguments

editor.ReplaceNode(node, replacement.WithTriviaFrom(node));
return editor.GetChangedDocument();
},
equivalenceKey: title),
context.Diagnostics);
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3163,6 +3163,46 @@ Obecné přetypování (IL unbox.any) používané sekvencí vrácenou metodou E
<target state="translated">Velikost klíče algoritmu asymetrického šifrování {0} je menší než 2048. Použijte radši algoritmus RSA s velikostí klíče alespoň 2048, ECDH nebo ECDSA.</target>
<note />
</trans-unit>
<trans-unit id="UseRegexCountDescription">
<source>'Regex.Count' is simpler and faster than 'Regex.Matches(...).Count'.</source>
<target state="new">'Regex.Count' is simpler and faster than 'Regex.Matches(...).Count'.</target>
<note />
</trans-unit>
<trans-unit id="UseRegexCountFix">
<source>Use 'Regex.Count'</source>
<target state="new">Use 'Regex.Count'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexCountMessage">
<source>Use 'Regex.Count' instead of 'Regex.Matches(...).Count'</source>
<target state="new">Use 'Regex.Count' instead of 'Regex.Matches(...).Count'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexCountTitle">
<source>Use 'Regex.Count'</source>
<target state="new">Use 'Regex.Count'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexIsMatchDescription">
<source>'Regex.IsMatch' is simpler and faster than 'Regex.Match(...).Success'.</source>
<target state="new">'Regex.IsMatch' is simpler and faster than 'Regex.Match(...).Success'.</target>
<note />
</trans-unit>
<trans-unit id="UseRegexIsMatchFix">
<source>Use 'Regex.IsMatch'</source>
<target state="new">Use 'Regex.IsMatch'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexIsMatchMessage">
<source>Use 'Regex.IsMatch' instead of 'Regex.Match(...).Success'</source>
<target state="new">Use 'Regex.IsMatch' instead of 'Regex.Match(...).Success'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexIsMatchTitle">
<source>Use 'Regex.IsMatch'</source>
<target state="new">Use 'Regex.IsMatch'</target>
<note />
</trans-unit>
<trans-unit id="UseSearchValuesCodeFixTitle">
<source>Use 'SearchValues'</source>
<target state="translated">Používat SearchValues</target>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3163,6 +3163,46 @@ Erweiterungen und benutzerdefinierte Konvertierungen werden bei generischen Type
<target state="translated">Die Schlüsselgröße des asymmetrischen Verschlüsselungsalgorithmus "{0}" beträgt weniger als 2048. Wechseln Sie stattdessen zu einer RSA-Verschlüsselung mit ECDH- oder ECDSA-Algorithmus mit einer Schlüsselgröße von mindestens 2048.</target>
<note />
</trans-unit>
<trans-unit id="UseRegexCountDescription">
<source>'Regex.Count' is simpler and faster than 'Regex.Matches(...).Count'.</source>
<target state="new">'Regex.Count' is simpler and faster than 'Regex.Matches(...).Count'.</target>
<note />
</trans-unit>
<trans-unit id="UseRegexCountFix">
<source>Use 'Regex.Count'</source>
<target state="new">Use 'Regex.Count'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexCountMessage">
<source>Use 'Regex.Count' instead of 'Regex.Matches(...).Count'</source>
<target state="new">Use 'Regex.Count' instead of 'Regex.Matches(...).Count'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexCountTitle">
<source>Use 'Regex.Count'</source>
<target state="new">Use 'Regex.Count'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexIsMatchDescription">
<source>'Regex.IsMatch' is simpler and faster than 'Regex.Match(...).Success'.</source>
<target state="new">'Regex.IsMatch' is simpler and faster than 'Regex.Match(...).Success'.</target>
<note />
</trans-unit>
<trans-unit id="UseRegexIsMatchFix">
<source>Use 'Regex.IsMatch'</source>
<target state="new">Use 'Regex.IsMatch'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexIsMatchMessage">
<source>Use 'Regex.IsMatch' instead of 'Regex.Match(...).Success'</source>
<target state="new">Use 'Regex.IsMatch' instead of 'Regex.Match(...).Success'</target>
<note />
</trans-unit>
<trans-unit id="UseRegexIsMatchTitle">
<source>Use 'Regex.IsMatch'</source>
<target state="new">Use 'Regex.IsMatch'</target>
<note />
</trans-unit>
<trans-unit id="UseSearchValuesCodeFixTitle">
<source>Use 'SearchValues'</source>
<target state="translated">"SearchValues" verwenden</target>
Expand Down
Loading
Loading