From 71360a439def43105a4a638a7f5d99908c94710b Mon Sep 17 00:00:00 2001 From: Alexander Luzgarev Date: Mon, 6 Jul 2020 19:01:01 +0200 Subject: [PATCH 1/4] Support basic cases in UseRangeOperator --- ...ngeOperatorDiagnosticAnalyzer.InfoCache.cs | 36 ++++ ...eRangeOperatorDiagnosticAnalyzer.Result.cs | 6 +- ...SharpUseRangeOperatorDiagnosticAnalyzer.cs | 47 +++++- .../UseIndexOrRangeOperator/Helpers.cs | 15 ++ .../CSharpUseRangeOperatorCodeFixProvider.cs | 29 +++- .../UseRangeOperatorTests.cs | 154 ++++++++++++++++++ 6 files changed, 275 insertions(+), 12 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs index 01d679d882f7f..95459b5f7c749 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs @@ -69,6 +69,42 @@ public bool TryGetMemberInfo(IMethodSymbol method, out MemberInfo memberInfo) return memberInfo.LengthLikeProperty != null; } + public bool TryGetMemberInfoOneArgument(IMethodSymbol method, out MemberInfo memberInfo) + { + if (!IsSliceLikeMethodWithOneArgument(method)) + { + memberInfo = default; + return false; + } + + if (_methodToMemberInfo.TryGetValue(method, out memberInfo)) + { + return memberInfo.LengthLikeProperty != null; + } + + // 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() + .FirstOrDefault(s => IsSliceLikeMethod(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)); diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.Result.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.Result.cs index babff3a37a7e3..b36908c5ccdad 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.Result.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.Result.cs @@ -12,7 +12,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. @@ -28,6 +28,10 @@ public readonly struct Result public readonly IMethodSymbol SliceLikeMethod; public readonly MemberInfo MemberInfo; public readonly IOperation Op1; + + /// + /// Can be null, if we are dealing with one-argument call to a slice-like method. + /// public readonly IOperation Op2; public Result( diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs index 12b1e496a25ce..eaa3202300f4c 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs @@ -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 => AnalyzeInvocationWithOneArgument(invocation, infoCache, invocationSyntax, option), + 2 => AnalyzeInvocationWithTwoArguments(invocation, infoCache, invocationSyntax, option), + _ => null, + }; + } + + private static Result? AnalyzeInvocationWithOneArgument( + IInvocationOperation invocation, + InfoCache infoCache, + InvocationExpressionSyntax invocationSyntax, + CodeStyleOption2 option) + { + // See if the call is to something slice-like. + var targetMethod = invocation.TargetMethod; + + // Try to see if we're calling into some sort of Slice method with a matching + // indexer or overload + 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, + startOperation, + null); // secondOperation is null: the range will run to the end. + } + + private static Result? AnalyzeInvocationWithTwoArguments( + IInvocationOperation invocation, + InfoCache infoCache, + InvocationExpressionSyntax invocationSyntax, + CodeStyleOption2 option) + { // See if the call is to something slice-like. var targetMethod = invocation.TargetMethod; diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs index 7152b41006ecd..139ae1f7eb22c 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs @@ -91,6 +91,21 @@ public static bool IsSliceLikeMethod(IMethodSymbol method) IsSliceFirstParameter(method.OriginalDefinition.Parameters[0]) && IsSliceSecondParameter(method.OriginalDefinition.Parameters[1]); + /// + /// 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. + /// + public static bool IsSliceLikeMethodWithOneArgument(IMethodSymbol method) + => method != null && + IsPublicInstance(method) && + method.Parameters.Length == 1 && + // 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]); + private static bool IsSliceFirstParameter(IParameterSymbol parameter) => parameter.Type.SpecialType == SpecialType.System_Int32 && (parameter.Name == "start" || parameter.Name == "startIndex"); diff --git a/src/Analyzers/CSharp/CodeFixes/UseIndexOrRangeOperator/CSharpUseRangeOperatorCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseIndexOrRangeOperator/CSharpUseRangeOperatorCodeFixProvider.cs index d48d9b8523300..079c1c69ae462 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseIndexOrRangeOperator/CSharpUseRangeOperatorCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseIndexOrRangeOperator/CSharpUseRangeOperatorCodeFixProvider.cs @@ -150,15 +150,28 @@ 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; + + if (!(endOperation is 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); + 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)) + { + endExpr = null; + } + } + else { + // We are dealing with one-argument case, so we should leave off + // the end part of the range: `start..`. endExpr = null; } diff --git a/src/Analyzers/CSharp/Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs b/src/Analyzers/CSharp/Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs index 9a594c1c376c8..f608f3b44e2eb 100644 --- a/src/Analyzers/CSharp/Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs +++ b/src/Analyzers/CSharp/Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs @@ -204,6 +204,96 @@ void Goo(string s, int bar, int baz) }.RunAsync(); } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseRangeOperator)] + public async Task TestSubstringOneArgument() + { + var source = +@" +class C +{ + void Goo(string s) + { + var v = s.Substring([|1|]); + } +}"; + var fixedSource = +@" +class C +{ + void Goo(string s) + { + var v = s[1..]; + } +}"; + + await new VerifyCS.Test + { + ReferenceAssemblies = ReferenceAssemblies.NetCore.NetCoreApp31, + TestCode = source, + FixedCode = fixedSource, + }.RunAsync(); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseRangeOperator)] + public async Task TestSliceOneArgument() + { + var source = +@" +class C +{ + void Goo(Span s) + { + var v = s.Slice([|1|]); + } +}"; + var fixedSource = +@" +class C +{ + void Goo(Span s) + { + var v = s[1..]; + } +}"; + + await new VerifyCS.Test + { + ReferenceAssemblies = ReferenceAssemblies.NetCore.NetCoreApp31, + TestCode = source, + FixedCode = fixedSource, + }.RunAsync(); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseRangeOperator)] + public async Task TestExpressionOneArgument() + { + var source = +@" +class C +{ + void Goo(string s, int bar) + { + var v = s.Substring([|bar|]); + } +}"; + var fixedSource = +@" +class C +{ + void Goo(string s, int bar) + { + var v = s[bar..]; + } +}"; + + await new VerifyCS.Test + { + ReferenceAssemblies = ReferenceAssemblies.NetCore.NetCoreApp31, + TestCode = source, + FixedCode = fixedSource, + }.RunAsync(); + } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseRangeOperator)] public async Task TestConstantSubtraction1() { @@ -416,6 +506,38 @@ void Goo(string s, string t) }.RunAsync(); } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseRangeOperator)] + public async Task TestFixAllInvocationToElementAccess2() + { + // Note: once the IOp tree has support for range operators, this should + // simplify even further. + var source = +@" +class C +{ + void Goo(string s, string t) + { + var v = t.Substring([|s.Substring([|1|])[0]|]); + } +}"; + var fixedSource = +@" +class C +{ + void Goo(string s, string t) + { + var v = t[s[1..][0]..]; + } +}"; + + await new VerifyCS.Test + { + ReferenceAssemblies = ReferenceAssemblies.NetCore.NetCoreApp31, + TestCode = source, + FixedCode = fixedSource, + }.RunAsync(); + } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseRangeOperator)] public async Task TestWithTypeWithActualSliceMethod1() { @@ -479,5 +601,37 @@ void Goo(Span s) FixedCode = fixedSource, }.RunAsync(); } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseRangeOperator)] + public async Task TestWithTypeWithActualSliceMethod3() + { + var source = +@" +using System; +class C +{ + void Goo(Span s) + { + var v = s.Slice([|1|]); + } +}"; + var fixedSource = +@" +using System; +class C +{ + void Goo(Span s) + { + var v = s[1..]; + } +}"; + + await new VerifyCS.Test + { + ReferenceAssemblies = ReferenceAssemblies.NetCore.NetCoreApp31, + TestCode = source, + FixedCode = fixedSource, + }.RunAsync(); + } } } From 320e434b94e5e9f959efa3102a60b5088aeac345 Mon Sep 17 00:00:00 2001 From: Alexander Luzgarev Date: Mon, 6 Jul 2020 21:54:56 +0200 Subject: [PATCH 2/4] Fix unit test --- .../Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Analyzers/CSharp/Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs b/src/Analyzers/CSharp/Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs index f608f3b44e2eb..bfbbba16f43b4 100644 --- a/src/Analyzers/CSharp/Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs +++ b/src/Analyzers/CSharp/Tests/UseIndexOrRangeOperator/UseRangeOperatorTests.cs @@ -239,6 +239,7 @@ public async Task TestSliceOneArgument() { var source = @" +using System; class C { void Goo(Span s) @@ -248,6 +249,7 @@ void Goo(Span s) }"; var fixedSource = @" +using System; class C { void Goo(Span s) From a24c04594d8af09b44fcf7a8d325e1e4753ef6c2 Mon Sep 17 00:00:00 2001 From: Alexander Luzgarev Date: Mon, 6 Jul 2020 21:59:15 +0200 Subject: [PATCH 3/4] Style fixes --- ...ngeOperatorDiagnosticAnalyzer.InfoCache.cs | 53 +++++++++---------- ...eRangeOperatorDiagnosticAnalyzer.Result.cs | 6 ++- ...SharpUseRangeOperatorDiagnosticAnalyzer.cs | 14 ++--- .../UseIndexOrRangeOperator/Helpers.cs | 42 ++++++++------- .../CSharpUseRangeOperatorCodeFixProvider.cs | 17 ++---- 5 files changed, 65 insertions(+), 67 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs index 95459b5f7c749..d985ed14794d1 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs @@ -45,7 +45,7 @@ public InfoCache(Compilation compilation) { var substringMethod = stringType.GetMembers(nameof(string.Substring)) .OfType() - .FirstOrDefault(m => IsSliceLikeMethod(m)); + .FirstOrDefault(m => IsTwoArgumentSliceLikeMethod(m)); _methodToMemberInfo[substringMethod] = ComputeMemberInfo(substringMethod, requireRangeMember: false); } @@ -54,12 +54,12 @@ public InfoCache(Compilation compilation) private static IMethodSymbol GetSliceLikeMethod(INamedTypeSymbol namedType) => namedType.GetMembers() .OfType() - .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; @@ -71,43 +71,42 @@ public bool TryGetMemberInfo(IMethodSymbol method, out MemberInfo memberInfo) public bool TryGetMemberInfoOneArgument(IMethodSymbol method, out MemberInfo memberInfo) { - if (!IsSliceLikeMethodWithOneArgument(method)) + if (!IsOneArgumentSliceLikeMethod(method)) { memberInfo = default; return false; } - if (_methodToMemberInfo.TryGetValue(method, out memberInfo)) + if (!_methodToMemberInfo.TryGetValue(method, out memberInfo)) { - return memberInfo.LengthLikeProperty != null; - } + // 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() + .FirstOrDefault(s => IsTwoArgumentSliceLikeMethod(s)); + if (overloadWithTwoArguments is null) + { + memberInfo = default; + return false; + } - // 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() - .FirstOrDefault(s => IsSliceLikeMethod(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; } - // 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. @@ -142,7 +141,7 @@ private MemberInfo ComputeMemberInfo(IMethodSymbol sliceLikeMethod, bool require var actualSliceMethod = sliceLikeMethod.ContainingType.GetMembers(nameof(Span.Slice)) .OfType() - .FirstOrDefault(s => IsSliceLikeMethod(s)); + .FirstOrDefault(s => IsTwoArgumentSliceLikeMethod(s)); if (actualSliceMethod != null) { return new MemberInfo(lengthLikeProperty, overloadedMethodOpt: null); diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.Result.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.Result.cs index b36908c5ccdad..02050fcc995b7 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.Result.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.Result.cs @@ -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; @@ -32,13 +34,13 @@ public readonly struct Result /// /// Can be null, if we are dealing with one-argument call to a slice-like method. /// - public readonly IOperation Op2; + public readonly IOperation? Op2; public Result( ResultKind kind, CodeStyleOption2 option, IInvocationOperation invocationOperation, InvocationExpressionSyntax invocation, IMethodSymbol sliceLikeMethod, MemberInfo memberInfo, - IOperation op1, IOperation op2) + IOperation op1, IOperation? op2) { Kind = kind; Option = option; diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs index eaa3202300f4c..bff9a2ec2ed0a 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs @@ -120,7 +120,7 @@ private void AnalyzeInvocation( return invocation.Arguments.Length switch { 1 => AnalyzeInvocationWithOneArgument(invocation, infoCache, invocationSyntax, option), - 2 => AnalyzeInvocationWithTwoArguments(invocation, infoCache, invocationSyntax, option), + 2 => AnalyzeTwoArgumentInvocation(invocation, infoCache, invocationSyntax, option), _ => null, }; } @@ -131,11 +131,11 @@ private void AnalyzeInvocation( InvocationExpressionSyntax invocationSyntax, CodeStyleOption2 option) { - // See if the call is to something slice-like. var targetMethod = invocation.TargetMethod; - // Try to see if we're calling into some sort of Slice method with a matching - // indexer or overload + // 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; @@ -149,11 +149,11 @@ private void AnalyzeInvocation( invocationSyntax, targetMethod, memberInfo, - startOperation, - null); // secondOperation is null: the range will run to the end. + op1: startOperation, + op2: null); // The range will run to the end. } - private static Result? AnalyzeInvocationWithTwoArguments( + private static Result? AnalyzeTwoArgumentInvocation( IInvocationOperation invocation, InfoCache infoCache, InvocationExpressionSyntax invocationSyntax, diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs index 139ae1f7eb22c..a21c93305c7d8 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/Helpers.cs @@ -80,31 +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. /// - 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]); + } /// /// 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. /// - public static bool IsSliceLikeMethodWithOneArgument(IMethodSymbol method) - => method != null && - IsPublicInstance(method) && - method.Parameters.Length == 1 && - // 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]); + 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 && diff --git a/src/Analyzers/CSharp/CodeFixes/UseIndexOrRangeOperator/CSharpUseRangeOperatorCodeFixProvider.cs b/src/Analyzers/CSharp/CodeFixes/UseIndexOrRangeOperator/CSharpUseRangeOperatorCodeFixProvider.cs index 079c1c69ae462..d0b5cebee6f7f 100644 --- a/src/Analyzers/CSharp/CodeFixes/UseIndexOrRangeOperator/CSharpUseRangeOperatorCodeFixProvider.cs +++ b/src/Analyzers/CSharp/CodeFixes/UseIndexOrRangeOperator/CSharpUseRangeOperatorCodeFixProvider.cs @@ -151,7 +151,7 @@ private static RangeExpressionSyntax CreateComputedRange(Result result) var startExpr = (ExpressionSyntax)startOperation.Syntax; var endFromEnd = false; - ExpressionSyntax endExpr; + ExpressionSyntax endExpr = null; if (!(endOperation is null)) { @@ -159,21 +159,14 @@ private static RangeExpressionSyntax CreateComputedRange(Result result) // 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); - 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)) + // 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 = null; + endExpr = (ExpressionSyntax)endOperation.Syntax; } } - else - { - // We are dealing with one-argument case, so we should leave off - // the end part of the range: `start..`. - endExpr = null; - } // If we're starting the range operation from 0, then we can just leave off the start of // the range. i.e. `..end` From 806127eab7732bf27e21c4d31d066d8b450463d0 Mon Sep 17 00:00:00 2001 From: Alexander Luzgarev Date: Mon, 6 Jul 2020 22:05:20 +0200 Subject: [PATCH 4/4] More style fixes --- .../CSharpUseRangeOperatorDiagnosticAnalyzer.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs index bff9a2ec2ed0a..c87de1349f35d 100644 --- a/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs +++ b/src/Analyzers/CSharp/Analyzers/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs @@ -119,13 +119,13 @@ private void AnalyzeInvocation( return invocation.Arguments.Length switch { - 1 => AnalyzeInvocationWithOneArgument(invocation, infoCache, invocationSyntax, option), + 1 => AnalyzeOneArgumentInvocation(invocation, infoCache, invocationSyntax, option), 2 => AnalyzeTwoArgumentInvocation(invocation, infoCache, invocationSyntax, option), _ => null, }; } - private static Result? AnalyzeInvocationWithOneArgument( + private static Result? AnalyzeOneArgumentInvocation( IInvocationOperation invocation, InfoCache infoCache, InvocationExpressionSyntax invocationSyntax,