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

Support basic cases in UseRangeOperator #45693

Merged
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
Expand Up @@ -45,7 +45,7 @@ public InfoCache(Compilation compilation)
{
var substringMethod = stringType.GetMembers(nameof(string.Substring))
.OfType<IMethodSymbol>()
.FirstOrDefault(m => IsSliceLikeMethod(m));
.FirstOrDefault(m => IsTwoArgumentSliceLikeMethod(m));

_methodToMemberInfo[substringMethod] = ComputeMemberInfo(substringMethod, requireRangeMember: false);
}
Expand All @@ -54,12 +54,12 @@ public InfoCache(Compilation compilation)
private static IMethodSymbol GetSliceLikeMethod(INamedTypeSymbol namedType)
=> namedType.GetMembers()
.OfType<IMethodSymbol>()
.Where(m => IsSliceLikeMethod(m))
.Where(m => IsTwoArgumentSliceLikeMethod(m))
.FirstOrDefault();

public bool TryGetMemberInfo(IMethodSymbol method, out MemberInfo memberInfo)
{
if (!IsSliceLikeMethod(method))
if (!IsTwoArgumentSliceLikeMethod(method))
{
memberInfo = default;
return false;
Expand All @@ -69,9 +69,44 @@ public bool TryGetMemberInfo(IMethodSymbol method, out MemberInfo memberInfo)
return memberInfo.LengthLikeProperty != null;
}

public bool TryGetMemberInfoOneArgument(IMethodSymbol method, out MemberInfo memberInfo)
{
if (!IsOneArgumentSliceLikeMethod(method))
{
memberInfo = default;
return false;
}

if (!_methodToMemberInfo.TryGetValue(method, out memberInfo))
{
// Find overload of our method that is a slice-like method with two arguments.
// Computing member info for this method will also check that the containing type
// has an int32 'Length' or 'Count' property, and has a suitable indexer,
// so we don't have to.
var overloadWithTwoArguments = method.ContainingType
.GetMembers(method.Name)
.OfType<IMethodSymbol>()
.FirstOrDefault(s => IsTwoArgumentSliceLikeMethod(s));
if (overloadWithTwoArguments is null)
{
memberInfo = default;
return false;
}

// Since the search is expensive, we keep both the original one-argument and
// two-arguments overload as keys in the cache, pointing to the same
// member information object.
var newMemberInfo = _methodToMemberInfo.GetOrAdd(overloadWithTwoArguments, _ => ComputeMemberInfo(overloadWithTwoArguments, requireRangeMember: true));
_methodToMemberInfo.GetOrAdd(method, _ => newMemberInfo);
memberInfo = newMemberInfo;
}

return memberInfo.LengthLikeProperty != null;
}

private MemberInfo ComputeMemberInfo(IMethodSymbol sliceLikeMethod, bool requireRangeMember)
{
Debug.Assert(IsSliceLikeMethod(sliceLikeMethod));
Debug.Assert(IsTwoArgumentSliceLikeMethod(sliceLikeMethod));

// Check that the type has an int32 'Length' or 'Count' property. If not, we don't
// consider it something indexable.
Expand Down Expand Up @@ -106,7 +141,7 @@ private MemberInfo ComputeMemberInfo(IMethodSymbol sliceLikeMethod, bool require
var actualSliceMethod =
sliceLikeMethod.ContainingType.GetMembers(nameof(Span<int>.Slice))
.OfType<IMethodSymbol>()
.FirstOrDefault(s => IsSliceLikeMethod(s));
.FirstOrDefault(s => IsTwoArgumentSliceLikeMethod(s));
if (actualSliceMethod != null)
{
return new MemberInfo(lengthLikeProperty, overloadedMethodOpt: null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

#nullable enable

using Microsoft.CodeAnalysis.CodeStyle;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Operations;
Expand All @@ -12,7 +14,7 @@ internal partial class CSharpUseRangeOperatorDiagnosticAnalyzer
{
public enum ResultKind
{
// like s.Substring(expr, s.Length - expr). 'expr' has to match on both sides.
// like s.Substring(expr, s.Length - expr) or s.Substring(expr). 'expr' has to match on both sides.
Computed,

// like s.Substring(constant1, s.Length - constant2). the constants don't have to match.
Expand All @@ -28,13 +30,17 @@ public readonly struct Result
public readonly IMethodSymbol SliceLikeMethod;
public readonly MemberInfo MemberInfo;
public readonly IOperation Op1;
public readonly IOperation Op2;

/// <summary>
/// Can be null, if we are dealing with one-argument call to a slice-like method.
/// </summary>
public readonly IOperation? Op2;

public Result(
ResultKind kind, CodeStyleOption2<bool> option,
IInvocationOperation invocationOperation, InvocationExpressionSyntax invocation,
IMethodSymbol sliceLikeMethod, MemberInfo memberInfo,
IOperation op1, IOperation op2)
IOperation op1, IOperation? op2)
{
Kind = kind;
Option = option;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,13 +111,54 @@ private void AnalyzeInvocation(
}
}

// look for `s.Slice(e1, end - e2)`
if (invocation.Instance is null ||
invocation.Arguments.Length != 2)
// look for `s.Slice(e1, end - e2)` or `s.Slice(e1)`
if (invocation.Instance is null)
{
return null;
}

return invocation.Arguments.Length switch
{
1 => AnalyzeOneArgumentInvocation(invocation, infoCache, invocationSyntax, option),
2 => AnalyzeTwoArgumentInvocation(invocation, infoCache, invocationSyntax, option),
_ => null,
};
}

private static Result? AnalyzeOneArgumentInvocation(
IInvocationOperation invocation,
InfoCache infoCache,
InvocationExpressionSyntax invocationSyntax,
CodeStyleOption2<bool> option)
{
var targetMethod = invocation.TargetMethod;

// We are dealing with a call like `.Substring(expr)`.
// Ensure that there is an overload with signature like `Substring(int start, int length)`
// and there is a suitable indexer to replace this with `[expr..]`.
if (!infoCache.TryGetMemberInfoOneArgument(targetMethod, out var memberInfo))
{
return null;
}

var startOperation = invocation.Arguments[0].Value;
return new Result(
ResultKind.Computed,
option,
invocation,
invocationSyntax,
targetMethod,
memberInfo,
op1: startOperation,
op2: null); // The range will run to the end.
}

private static Result? AnalyzeTwoArgumentInvocation(
IInvocationOperation invocation,
InfoCache infoCache,
InvocationExpressionSyntax invocationSyntax,
CodeStyleOption2<bool> option)
{
// See if the call is to something slice-like.
var targetMethod = invocation.TargetMethod;

Expand Down
39 changes: 29 additions & 10 deletions src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,35 @@ public static bool IsIntIndexingMethod(IMethodSymbol method)
/// names of the parameters are checked to ensure they are appropriate slice-like. These
/// names were picked by examining the patterns in the BCL for slicing members.
/// </summary>
public static bool IsSliceLikeMethod(IMethodSymbol method)
=> method != null &&
IsPublicInstance(method) &&
method.Parameters.Length == 2 &&
// From: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-8.0/ranges.md#decisions-made-during-implementation
//
// When looking for the pattern members, we look for original definitions, not
// constructed members
IsSliceFirstParameter(method.OriginalDefinition.Parameters[0]) &&
IsSliceSecondParameter(method.OriginalDefinition.Parameters[1]);
public static bool IsTwoArgumentSliceLikeMethod(IMethodSymbol method)
{
// From: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-8.0/ranges.md#decisions-made-during-implementation
//
// When looking for the pattern members, we look for original definitions, not
// constructed members
return method != null &&
IsPublicInstance(method) &&
method.Parameters.Length == 2 &&
IsSliceFirstParameter(method.OriginalDefinition.Parameters[0]) &&
IsSliceSecondParameter(method.OriginalDefinition.Parameters[1]);
}

/// <summary>
/// Look for methods like "SomeType MyType.Slice(int start)". Note that the
/// name of the parameter is checked to ensure it is appropriate slice-like.
/// This name was picked by examining the patterns in the BCL for slicing members.
/// </summary>
public static bool IsOneArgumentSliceLikeMethod(IMethodSymbol method)
{
// From: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-8.0/ranges.md#decisions-made-during-implementation
//
// When looking for the pattern members, we look for original definitions, not
// constructed members
return method != null &&
IsPublicInstance(method) &&
method.Parameters.Length == 1 &&
IsSliceFirstParameter(method.OriginalDefinition.Parameters[0]);
}

private static bool IsSliceFirstParameter(IParameterSymbol parameter)
=> parameter.Type.SpecialType == SpecialType.System_Int32 &&
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,22 @@ private static RangeExpressionSyntax CreateComputedRange(Result result)
var startFromEnd = IsFromEnd(lengthLikeProperty, instance, ref startOperation);
var startExpr = (ExpressionSyntax)startOperation.Syntax;

// Similarly, if our end-op is actually equivalent to `expr.Length - val`, then just
// change our end-op to be `val` and record that we should emit it as `^val`.
var endFromEnd = IsFromEnd(lengthLikeProperty, instance, ref endOperation);
var endExpr = (ExpressionSyntax)endOperation.Syntax;

// If the range operation goes to 'expr.Length' then we can just leave off the end part
// of the range. i.e. `start..`
if (IsInstanceLengthCheck(lengthLikeProperty, instance, endOperation))
var endFromEnd = false;
ExpressionSyntax endExpr = null;

if (!(endOperation is null))
{
endExpr = null;
// We need to do the same for the second argument, since it's present.
// Similarly, if our end-op is actually equivalent to `expr.Length - val`, then just
// change our end-op to be `val` and record that we should emit it as `^val`.
endFromEnd = IsFromEnd(lengthLikeProperty, instance, ref endOperation);

// Check if the range goes to 'expr.Length'; if it does, we leave off
// the end part of the range, i.e. `start..`.
if (!IsInstanceLengthCheck(lengthLikeProperty, instance, endOperation))
{
endExpr = (ExpressionSyntax)endOperation.Syntax;
}
}

// If we're starting the range operation from 0, then we can just leave off the start of
Expand Down
Loading