Simplify and document.

src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.InfoCache.cs

@@ -38,9 +38,15 @@ public InfoCache(Compilation compilation)
                 _methodToMemberInfo[indexer.GetMethod] = ComputeMemberInfo(indexer.GetMethod, requireIndexMember: false);
             }
 
-            public bool TryGetMemberInfo(IMethodSymbol intIndexingMethod, out MemberInfo memberInfo)
+            public bool TryGetMemberInfo(IMethodSymbol methodSymbol, out MemberInfo memberInfo)
             {
-                memberInfo = _methodToMemberInfo.GetOrAdd(intIndexingMethod, m => ComputeMemberInfo(m, requireIndexMember: true));
+                if (!IsIntIndexingMethod(methodSymbol))
+                {
+                    memberInfo = default;
+                    return false;
+                }
+
+                memberInfo = _methodToMemberInfo.GetOrAdd(methodSymbol, m => ComputeMemberInfo(m, requireIndexMember: true));
                 return memberInfo.LengthLikeProperty != null;
             }
 

src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs

@@ -2,6 +2,7 @@
 
 using System.Collections.Immutable;
 using System.Composition;
+using System.Threading;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.CSharp.CodeStyle;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -10,19 +11,27 @@
 
 namespace Microsoft.CodeAnalysis.CSharp.UseIndexOrRangeOperator
 {
-    using System;
-    using System.Threading;
     using static Helpers;
 
     /// <summary>
-    /// Analyzer that looks for code like `s[s.Length - n]` and offers to change that to `s[^n]`. In
-    /// order to do this, the type must look 'indexable'.  Meaning, it must have an int-returning
-    /// property called 'Length' or 'Count', and it must have both an int-indexer, and a
-    /// System.Index indexer.
+    /// Analyzer that looks for code like: 
+    /// 
+    /// 1) `s[s.Length - n]` and offers to change that to `s[^n]`. and.
+    /// 2) `s.Get(s.Length - n)` and offers to change that to `s.Get(^n)`
+    ///
+    /// In order to do convert between indexers, the type must look 'indexable'.  Meaning, it must
+    /// have an int-returning property called 'Length' or 'Count', and it must have both an
+    /// int-indexer, and a System.Index-indexer.  In order to convert between methods, the type
+    /// must have identical overloads except that one takes an int, and the other a System.Index.
     ///
     /// It is assumed that if the type follows this shape that it is well behaved and that this
     /// transformation will preserve semantics.  If this assumption is not good in practice, we
     /// could always limit the feature to only work on a whitelist of known safe types.
+    /// 
+    /// Note that this feature only works if the code literally has `expr1.Length - expr2`.  If
+    /// code has this, and is calling into a method that takes either an int or a System.Index,
+    /// it feels very safe to assume this is well behaved and switching to `^expr2` is going to
+    /// preserve semantics.
     /// </summary>
     [DiagnosticAnalyzer(LanguageNames.CSharp), Shared]
     internal partial class CSharpUseIndexOperatorDiagnosticAnalyzer : AbstractCodeStyleDiagnosticAnalyzer
@@ -38,9 +47,9 @@ protected override void InitializeWorker(AnalysisContext context)
         {
             context.RegisterCompilationStartAction(startContext =>
             {
-                // We're going to be checking every property-reference in the compilation. Cache
-                // information we compute in this object so we don't have to continually recompute
-                // it.
+                // We're going to be checking every property-reference and invocation in the
+                // compilation. Cache information we compute in this object so we don't have to
+                // continually recompute it.
                 var infoCache = new InfoCache(startContext.Compilation);
                 context.RegisterOperationAction(
                     c => AnalyzePropertyReference(c, infoCache),
@@ -128,9 +137,9 @@ private void AnalyzeInvocation(OperationAnalysisContext context, InfoCache infoC
                 return;
             }
 
-            // look for `s[s.Length - value]` and convert to `s[^val]`
+            // look for `s[s.Length - value]` or `s.Get(s.Length- value)`.
 
-            // Needs to have the one arg for `[s.Length - value]`, and that arg needs to be
+            // Needs to have the one arg for `s.Length - value`, and that arg needs to be
             // a subtraction.
             if (instance is null ||
                 arguments.Length != 1 ||
@@ -139,14 +148,13 @@ private void AnalyzeInvocation(OperationAnalysisContext context, InfoCache infoC
                 return;
             }
 
-            // Ok, looks promising.  We're indexing in with some subtraction expression.
-            // Examine the type this indexer is in to see if there's another meber that
-            // takes a System.Index that we can convert to.
+            // Ok, looks promising.  We're indexing in with some subtraction expression. Examine the
+            // type this indexer is in to see if there's another member that takes a System.Index
+            // that we can convert to.
             //
-            // Also ensure that the left side of the subtraction : `s.Length - value` is
-            // actually getting the length off hte same instance we're indexing into.
-            if (!IsIntIndexingMethod(targetMethod) ||
-                !infoCache.TryGetMemberInfo(targetMethod, out var memberInfo) ||
+            // Also ensure that the left side of the subtraction : `s.Length - value` is actually
+            // getting the length off the same instance we're indexing into.
+            if (!infoCache.TryGetMemberInfo(targetMethod, out var memberInfo) ||
                 !IsInstanceLengthCheck(memberInfo.LengthLikeProperty, instance, subtraction.LeftOperand))
             {
                 return;

src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.InfoCache.cs

@@ -45,9 +45,15 @@ private IMethodSymbol GetSliceLikeMethod(INamedTypeSymbol namedType)
                             .Where(m => IsSliceLikeMethod(m))
                             .FirstOrDefault();
 
-            public bool TryGetMemberInfo(IMethodSymbol sliceLikeMethod, out MemberInfo memberInfo)
+            public bool TryGetMemberInfo(IMethodSymbol method, out MemberInfo memberInfo)
             {
-                memberInfo = _methodToMemberInfo.GetOrAdd(sliceLikeMethod, m => ComputeMemberInfo(m, requireRangeMember: true));
+                if (!IsSliceLikeMethod(method))
+                {
+                    memberInfo = default;
+                    return false;
+                }
+
+                memberInfo = _methodToMemberInfo.GetOrAdd(method, m => ComputeMemberInfo(m, requireRangeMember: true));
                 return memberInfo.LengthLikeProperty != null;
             }
 

src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseRangeOperatorDiagnosticAnalyzer.cs

@@ -15,9 +15,12 @@ namespace Microsoft.CodeAnalysis.CSharp.UseIndexOrRangeOperator
 
     /// <summary>
     /// Analyzer that looks for several variants of code like `s.Slice(start, end - start)` and
-    /// offers to update to `s[start..end]`.  In order to convert, the type being called on needs a
-    /// slice-like method that takes two ints, and returns an instance of the same type. It also
-    /// needs a Length/Count property, as well as an indexer that takes a System.Range instance.
+    /// offers to update to `s[start..end]` or `s.Slice(start..end)`.  In order to convert to the
+    /// indexer, the type being called on needs a slice-like method that takes two ints, and returns
+    /// an instance of the same type. It also needs a Length/Count property, as well as an indexer
+    /// that takes a System.Range instance.  In order to convert between methods, there need to be
+    /// two overloads that are equivalent except that one takes two ints, and the other takes a
+    /// System.Range.
     ///
     /// It is assumed that if the type follows this shape that it is well behaved and that this
     /// transformation will preserve semantics.  If this assumption is not good in practice, we
@@ -83,25 +86,8 @@ protected override void InitializeWorker(AnalysisContext context)
                 return;
             }
 
-            // See if the call is to something slice-like.
-            var targetMethod = invocation.TargetMethod;
-            if (!IsSliceLikeMethod(invocation.TargetMethod))
-            {
-                return;
-            }
 
-            var sliceLikeMethod = targetMethod;
-            // See if this is a type we can use range-indexer for, and also if this is a call to the
-            // Slice-Like method we've found for that type.  Use the InfoCache so that we can reuse
-            // any previously computed values for this type.
-            if (!infoCache.TryGetMemberInfo(sliceLikeMethod, out var memberInfo))
-            {
-                return;
-            }
-
-            // look for `s.Slice(start, end - start)` and convert to `s[Range]`
-
-            // Needs to have the two args for `start` and `end - start`
+            // look for `s.Slice(e1, end - e2)`
             if (invocation.Instance is null ||
                 invocation.Instance.Syntax is null ||
                 invocation.Arguments.Length != 2)
@@ -109,21 +95,26 @@ protected override void InitializeWorker(AnalysisContext context)
                 return;
             }
 
-            // Arg2 needs to be a subtraction for: `end - start`
-            var arg2 = invocation.Arguments[1];
-            if (!IsSubtraction(arg2, out var subtraction))
+            // See if the call is to something slice-like.
+            var targetMethod = invocation.TargetMethod;
+
+            // Second arg needs to be a subtraction for: `end - e2`.  Once we've seen that we have
+            // that, try to see if we're calling into some sort of Slice method with a matching
+            // indexer or overload
+            if (!IsSubtraction(invocation.Arguments[1], out var subtraction) ||
+                !infoCache.TryGetMemberInfo(targetMethod, out var memberInfo))
             {
                 return;
             }
 
-            // See if we have: (start, end - start).  The start operation has to be the same as the
-            // right side of the subtraction.
+            // See if we have: (start, end - start).  Specifically where the start operation it the
+            // same as the right side of the subtraction.
             var startOperation = invocation.Arguments[0].Value;
 
             if (CSharpSyntaxFactsService.Instance.AreEquivalent(startOperation.Syntax, subtraction.RightOperand.Syntax))
             {
                 context.ReportDiagnostic(CreateDiagnostic(
-                    ComputedRange, option, invocationSyntax, sliceLikeMethod,
+                    ComputedRange, option, invocationSyntax, targetMethod,
                     memberInfo, startOperation, subtraction.LeftOperand));
                 return;
             }
@@ -135,7 +126,7 @@ protected override void InitializeWorker(AnalysisContext context)
                 IsInstanceLengthCheck(memberInfo.LengthLikeProperty, invocation.Instance, subtraction.LeftOperand))
             {
                 context.ReportDiagnostic(CreateDiagnostic(
-                    ConstantRange, option, invocationSyntax, sliceLikeMethod,
+                    ConstantRange, option, invocationSyntax, targetMethod,
                     memberInfo, startOperation, subtraction.RightOperand));
                 return;
             }