From 211392234bc2ece897ce1065fe4d44485a076e1a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Sat, 20 Oct 2018 21:56:27 -0700 Subject: [PATCH] Add features to offer using index and range operators. --- .../CSharpUseIndexOperatorCodeFixProvider.cs | 66 ++++++++ ...SharpUseIndexOperatorDiagnosticAnalyzer.cs | 147 ++++++++++++++++++ .../Diagnostics/Analyzers/IDEDiagnosticIds.cs | 2 + .../Portable/FeaturesResources.Designer.cs | 18 +++ .../Core/Portable/FeaturesResources.resx | 6 + .../CodeStyle/CSharpCodeStyleOptions.cs | 8 + 6 files changed, 247 insertions(+) create mode 100644 src/Features/CSharp/Portable/UseIndexOperator/CSharpUseIndexOperatorCodeFixProvider.cs create mode 100644 src/Features/CSharp/Portable/UseIndexOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs diff --git a/src/Features/CSharp/Portable/UseIndexOperator/CSharpUseIndexOperatorCodeFixProvider.cs b/src/Features/CSharp/Portable/UseIndexOperator/CSharpUseIndexOperatorCodeFixProvider.cs new file mode 100644 index 00000000000..114a6ebcb89 --- /dev/null +++ b/src/Features/CSharp/Portable/UseIndexOperator/CSharpUseIndexOperatorCodeFixProvider.cs @@ -0,0 +1,66 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using System.Composition; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.CodeActions; +using Microsoft.CodeAnalysis.CodeFixes; +using Microsoft.CodeAnalysis.CSharp.Extensions; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Shared.Extensions; + +namespace Microsoft.CodeAnalysis.CSharp.UseIndexOperator +{ + [ExportCodeFixProvider(LanguageNames.CSharp), Shared] + internal class CSharpUseIndexOperatorCodeFixProvider : SyntaxEditorBasedCodeFixProvider + { + public override ImmutableArray FixableDiagnosticIds { get; } = + ImmutableArray.Create(IDEDiagnosticIds.UseIndexOperatorDiagnosticId); + + public override Task RegisterCodeFixesAsync(CodeFixContext context) + { + context.RegisterCodeFix(new MyCodeAction( + c => FixAsync(context.Document, context.Diagnostics[0], c)), + context.Diagnostics); + + return Task.CompletedTask; + } + + protected override Task FixAllAsync( + Document document, ImmutableArray diagnostics, + SyntaxEditor editor, CancellationToken cancellationToken) + { + foreach (var diagnostic in diagnostics) + { + FixOne(diagnostic, editor, cancellationToken); + } + + return Task.CompletedTask; + } + + private void FixOne( + Diagnostic diagnostic, SyntaxEditor editor, CancellationToken cancellationToken) + { + var elementAccess = (ElementAccessExpressionSyntax)diagnostic.Location.FindNode(getInnermostNodeForTie: true, cancellationToken); + var value = (ExpressionSyntax)diagnostic.AdditionalLocations[0].FindNode(getInnermostNodeForTie: true, cancellationToken); + + editor.ReplaceNode( + elementAccess.ArgumentList.Arguments[0].Expression, + SyntaxFactory.PrefixUnaryExpression( + SyntaxKind.IndexExpression, + value.Parenthesize())); + } + + private class MyCodeAction : CodeAction.DocumentChangeAction + { + public MyCodeAction(Func> createChangedDocument) + : base(FeaturesResources.Use_index_operator, createChangedDocument, FeaturesResources.Use_index_operator) + { + } + } + } +} diff --git a/src/Features/CSharp/Portable/UseIndexOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/UseIndexOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs new file mode 100644 index 00000000000..25e92cb33cf --- /dev/null +++ b/src/Features/CSharp/Portable/UseIndexOperator/CSharpUseIndexOperatorDiagnosticAnalyzer.cs @@ -0,0 +1,147 @@ +// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using System.Collections.Immutable; +using System.Composition; +using System.Linq; +using Microsoft.CodeAnalysis.CodeStyle; +using Microsoft.CodeAnalysis.CSharp.CodeStyle; +using Microsoft.CodeAnalysis.CSharp.Syntax; +using Microsoft.CodeAnalysis.Diagnostics; +using Microsoft.CodeAnalysis.Operations; + +namespace Microsoft.CodeAnalysis.CSharp.UseIndexOperator +{ + [DiagnosticAnalyzer(LanguageNames.CSharp), Shared] + internal class CSharpUseIndexOperatorDiagnosticAnalyzer : AbstractCodeStyleDiagnosticAnalyzer + { + public CSharpUseIndexOperatorDiagnosticAnalyzer() + : base(IDEDiagnosticIds.UseIndexOperatorDiagnosticId, + new LocalizableResourceString(nameof(FeaturesResources.Use_index_operator), FeaturesResources.ResourceManager, typeof(FeaturesResources)), + new LocalizableResourceString(nameof(FeaturesResources.Indexing_can_be_simplified), FeaturesResources.ResourceManager, typeof(FeaturesResources))) + { + } + + protected override void InitializeWorker(AnalysisContext context) + { + context.RegisterCompilationStartAction(compilationContext => + { + var compilation = compilationContext.Compilation; + var stringType = compilation.GetSpecialType(SpecialType.System_String); + + var stringIndexer = + stringType.GetMembers() + .OfType() + .Where(p => IsStringIndexer(p)) + .FirstOrDefault(); + + var stringLength = + stringType.GetMembers() + .OfType() + .Where(p => p.Name == nameof(string.Length)) + .FirstOrDefault(); + + + if (stringIndexer != null && stringLength != null) + { + compilationContext.RegisterOperationAction( + c => AnalyzePropertyReference(c, stringIndexer, stringLength), + OperationKind.PropertyReference); + } + }); + } + + private void AnalyzePropertyReference( + OperationAnalysisContext context, + IPropertySymbol stringIndexer, IPropertySymbol stringLength) + { + var cancellationToken = context.CancellationToken; + var propertyReference = (IPropertyReferenceOperation)context.Operation; + + if (!stringIndexer.Equals(propertyReference.Property)) + { + return; + } + + var syntax = propertyReference.Syntax; + if (syntax == null) + { + return; + } + + var syntaxTree = syntax.SyntaxTree; + var parseOptions = (CSharpParseOptions)syntaxTree.Options; + //if (parseOptions.LanguageVersion < LanguageVersion.CSharp8) + //{ + // return; + //} + + var optionSet = context.Options.GetDocumentOptionSetAsync(syntaxTree, cancellationToken).GetAwaiter().GetResult(); + if (optionSet == null) + { + return; + } + + var option = optionSet.GetOption(CSharpCodeStyleOptions.PreferIndexOperator); + if (!option.Value) + { + return; + } + + // look for `s[s.Length - index.Value]` and convert to `s[^index]` + + // Needs to have the one arg for `[s.Length - index.Value]` + if (propertyReference.Instance is null || + propertyReference.Arguments.Length != 1) + { + return; + } + + // Arg needs to be a subtraction for: `s.Length - index.Value` + var arg = propertyReference.Arguments[0]; + if (!(arg.Value is IBinaryOperation binaryOperation) || + binaryOperation.OperatorKind != BinaryOperatorKind.Subtract) + { + return; + } + + // Left side of the subtraction needs to be `s.Length`. First make + // sure we're referencing String.Length. + if (!(binaryOperation.LeftOperand is IPropertyReferenceOperation leftPropertyRef) || + leftPropertyRef.Instance is null || + !stringLength.Equals(leftPropertyRef.Property)) + { + return; + } + + // make sure that we're indexing and getting the length off hte same value: + // `s[s.Length` + var indexInstanceSyntax = propertyReference.Instance.Syntax; + var lengthInstanceSyntax = leftPropertyRef.Instance.Syntax; + + var syntaxFacts = CSharpSyntaxFactsService.Instance; + if (!syntaxFacts.AreEquivalent(indexInstanceSyntax, lengthInstanceSyntax)) + { + return; + } + + if (!(propertyReference.Syntax is ElementAccessExpressionSyntax elementAccess)) + { + return; + } + + var additionalLocations = ImmutableArray.Create( + binaryOperation.RightOperand.Syntax.GetLocation()); + + context.ReportDiagnostic( + DiagnosticHelper.Create( + Descriptor, + elementAccess.GetLocation(), + option.Notification.Severity, + additionalLocations, + ImmutableDictionary.Empty)); + } + + private static bool IsStringIndexer(IPropertySymbol property) + => property.IsIndexer && property.Parameters.Length == 1 && property.Parameters[0].Type.SpecialType == SpecialType.System_Int32; + } +} diff --git a/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs b/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs index cc7412993d2..30f67ddc8e7 100644 --- a/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs +++ b/src/Features/Core/Portable/Diagnostics/Analyzers/IDEDiagnosticIds.cs @@ -88,6 +88,8 @@ internal static class IDEDiagnosticIds public const string FormattingDiagnosticId = "IDE0055"; + public const string UseIndexOperatorDiagnosticId = "IDE0055"; + // Analyzer error Ids public const string AnalyzerChangedId = "IDE1001"; public const string AnalyzerDependencyConflictId = "IDE1002"; diff --git a/src/Features/Core/Portable/FeaturesResources.Designer.cs b/src/Features/Core/Portable/FeaturesResources.Designer.cs index 47b70387491..8a5d1124fd9 100644 --- a/src/Features/Core/Portable/FeaturesResources.Designer.cs +++ b/src/Features/Core/Portable/FeaturesResources.Designer.cs @@ -1855,6 +1855,15 @@ internal class FeaturesResources { } } + /// + /// Looks up a localized string similar to Indexing can be simplified. + /// + internal static string Indexing_can_be_simplified { + get { + return ResourceManager.GetString("Indexing_can_be_simplified", resourceCulture); + } + } + /// /// Looks up a localized string similar to Initialize field '{0}'. /// @@ -3933,6 +3942,15 @@ internal class FeaturesResources { } } + /// + /// Looks up a localized string similar to Use index operator. + /// + internal static string Use_index_operator { + get { + return ResourceManager.GetString("Use_index_operator", resourceCulture); + } + } + /// /// Looks up a localized string similar to Use inferred member name. /// diff --git a/src/Features/Core/Portable/FeaturesResources.resx b/src/Features/Core/Portable/FeaturesResources.resx index a60f945a486..540ffd81325 100644 --- a/src/Features/Core/Portable/FeaturesResources.resx +++ b/src/Features/Core/Portable/FeaturesResources.resx @@ -1469,4 +1469,10 @@ This version used in: {2} Fix formatting + + Indexing can be simplified + + + Use index operator + \ No newline at end of file diff --git a/src/Workspaces/CSharp/Portable/CodeStyle/CSharpCodeStyleOptions.cs b/src/Workspaces/CSharp/Portable/CodeStyle/CSharpCodeStyleOptions.cs index 7de7eba026d..3e262767af1 100644 --- a/src/Workspaces/CSharp/Portable/CodeStyle/CSharpCodeStyleOptions.cs +++ b/src/Workspaces/CSharp/Portable/CodeStyle/CSharpCodeStyleOptions.cs @@ -61,6 +61,13 @@ private static Option CreateOption(OptionGroup group, string name, T defau EditorConfigStorageLocation.ForBoolCodeStyleOption("csharp_style_pattern_matching_over_is_with_cast_check"), new RoamingProfileStorageLocation($"TextEditor.CSharp.Specific.{nameof(PreferPatternMatchingOverIsWithCastCheck)}")}); + public static readonly Option> PreferIndexOperator = CreateOption( + CSharpCodeStyleOptionGroups.ExpressionLevelPreferences, nameof(PreferIndexOperator), + defaultValue: CodeStyleOptions.TrueWithSuggestionEnforcement, + storageLocations: new OptionStorageLocation[] { + EditorConfigStorageLocation.ForBoolCodeStyleOption("csharp_style_prefer_index_operator"), + new RoamingProfileStorageLocation("TextEditor.CSharp.Specific.PreferIndexOperator")}); + public static readonly CodeStyleOption NeverWithSilentEnforcement = new CodeStyleOption(ExpressionBodyPreference.Never, NotificationOption.Silent); @@ -205,6 +212,7 @@ public static IEnumerable>> GetCodeStyleOptions() yield return PreferBraces; yield return PreferSimpleDefaultExpression; yield return PreferLocalOverAnonymousFunction; + yield return PreferIndexOperator; } public static IEnumerable>> GetExpressionBodyOptions() -- GitLab