Merge pull request #40558 from YairHalberstadt/dont-suggest-named-args-for-implicit-indexers

Don't suggest using named arguments for implicit Index and Range indexers

src/EditorFeatures/CSharpTest/UseNamedArguments/UseNamedArgumentsTests.cs

@@ -512,5 +512,51 @@ class C : System.Attribute
     public C(int @default, int @params) {}
 }");
         }
+
+        [Fact, WorkItem(39852, "https://github.com/dotnet/roslyn/issues/39852")]
+        public async Task TestMissingForImplicitRangeIndexer()
+        {
+            await TestMissingInRegularAndScriptAsync(
+                @"class C { string M(string arg1) => arg1[[||]1..^1]; }" + TestSources.Range + TestSources.Index);
+        }
+
+        [Fact, WorkItem(39852, "https://github.com/dotnet/roslyn/issues/39852")]
+        public async Task TestMissingForImplicitIndexIndexer()
+        {
+            await TestMissingInRegularAndScriptAsync(
+                @"class C { string M(string arg1) => arg1[[||]^1]; }" + TestSources.Index);
+        }
+
+        [Fact, WorkItem(39852, "https://github.com/dotnet/roslyn/issues/39852")]
+        public async Task TestForRealRangeIndexer()
+        {
+            await TestInRegularAndScriptAsync(
+                @"using System; 
+class C { 
+    int this[Range range] => default; 
+    int M(C arg1) => arg1[[||]1..^1]; 
+}" + TestSources.Range + TestSources.Index,
+                @"using System; 
+class C { 
+    int this[Range range] => default; 
+    int M(C arg1) => arg1[range: 1..^1]; 
+}" + TestSources.Range + TestSources.Index);
+        }
+
+        [Fact, WorkItem(39852, "https://github.com/dotnet/roslyn/issues/39852")]
+        public async Task TestForRealIndexIndexer()
+        {
+            await TestInRegularAndScriptAsync(
+                @"using System; 
+class C { 
+    int this[Index index] => default; 
+    int M(C arg1) => arg1[[||]^1]; 
+}" + TestSources.Index,
+                @"using System; 
+class C { 
+    int this[Index index] => default; 
+    int M(C arg1) => arg1[index: ^1]; 
+}" + TestSources.Index);
+        }
     }
 }

src/Features/CSharp/Portable/UseNamedArguments/CSharpUseNamedArgumentsCodeRefactoringProvider.cs

@@ -8,6 +8,7 @@
 using System.Collections.Immutable;
 using System.Composition;
 using System.Linq;
