Skip to content

Commit

Permalink
Add an option to detect overloads with optional parameters (#771)
Browse files Browse the repository at this point in the history
  • Loading branch information
meziantou authored Nov 19, 2024
1 parent 970347a commit ac26787
Show file tree
Hide file tree
Showing 7 changed files with 104 additions and 16 deletions.
16 changes: 16 additions & 0 deletions docs/Rules/MA0032.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,22 @@ class Test
}
````

## Configuration

````
MA0032.allowOverloadsWithOptionalParameters = false
````

````c#
Foo.Bar(); // report when MA0032.allowOverloadsWithOptionalParameters is true
class Foo
{
public static void Bar() => throw null;
public static void Bar(CancellationToken cancellationToken, bool dummy = false) => throw null;
}
````

## Additional resources

- [Enforcing asynchronous code good practices using a Roslyn analyzer](https://www.meziantou.net/enforcing-asynchronous-code-good-practices-using-a-roslyn-analyzer.htm)
16 changes: 16 additions & 0 deletions docs/Rules/MA0040.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,22 @@ class Test

This rule only reports a diagnostic when a `CancellationToken` is available in the scope. [MA0032](MA0032.md) detects the same cases, but reports them even if applying a fix would require you to change the calling method's signature.

## Configuration

````
MA0032.allowOverloadsWithOptionalParameters = false
````

````c#
Foo.Bar(); // report when MA0032.allowOverloadsWithOptionalParameters is true
class Foo
{
public static void Bar() => throw null;
public static void Bar(CancellationToken cancellationToken, bool dummy = false) => throw null;
}
````

## Additional resources

- [Enforcing asynchronous code good practices using a Roslyn analyzer](https://www.meziantou.net/enforcing-asynchronous-code-good-practices-using-a-roslyn-analyzer.htm)
Expand Down
29 changes: 21 additions & 8 deletions src/Meziantou.Analyzer/Internals/OverloadFinder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,40 +22,43 @@ public bool HasOverloadWithAdditionalParameterOfType(
if (currentOperation.SemanticModel is null)
return false;

return FindOverloadWithAdditionalParameterOfType(methodSymbol, syntaxNode: currentOperation.Syntax, includeObsoleteMethods: false, additionalParameterTypes) is not null;
return FindOverloadWithAdditionalParameterOfType(methodSymbol, syntaxNode: currentOperation.Syntax, includeObsoleteMethods: false, allowOptionalParameters: false, additionalParameterTypes) is not null;
}

private IMethodSymbol? FindOverloadWithAdditionalParameterOfType(
IMethodSymbol methodSymbol,
params ITypeSymbol[] additionalParameterTypes)
{
return FindOverloadWithAdditionalParameterOfType(methodSymbol, includeObsoleteMethods: false, additionalParameterTypes);
return FindOverloadWithAdditionalParameterOfType(methodSymbol, includeObsoleteMethods: false, allowOptionalParameters: false, additionalParameterTypes);
}

public IMethodSymbol? FindOverloadWithAdditionalParameterOfType(
IMethodSymbol methodSymbol,
bool includeObsoleteMethods,
bool allowOptionalParameters,
params ITypeSymbol[] additionalParameterTypes)
{
return FindOverloadWithAdditionalParameterOfType(methodSymbol, syntaxNode: null, includeObsoleteMethods, additionalParameterTypes);
return FindOverloadWithAdditionalParameterOfType(methodSymbol, syntaxNode: null, includeObsoleteMethods, allowOptionalParameters, additionalParameterTypes);
}

public IMethodSymbol? FindOverloadWithAdditionalParameterOfType(
IMethodSymbol methodSymbol,
IOperation operation,
bool includeObsoleteMethods,
bool allowOptionalParameters,
params ITypeSymbol[] additionalParameterTypes)
{
if (operation.SemanticModel is null)
return null;

return FindOverloadWithAdditionalParameterOfType(methodSymbol, operation.Syntax, includeObsoleteMethods, additionalParameterTypes);
return FindOverloadWithAdditionalParameterOfType(methodSymbol, operation.Syntax, includeObsoleteMethods, allowOptionalParameters, additionalParameterTypes);
}

public IMethodSymbol? FindOverloadWithAdditionalParameterOfType(
IMethodSymbol methodSymbol,
SyntaxNode? syntaxNode,
bool includeObsoleteMethods,
bool allowOptionalParameters,
params ITypeSymbol[] additionalParameterTypes)
{
if (additionalParameterTypes is null)
Expand Down Expand Up @@ -83,20 +86,20 @@ public bool HasOverloadWithAdditionalParameterOfType(
if (!includeObsoleteMethods && IsObsolete(method))
continue;

if (HasSimilarParameters(methodSymbol, method, additionalParameterTypes))
if (HasSimilarParameters(methodSymbol, method, allowOptionalParameters, additionalParameterTypes))
return method;
}
}

return null;
}

public static bool HasSimilarParameters(IMethodSymbol method, IMethodSymbol otherMethod, params ITypeSymbol[] additionalParameterTypes)
public static bool HasSimilarParameters(IMethodSymbol method, IMethodSymbol otherMethod, bool allowOptionalParameters, params ITypeSymbol[] additionalParameterTypes)
{
if (method.IsEqualTo(otherMethod))
return false;

if (otherMethod.Parameters.Length - method.Parameters.Length != additionalParameterTypes.Length)
if (!allowOptionalParameters && otherMethod.Parameters.Length - method.Parameters.Length != additionalParameterTypes.Length)
return false;

// Most of the time, an overload has the same order for the parameters
Expand Down Expand Up @@ -139,6 +142,7 @@ public static bool HasSimilarParameters(IMethodSymbol method, IMethodSymbol othe
}

// Slower search, allows to find overload with different parameter order
// Also, handle allow optional parameters
{
var otherMethodParameters = otherMethod.Parameters;

Expand Down Expand Up @@ -166,7 +170,16 @@ public static bool HasSimilarParameters(IMethodSymbol method, IMethodSymbol othe
}
}

return otherMethodParameters.Length == 0;
if (otherMethodParameters.Length == 0)
return true;

if (allowOptionalParameters)
{
if (otherMethodParameters.All(p => p.IsOptional))
return true;
}

return false;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -299,10 +299,10 @@ private bool IsPotentialMember(IInvocationOperation operation, IMethodSymbol met
if (methodSymbol.HasAttribute(ObsoleteAttributeSymbol))
return false;

if (OverloadFinder.HasSimilarParameters(method, methodSymbol))
if (OverloadFinder.HasSimilarParameters(method, methodSymbol, allowOptionalParameters: false))
return true;

if (CancellationTokenSymbol is not null && OverloadFinder.HasSimilarParameters(method, methodSymbol, CancellationTokenSymbol))
if (CancellationTokenSymbol is not null && OverloadFinder.HasSimilarParameters(method, methodSymbol, allowOptionalParameters: false, CancellationTokenSymbol))
return true;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using System.Linq;
using System.Runtime.InteropServices;
using System.Threading;
using Meziantou.Analyzer.Configurations;
using Meziantou.Analyzer.Internals;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CSharp;
Expand Down Expand Up @@ -105,7 +106,7 @@ private bool HasExplicitCancellationTokenArgument(IInvocationOperation operation

private sealed record AdditionalParameterInfo(int ParameterIndex, string? Name, bool HasEnumeratorCancellationAttribute);

private bool HasAnOverloadWithCancellationToken(IInvocationOperation operation, [NotNullWhen(true)] out AdditionalParameterInfo? parameterInfo)
private bool HasAnOverloadWithCancellationToken(OperationAnalysisContext context, IInvocationOperation operation, [NotNullWhen(true)] out AdditionalParameterInfo? parameterInfo)
{
parameterInfo = default;
var method = operation.TargetMethod;
Expand All @@ -115,7 +116,8 @@ private bool HasAnOverloadWithCancellationToken(IInvocationOperation operation,
if (IsArgumentImplicitlyDeclared(operation, CancellationTokenSymbol, out parameterInfo))
return true;

var overload = _overloadFinder.FindOverloadWithAdditionalParameterOfType(operation.TargetMethod, operation, includeObsoleteMethods: false, CancellationTokenSymbol);
var allowOptionalParameters = context.Options.GetConfigurationValue(operation, "MA0032.allowOverloadsWithOptionalParameters", defaultValue: false);
var overload = _overloadFinder.FindOverloadWithAdditionalParameterOfType(operation.TargetMethod, operation, includeObsoleteMethods: false, allowOptionalParameters, CancellationTokenSymbol);
if (overload is not null)
{
for (var i = 0; i < overload.Parameters.Length; i++)
Expand Down Expand Up @@ -163,7 +165,7 @@ public void AnalyzeInvocation(OperationAnalysisContext context)
if (HasExplicitCancellationTokenArgument(operation))
return;

if (!HasAnOverloadWithCancellationToken(operation, out var parameterInfo))
if (!HasAnOverloadWithCancellationToken(context, operation, out var parameterInfo))
return;

var availableCancellationTokens = FindCancellationTokens(operation, context.CancellationToken);
Expand Down Expand Up @@ -208,7 +210,7 @@ public void AnalyzeLoop(OperationAnalysisContext context)
return;

// Already handled by AnalyzeInvocation
if (HasAnOverloadWithCancellationToken(invocation, out _))
if (HasAnOverloadWithCancellationToken(context, invocation, out _))
return;

collection = invocation.GetChildOperations().FirstOrDefault();
Expand Down
4 changes: 2 additions & 2 deletions src/Meziantou.Analyzer/Rules/UseIFormatProviderAnalyzer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ public void AnalyzeInvocation(OperationAnalysisContext context)
return;
}

var overload = _overloadFinder.FindOverloadWithAdditionalParameterOfType(operation.TargetMethod, operation, includeObsoleteMethods: false, _cultureSensitiveContext.FormatProviderSymbol);
var overload = _overloadFinder.FindOverloadWithAdditionalParameterOfType(operation.TargetMethod, operation, includeObsoleteMethods: false, allowOptionalParameters: false, _cultureSensitiveContext.FormatProviderSymbol);
if (overload is not null)
{
context.ReportDiagnostic(Rule, operation, operation.TargetMethod.Name, _cultureSensitiveContext.FormatProviderSymbol.ToDisplayString());
Expand All @@ -106,7 +106,7 @@ public void AnalyzeInvocation(OperationAnalysisContext context)

if (_cultureSensitiveContext.CultureInfoSymbol is not null && !operation.HasArgumentOfType(_cultureSensitiveContext.CultureInfoSymbol))
{
var overload = _overloadFinder.FindOverloadWithAdditionalParameterOfType(operation.TargetMethod, includeObsoleteMethods: false, _cultureSensitiveContext.CultureInfoSymbol);
var overload = _overloadFinder.FindOverloadWithAdditionalParameterOfType(operation.TargetMethod, includeObsoleteMethods: false, allowOptionalParameters: false, _cultureSensitiveContext.CultureInfoSymbol);
if (overload is not null)
{
context.ReportDiagnostic(Rule, operation, operation.TargetMethod.Name, _cultureSensitiveContext.CultureInfoSymbol.ToDisplayString());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1187,4 +1187,45 @@ await CreateProjectBuilder()
.ShouldFixCodeWith(Fix)
.ValidateAsync();
}

[Fact]
public async Task SuggestOverloadWithOptionalParameters_AllowOptionalParameters_True()
{
await CreateProjectBuilder()
.WithSourceCode("""
using System.Threading;
using System.Threading.Tasks;

[|Sample.Repro()|];

class Sample
{
public static void Repro() => throw null;
public static void Repro(CancellationToken cancellationToken, bool dummy = false) => throw null;
}
""")
.AddAnalyzerConfiguration("MA0032.allowOverloadsWithOptionalParameters", "true")
.WithOutputKind(Microsoft.CodeAnalysis.OutputKind.ConsoleApplication)
.ValidateAsync();
}

[Fact]
public async Task SuggestOverloadWithOptionalParameters_AllowOptionalParameters_False()
{
await CreateProjectBuilder()
.WithSourceCode("""
using System.Threading;
using System.Threading.Tasks;

Sample.Repro();

class Sample
{
public static void Repro() => throw null;
public static void Repro(CancellationToken cancellationToken, bool dummy = false) => throw null;
}
""")
.WithOutputKind(Microsoft.CodeAnalysis.OutputKind.ConsoleApplication)
.ValidateAsync();
}
}

0 comments on commit ac26787

Please sign in to comment.