diff --git a/src/EditorFeatures/CSharpTest/UseIndexOrRangeOperator/UseIndexOperatorTests.cs b/src/EditorFeatures/CSharpTest/UseIndexOrRangeOperator/UseIndexOperatorTests.cs index 32c680d82c9aea1a3e069bfd2c6895847a95b1fe..d0f2e9d322fb160b921010ae4ae87f28663d4016 100644 --- a/src/EditorFeatures/CSharpTest/UseIndexOrRangeOperator/UseIndexOperatorTests.cs +++ b/src/EditorFeatures/CSharpTest/UseIndexOrRangeOperator/UseIndexOperatorTests.cs @@ -306,5 +306,27 @@ void Goo(S s) } }", parameters: s_testParameters); } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseIndexOperator)] + public async Task TestArray() + { + await TestAsync( +@" +class C +{ + void Goo(string[] s) + { + var v = s[[||]s.Length - 1]; + } +}", +@" +class C +{ + void Goo(string[] s) + { + var v = s[^1]; + } +}", parseOptions: s_parseOptions); + } } } diff --git a/src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.InfoCache.cs b/src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.InfoCache.cs index 0acd9ed95c0a11f71d09a8edc172ce35a0bb228f..88e24d2d5f2cf89a38d18849f6fa205fba8a4082 100644 --- a/src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.InfoCache.cs +++ b/src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.InfoCache.cs @@ -5,6 +5,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UseIndexOrRangeOperator { + using System; using static Helpers; internal partial class CSharpUseIndexOperatorDiagnosticAnalyzer @@ -19,13 +20,24 @@ private class InfoCache /// we're using has an indexer that takes an Index. /// private readonly INamedTypeSymbol _indexType; + + /// + /// Mapping from a method like 'MyType.Get(int)' to the Length/Count property for + /// 'MyType' as well as the optional 'MyType.Get(System.Index)' member if it exists. + /// private readonly ConcurrentDictionary _methodToMemberInfo; + /// + /// Mapping from an array symbol (like 'int[]') to the Length property for it. + /// + private readonly ConcurrentDictionary _arrayTypeToMemberInfo; + public InfoCache(Compilation compilation) { _indexType = compilation.GetTypeByMetadataName("System.Index"); _methodToMemberInfo = new ConcurrentDictionary(); + _arrayTypeToMemberInfo = new ConcurrentDictionary(); // Always allow using System.Index indexers with System.String. The compiler has // hard-coded knowledge on how to use this type, even if there is no this[Index] @@ -38,18 +50,37 @@ public InfoCache(Compilation compilation) _methodToMemberInfo[indexer.GetMethod] = ComputeMemberInfo(indexer.GetMethod, requireIndexMember: false); } - public bool TryGetMemberInfo(IMethodSymbol methodSymbol, out MemberInfo memberInfo) + public bool TryGetMemberInfo( + IMethodSymbol methodSymbolOpt, ITypeSymbol typeSymbol, out MemberInfo memberInfo) { - if (!IsIntIndexingMethod(methodSymbol)) + memberInfo = default; + + if (methodSymbolOpt != null) + { + if (IsIntIndexingMethod(methodSymbolOpt)) + { + memberInfo = _methodToMemberInfo.GetOrAdd(methodSymbolOpt, m => ComputeMemberInfo(m, requireIndexMember: true)); + } + } + else if (typeSymbol != null) { - memberInfo = default; - return false; + memberInfo = _arrayTypeToMemberInfo.GetOrAdd(typeSymbol, t => ComputeMemberInfo(t)); } - memberInfo = _methodToMemberInfo.GetOrAdd(methodSymbol, m => ComputeMemberInfo(m, requireIndexMember: true)); return memberInfo.LengthLikeProperty != null; } + private MemberInfo ComputeMemberInfo(ITypeSymbol type) + { + if (type is IArrayTypeSymbol) + { + var lengthProperty = GetNoArgInt32Property(type.BaseType, nameof(Array.Length)); + return new MemberInfo(lengthProperty, overloadedMethodOpt: null); + } + + return default; + } + private MemberInfo ComputeMemberInfo(IMethodSymbol method, bool requireIndexMember) { Debug.Assert(IsIntIndexingMethod(method)); diff --git a/src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs index 9d00259fd5854437d94b8c30db3172a3a5a99967..add6b75124c1de9b9e51a1d68acd3d069579ec49 100644 --- a/src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/UseIndexOrRangeOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs @@ -51,10 +51,20 @@ protected override void InitializeWorker(AnalysisContext context) // compilation. Cache information we compute in this object so we don't have to // continually recompute it. var infoCache = new InfoCache(startContext.Compilation); + + // Register to hear property references, so we can hear about calls to indexers + // like: s[s.Length - n] context.RegisterOperationAction( c => AnalyzePropertyReference(c, infoCache), OperationKind.PropertyReference); + // Array indexing is represented with a different operation kind. Register + // specifically for that. + context.RegisterOperationAction( + c => AnalyzeArrayElementReference(c, infoCache), + OperationKind.ArrayElementReference); + + // Register to hear about methods for: s.Get(s.Length - n) context.RegisterOperationAction( c => AnalyzeInvocation(c, infoCache), OperationKind.Invocation); @@ -66,6 +76,11 @@ private void AnalyzeInvocation(OperationAnalysisContext context, InfoCache infoC var cancellationToken = context.CancellationToken; var invocationOperation = (IInvocationOperation)context.Operation; + if (invocationOperation.Arguments.Length != 1) + { + return; + } + // Make sure we're actually on an invocation something like `s.Get(...)`. var invocationSyntax = invocationOperation.Syntax as InvocationExpressionSyntax; if (invocationSyntax is null) @@ -73,13 +88,13 @@ private void AnalyzeInvocation(OperationAnalysisContext context, InfoCache infoC return; } - var instance = invocationOperation.Instance; - var targetMethod = invocationOperation.TargetMethod; - var arguments = invocationOperation.Arguments; - AnalyzeInvokedMember( - context, infoCache, invocationOperation, - instance, targetMethod, arguments, cancellationToken); + context, infoCache, + invocationSyntax, + invocationOperation.Instance, + invocationOperation.TargetMethod, + invocationOperation.Arguments[0].Value, + cancellationToken); } private void AnalyzePropertyReference( @@ -87,10 +102,14 @@ private void AnalyzeInvocation(OperationAnalysisContext context, InfoCache infoC { var cancellationToken = context.CancellationToken; var propertyReference = (IPropertyReferenceOperation)context.Operation; - var property = propertyReference.Property; // Only analyze indexer calls. - if (!property.IsIndexer) + if (!propertyReference.Property.IsIndexer) + { + return; + } + + if (propertyReference.Arguments.Length != 1) { return; } @@ -102,22 +121,50 @@ private void AnalyzeInvocation(OperationAnalysisContext context, InfoCache infoC return; } - var instance = propertyReference.Instance; - var targetMethod = property.GetMethod; - var arguments = propertyReference.Arguments; + AnalyzeInvokedMember( + context, infoCache, + elementAccess, + propertyReference.Instance, + propertyReference.Property.GetMethod, + propertyReference.Arguments[0].Value, + cancellationToken); + } + + private void AnalyzeArrayElementReference( + OperationAnalysisContext context, InfoCache infoCache) + { + var cancellationToken = context.CancellationToken; + var arrayElementReference = (IArrayElementReferenceOperation)context.Operation; + + // Has to be a single-dimensional element access. + if (arrayElementReference.Indices.Length != 1) + { + return; + } + + // Make sure we're actually on something like `s[...]`. + var elementAccess = arrayElementReference.Syntax as ElementAccessExpressionSyntax; + if (elementAccess is null) + { + return; + } AnalyzeInvokedMember( - context, infoCache, propertyReference, - instance, targetMethod, arguments, cancellationToken); + context, infoCache, + elementAccess, + arrayElementReference.ArrayReference, + targetMethodOpt: null, + arrayElementReference.Indices[0], + cancellationToken); } private void AnalyzeInvokedMember( - OperationAnalysisContext context, InfoCache infoCache, IOperation invocation, - IOperation instance, IMethodSymbol targetMethod, ImmutableArray arguments, + OperationAnalysisContext context, InfoCache infoCache, SyntaxNode syntax, + IOperation instance, IMethodSymbol targetMethodOpt, IOperation argumentValue, CancellationToken cancellationToken) { // Only supported on C# 8 and above. - var syntaxTree = invocation.Syntax.SyntaxTree; + var syntaxTree = syntax.SyntaxTree; var parseOptions = (CSharpParseOptions)syntaxTree.Options; if (parseOptions.LanguageVersion < LanguageVersion.CSharp8) { @@ -142,8 +189,7 @@ private void AnalyzeInvocation(OperationAnalysisContext context, InfoCache infoC // 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 || - !IsSubtraction(arguments[0].Value, out var subtraction)) + !IsSubtraction(argumentValue, out var subtraction)) { return; } @@ -154,7 +200,8 @@ private void AnalyzeInvocation(OperationAnalysisContext context, InfoCache infoC // // 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) || + + if (!infoCache.TryGetMemberInfo(targetMethodOpt, instance.Type, out var memberInfo) || !IsInstanceLengthCheck(memberInfo.LengthLikeProperty, instance, subtraction.LeftOperand)) { return; diff --git a/src/Features/CSharp/Portable/UseIndexOrRangeOperator/Helpers.cs b/src/Features/CSharp/Portable/UseIndexOrRangeOperator/Helpers.cs index b34bad38f71191e905beca311690a4187df12ff5..da58d48bbe00580aed5884fd86ea125d1fb8ed37 100644 --- a/src/Features/CSharp/Portable/UseIndexOrRangeOperator/Helpers.cs +++ b/src/Features/CSharp/Portable/UseIndexOrRangeOperator/Helpers.cs @@ -14,7 +14,7 @@ public static IPropertySymbol GetLengthOrCountProperty(INamedTypeSymbol namedTyp => GetNoArgInt32Property(namedType, nameof(string.Length)) ?? GetNoArgInt32Property(namedType, nameof(ICollection.Count)); - private static IPropertySymbol GetNoArgInt32Property(INamedTypeSymbol type, string name) + public static IPropertySymbol GetNoArgInt32Property(ITypeSymbol type, string name) => type.GetMembers(name) .OfType() .Where(p => IsPublicInstance(p) && diff --git a/src/VisualStudio/CSharp/Impl/Options/Formatting/StyleViewModel.cs b/src/VisualStudio/CSharp/Impl/Options/Formatting/StyleViewModel.cs index d3c34bb445357c51f28678c033f99dfdb3bfc149..f50da4b3c39af0f0cbae8515cf899873fe6b152b 100644 --- a/src/VisualStudio/CSharp/Impl/Options/Formatting/StyleViewModel.cs +++ b/src/VisualStudio/CSharp/Impl/Options/Formatting/StyleViewModel.cs @@ -716,7 +716,6 @@ void M2(string value) private static readonly string s_preferCompoundAssignments = $@" using System; - class Customer {{ void M1(int value) @@ -734,6 +733,51 @@ void M2(int value) //] }} }} +"; + + private static readonly string s_preferIndexOperator = $@" +using System; + +class Customer +{{ + void M1(string value) + {{ +//[ + // {ServicesVSResources.Prefer_colon} + var ch = value[^1]; +//] + }} + void M2(string value) + {{ +//[ + // {ServicesVSResources.Over_colon} + var ch = value[value.Length - 1]; +//] + }} +}} +"; + + private static readonly string s_preferRangeOperator = $@" +using System; + +class Customer +{{ + void M1(string value) + {{ +//[ + // {ServicesVSResources.Prefer_colon} + var sub = value[1..^1]; +//] + }} + void M2(string value) + {{ +//[ + // {ServicesVSResources.Over_colon} + var sub = value.Substring(1, value.Length - 2); +>>>>>>> UI options. Also support arrays. +//] + }} +}} "; private static readonly string s_preferIsNullOverReferenceEquals = $@" @@ -1193,6 +1237,9 @@ internal StyleViewModel(OptionSet optionSet, IServiceProvider serviceProvider) : CodeStyleItems.Add(new BooleanCodeStyleOptionViewModel(CSharpCodeStyleOptions.PreferLocalOverAnonymousFunction, ServicesVSResources.Prefer_local_function_over_anonymous_function, s_preferLocalFunctionOverAnonymousFunction, s_preferLocalFunctionOverAnonymousFunction, this, optionSet, expressionPreferencesGroupTitle)); CodeStyleItems.Add(new BooleanCodeStyleOptionViewModel(CodeStyleOptions.PreferCompoundAssignment, ServicesVSResources.Prefer_compound_assignments, s_preferCompoundAssignments, s_preferCompoundAssignments, this, optionSet, expressionPreferencesGroupTitle)); + CodeStyleItems.Add(new BooleanCodeStyleOptionViewModel(CSharpCodeStyleOptions.PreferIndexOperator, ServicesVSResources.Prefer_index_operator, s_preferIndexOperator, s_preferIndexOperator, this, optionSet, expressionPreferencesGroupTitle)); + CodeStyleItems.Add(new BooleanCodeStyleOptionViewModel(CSharpCodeStyleOptions.PreferRangeOperator, ServicesVSResources.Prefer_range_operator, s_preferRangeOperator, s_preferRangeOperator, this, optionSet, expressionPreferencesGroupTitle)); + AddExpressionBodyOptions(optionSet, expressionPreferencesGroupTitle); // Variable preferences diff --git a/src/VisualStudio/Core/Def/ServicesVSResources.Designer.cs b/src/VisualStudio/Core/Def/ServicesVSResources.Designer.cs index 27c626583a6dd5a17ce446c73afd665aca59d0bb..7a0b438daaed9c822ec334557a024589335ec74b 100644 --- a/src/VisualStudio/Core/Def/ServicesVSResources.Designer.cs +++ b/src/VisualStudio/Core/Def/ServicesVSResources.Designer.cs @@ -1857,6 +1857,15 @@ internal class ServicesVSResources { } } + /// + /// Looks up a localized string similar to Prefer index operator. + /// + internal static string Prefer_index_operator { + get { + return ResourceManager.GetString("Prefer_index_operator", resourceCulture); + } + } + /// /// Looks up a localized string similar to Prefer inferred anonymous type member names. /// @@ -1920,6 +1929,15 @@ internal class ServicesVSResources { } } + /// + /// Looks up a localized string similar to Prefer range operator. + /// + internal static string Prefer_range_operator { + get { + return ResourceManager.GetString("Prefer_range_operator", resourceCulture); + } + } + /// /// Looks up a localized string similar to Prefer readonly. /// diff --git a/src/VisualStudio/Core/Def/ServicesVSResources.resx b/src/VisualStudio/Core/Def/ServicesVSResources.resx index 6228494e86fc7a5638cdfa4466ff606db1407137..72f563ea4a0939804e3a5af63d8d2f799ef94d16 100644 --- a/src/VisualStudio/Core/Def/ServicesVSResources.resx +++ b/src/VisualStudio/Core/Def/ServicesVSResources.resx @@ -1078,4 +1078,10 @@ I agree to all of the foregoing: Kind + + Prefer index operator + + + Prefer range operator + \ No newline at end of file