+using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CodeRefactorings;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
@@ -23,6 +24,8 @@ private abstract class BaseAnalyzer<TSyntax, TSyntaxList> : Analyzer<TSyntax, TS
             where TSyntax : SyntaxNode
             where TSyntaxList : SyntaxNode
         {
+            protected abstract ExpressionSyntax GetArgumentExpression(TSyntax argumentSyntax);
+
             protected sealed override SyntaxNode? GetReceiver(SyntaxNode argument)
                 => argument.Parent?.Parent;
 
@@ -31,6 +34,27 @@ protected sealed override bool IsLegalToAddNamedArguments(ImmutableArray<IParame
 
             protected override bool SupportsNonTrailingNamedArguments(ParseOptions options)
                 => ((CSharpParseOptions)options).LanguageVersion >= LanguageVersion.CSharp7_2;
+
+            protected override bool IsImplicitIndexOrRangeIndexer(ImmutableArray<IParameterSymbol> parameters, TSyntax argument, SemanticModel semanticModel)
+            {
+                // There is no direct way to tell if an implicit range or index indexer was used.
+                // The heuristic we use here is to check if the parameter doesn't fit the method it's being used with. 
+                // The easiest way to check that is to see if the argType only has at most an explicit conversion 
+                // to the indexers parameter types.
+
+                var argType = semanticModel.GetTypeInfo(GetArgumentExpression(argument)).Type;
+                if (argType?.ContainingNamespace is { Name: nameof(System), ContainingNamespace: { IsGlobalNamespace: true } } &&
+                    (argType.Name == "Range" || argType.Name == "Index"))
+                {
+                    var conversion = semanticModel.Compilation.ClassifyConversion(argType, parameters[0].Type);
+                    if (!conversion.Exists || conversion.IsExplicit)
+                    {
+                        return true;
+                    }
+                }
+
+                return false;
+            }
         }
 
         private class ArgumentAnalyzer :
@@ -48,6 +72,9 @@ protected override SeparatedSyntaxList<ArgumentSyntax> GetArguments(BaseArgument
 
             protected override ArgumentSyntax WithName(ArgumentSyntax argument, string name)
                 => argument.WithNameColon(SyntaxFactory.NameColon(name.ToIdentifierName()));
+
+            protected override ExpressionSyntax GetArgumentExpression(ArgumentSyntax argumentSyntax)
+                => argumentSyntax.Expression;
         }
 
         private class AttributeArgumentAnalyzer :
@@ -65,6 +92,9 @@ protected override SeparatedSyntaxList<AttributeArgumentSyntax> GetArguments(Att
 
             protected override AttributeArgumentSyntax WithName(AttributeArgumentSyntax argument, string name)
                 => argument.WithNameColon(SyntaxFactory.NameColon(name.ToIdentifierName()));
+
+            protected override ExpressionSyntax GetArgumentExpression(AttributeArgumentSyntax argumentSyntax)
+                => argumentSyntax.Expression;
         }
 
         [ImportingConstructor]

src/Features/Core/Portable/UseNamedArguments/AbstractUseNamedArgumentsCodeRefactoringProvider.cs

@@ -88,6 +88,11 @@ protected abstract class Analyzer<TBaseArgumentSyntax, TSimpleArgumentSyntax, TA
                     return;
                 }
 
+                if (IsImplicitIndexOrRangeIndexer(parameters, argument, semanticModel))
+                {
+                    return;
+                }
+
                 for (var i = argumentIndex; i < argumentCount; i++)
                 {
                     if (!(arguments[i] is TSimpleArgumentSyntax))
@@ -171,6 +176,7 @@ bool ShouldAddName(TBaseArgumentSyntax argument, int currentIndex)
             protected abstract SeparatedSyntaxList<TBaseArgumentSyntax> GetArguments(TArgumentListSyntax argumentList);
             protected abstract SyntaxNode? GetReceiver(SyntaxNode argument);
             protected abstract bool SupportsNonTrailingNamedArguments(ParseOptions options);
+            protected abstract bool IsImplicitIndexOrRangeIndexer(ImmutableArray<IParameterSymbol> parameters, TBaseArgumentSyntax argument, SemanticModel semanticModel);
         }
 
         private readonly IAnalyzer _argumentAnalyzer;

src/Features/VisualBasic/Portable/UseNamedArguments/VisualBasicUseNamedArgumentsCodeRefactoringProvider.vb

@@ -4,9 +4,9 @@
 
 Imports System.Collections.Immutable
 Imports System.Composition
+Imports Microsoft.CodeAnalysis
 Imports Microsoft.CodeAnalysis.CodeRefactorings
 Imports Microsoft.CodeAnalysis.UseNamedArguments
-Imports Microsoft.CodeAnalysis.VisualBasic.Extensions
 Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 
 Namespace Microsoft.CodeAnalysis.VisualBasic.UseNamedArguments
@@ -49,6 +49,10 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseNamedArguments
             Protected Overrides Function SupportsNonTrailingNamedArguments(options As ParseOptions) As Boolean
                 Return DirectCast(options, VisualBasicParseOptions).LanguageVersion >= LanguageVersion.VisualBasic15_5
             End Function
+
+            Protected Overrides Function IsImplicitIndexOrRangeIndexer(parameters As ImmutableArray(Of IParameterSymbol), argument As ArgumentSyntax, semanticModel As SemanticModel) As Boolean
+                Return False
+            End Function
         End Class
 
         <ImportingConstructor>