From 6bb8d9f570bb7bbec28e24d51dc327e86370664f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 10 Sep 2015 12:35:33 -0700 Subject: [PATCH] PR feedback. Also handle fields with initializers. --- .../UseAutoPropertyAnalyzer.cs | 10 +++++++ .../UseAutoPropertyCodeFixProvider.cs | 19 +++++++++++-- .../UseAutoProperty/UseAutoPropertyTests.cs | 16 +++++++++++ .../UseAutoPropertyAnalyzer.vb | 9 +++++++ .../UseAutoPropertyCodeFixProvider.vb | 27 ++++++++++++++----- .../UseAutoProperty/UseAutoPropertyTests.vb | 7 +++++ .../AbstractUseAutoPropertyAnalyzer.cs | 8 ++++++ .../AbstractUseAutoPropertyCodeFixProvider.cs | 7 ++--- 8 files changed, 91 insertions(+), 12 deletions(-) diff --git a/src/EditorFeatures/CSharp/UseAutoProperty/UseAutoPropertyAnalyzer.cs b/src/EditorFeatures/CSharp/UseAutoProperty/UseAutoPropertyAnalyzer.cs index d1cb6dd2db1..1429098969f 100644 --- a/src/EditorFeatures/CSharp/UseAutoProperty/UseAutoPropertyAnalyzer.cs +++ b/src/EditorFeatures/CSharp/UseAutoProperty/UseAutoPropertyAnalyzer.cs @@ -19,11 +19,21 @@ protected override bool SupportsReadOnlyProperties(Compilation compilation) return ((CSharpCompilation)compilation).LanguageVersion >= LanguageVersion.CSharp6; } + protected override bool SupportsPropertyInitializer(Compilation compilation) + { + return ((CSharpCompilation)compilation).LanguageVersion >= LanguageVersion.CSharp6; + } + protected override void RegisterIneligibleFieldsAction(CompilationStartAnalysisContext context, ConcurrentBag ineligibleFields) { context.RegisterSyntaxNodeAction(snac => AnalyzeArgument(ineligibleFields, snac), SyntaxKind.Argument); } + protected override ExpressionSyntax GetFieldInitializer(VariableDeclaratorSyntax variable, CancellationToken cancellationToken) + { + return variable.Initializer?.Value; + } + private void AnalyzeArgument(ConcurrentBag ineligibleFields, SyntaxNodeAnalysisContext context) { // An argument will disqualify a field if that field is used in a ref/out position. diff --git a/src/EditorFeatures/CSharp/UseAutoProperty/UseAutoPropertyCodeFixProvider.cs b/src/EditorFeatures/CSharp/UseAutoProperty/UseAutoPropertyCodeFixProvider.cs index 54d5318a588..fd335f877a8 100644 --- a/src/EditorFeatures/CSharp/UseAutoProperty/UseAutoPropertyCodeFixProvider.cs +++ b/src/EditorFeatures/CSharp/UseAutoProperty/UseAutoPropertyCodeFixProvider.cs @@ -1,8 +1,10 @@ -using System.Collections.Generic; +using System; +using System.Collections.Generic; using System.Collections.Immutable; using System.Composition; using System.Linq; using System.Threading; +using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -24,7 +26,7 @@ protected override SyntaxNode GetNodeToRemove(VariableDeclaratorSyntax declarato return nodeToRemove; } - protected override SyntaxNode UpdateProperty( + protected override async Task UpdatePropertyAsync( Project project, Compilation compilation, IFieldSymbol fieldSymbol, IPropertySymbol propertySymbol, PropertyDeclarationSyntax propertyDeclaration, bool isWrittenOutsideOfConstructor, CancellationToken cancellationToken) { @@ -46,9 +48,22 @@ protected override SyntaxNode GetNodeToRemove(VariableDeclaratorSyntax declarato updatedProperty = updatedProperty.AddAccessorListAccessors(accessor); } + var fieldInitializer = await GetFieldInitializerAsync(fieldSymbol, cancellationToken).ConfigureAwait(false); + if (fieldInitializer != null) + { + updatedProperty = updatedProperty.WithInitializer(SyntaxFactory.EqualsValueClause(fieldInitializer)) + .WithSemicolonToken(SyntaxFactory.Token(SyntaxKind.SemicolonToken)); + } + return updatedProperty; } + private async Task GetFieldInitializerAsync(IFieldSymbol fieldSymbol, CancellationToken cancellationToken) + { + var variableDeclarator = (VariableDeclaratorSyntax)await fieldSymbol.DeclaringSyntaxReferences[0].GetSyntaxAsync(cancellationToken).ConfigureAwait(false); + return variableDeclarator.Initializer?.Value; + } + private bool NeedsSetter(Compilation compilation, PropertyDeclarationSyntax propertyDeclaration, bool isWrittenOutsideOfConstructor) { if (propertyDeclaration.AccessorList.Accessors.Any(SyntaxKind.SetAccessorDeclaration)) diff --git a/src/EditorFeatures/CSharpTest/Diagnostics/UseAutoProperty/UseAutoPropertyTests.cs b/src/EditorFeatures/CSharpTest/Diagnostics/UseAutoProperty/UseAutoPropertyTests.cs index 98a08c5951b..a6a64333ca3 100644 --- a/src/EditorFeatures/CSharpTest/Diagnostics/UseAutoProperty/UseAutoPropertyTests.cs +++ b/src/EditorFeatures/CSharpTest/Diagnostics/UseAutoProperty/UseAutoPropertyTests.cs @@ -41,6 +41,22 @@ public void TestCSharp5_2() CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp5)); } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)] + public void TestInitializer() + { + Test( +@"class Class { [|int i = 1|]; int P { get { return i; } } }", +@"class Class { int P { get; } = 1; }"); + } + + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)] + public void TestInitializer_CSharp5() + { + TestMissing( +@"class Class { [|int i = 1|]; int P { get { return i; } } }", + CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp5)); + } + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)] public void TestSingleGetter2() { diff --git a/src/EditorFeatures/VisualBasic/UseAutoProperty/UseAutoPropertyAnalyzer.vb b/src/EditorFeatures/VisualBasic/UseAutoProperty/UseAutoPropertyAnalyzer.vb index dbc267e540d..ee0445d6881 100644 --- a/src/EditorFeatures/VisualBasic/UseAutoProperty/UseAutoPropertyAnalyzer.vb +++ b/src/EditorFeatures/VisualBasic/UseAutoProperty/UseAutoPropertyAnalyzer.vb @@ -17,9 +17,18 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UseAutoProperty Return True End Function + Protected Overrides Function SupportsPropertyInitializer(compilation As Compilation) As Boolean + Return True + End Function + Protected Overrides Sub RegisterIneligibleFieldsAction(context As CompilationStartAnalysisContext, ineligibleFields As ConcurrentBag(Of IFieldSymbol)) End Sub + Protected Overrides Function GetFieldInitializer(variable As ModifiedIdentifierSyntax, cancellationToken As CancellationToken) As ExpressionSyntax + Dim declarator = TryCast(variable.Parent, VariableDeclaratorSyntax) + Return declarator?.Initializer?.Value + End Function + Private Function CheckExpressionSyntactically(expression As ExpressionSyntax) As Boolean If expression?.Kind() = SyntaxKind.SimpleMemberAccessExpression Then Dim memberAccessExpression = DirectCast(expression, MemberAccessExpressionSyntax) diff --git a/src/EditorFeatures/VisualBasic/UseAutoProperty/UseAutoPropertyCodeFixProvider.vb b/src/EditorFeatures/VisualBasic/UseAutoProperty/UseAutoPropertyCodeFixProvider.vb index 693174fff8b..67d1a30bf41 100644 --- a/src/EditorFeatures/VisualBasic/UseAutoProperty/UseAutoPropertyCodeFixProvider.vb +++ b/src/EditorFeatures/VisualBasic/UseAutoProperty/UseAutoPropertyCodeFixProvider.vb @@ -1,5 +1,6 @@ Imports System.Composition Imports System.Threading +Imports System.Threading.Tasks Imports Microsoft.CodeAnalysis.CodeFixes Imports Microsoft.CodeAnalysis.Editing Imports Microsoft.CodeAnalysis.UseAutoProperty @@ -15,19 +16,31 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UseAutoProperty Return Utilities.GetNodeToRemove(identifier) End Function - Protected Overrides Function UpdateProperty(project As Project, - compilation As Compilation, - fieldSymbol As IFieldSymbol, - propertySymbol As IPropertySymbol, - propertyDeclaration As PropertyBlockSyntax, - isWrittenToOutsideOfConstructor As Boolean, - cancellationToken As CancellationToken) As SyntaxNode + Protected Overrides Async Function UpdatePropertyAsync(project As Project, + compilation As Compilation, + fieldSymbol As IFieldSymbol, + propertySymbol As IPropertySymbol, + propertyDeclaration As PropertyBlockSyntax, + isWrittenToOutsideOfConstructor As Boolean, + cancellationToken As CancellationToken) As Task(Of SyntaxNode) Dim statement = propertyDeclaration.PropertyStatement If Not isWrittenToOutsideOfConstructor AndAlso Not propertyDeclaration.Accessors.Any(SyntaxKind.SetAccessorBlock) Then Dim generator = SyntaxGenerator.GetGenerator(project) statement = DirectCast(generator.WithModifiers(statement, DeclarationModifiers.ReadOnly), PropertyStatementSyntax) End If + + Dim initializer = Await GetFieldInitializer(fieldSymbol, cancellationToken).ConfigureAwait(False) + If initializer IsNot Nothing Then + statement = statement.WithInitializer(SyntaxFactory.EqualsValue(initializer)) + End If + Return statement End Function + + Private Async Function GetFieldInitializer(fieldSymbol As IFieldSymbol, cancellationToken As CancellationToken) As Task(Of ExpressionSyntax) + Dim identifier = TryCast(Await fieldSymbol.DeclaringSyntaxReferences(0).GetSyntaxAsync(cancellationToken).ConfigureAwait(False), ModifiedIdentifierSyntax) + Dim declarator = TryCast(identifier?.Parent, VariableDeclaratorSyntax) + Return declarator?.Initializer?.Value + End Function End Class End Namespace \ No newline at end of file diff --git a/src/EditorFeatures/VisualBasicTest/Diagnostics/UseAutoProperty/UseAutoPropertyTests.vb b/src/EditorFeatures/VisualBasicTest/Diagnostics/UseAutoProperty/UseAutoPropertyTests.vb index 37ad1500dec..099c2fc301d 100644 --- a/src/EditorFeatures/VisualBasicTest/Diagnostics/UseAutoProperty/UseAutoPropertyTests.vb +++ b/src/EditorFeatures/VisualBasicTest/Diagnostics/UseAutoProperty/UseAutoPropertyTests.vb @@ -38,6 +38,13 @@ NewLines("class Class1 \n [|dim i as integer|] \n property P as Integer \n get \ NewLines("class Class1 \n property P as Integer \n end class")) End Sub + + Public Sub TestInitializer() + Test( +NewLines("class Class1 \n dim i as Integer = 1 \n [|readonly property P as integer \n get \n return i \n end get \n end property|] \n end class"), +NewLines("class Class1 \n readonly property P as integer = 1 \n end class")) + End Sub + Public Sub TestDifferentValueName() Test( diff --git a/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyAnalyzer.cs b/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyAnalyzer.cs index fae31c74a07..cd5d75dbc61 100644 --- a/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyAnalyzer.cs +++ b/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyAnalyzer.cs @@ -29,6 +29,8 @@ internal abstract class AbstractUseAutoPropertyAnalyzer ineligibleFields); protected abstract bool SupportsReadOnlyProperties(Compilation compilation); + protected abstract bool SupportsPropertyInitializer(Compilation compilation); + protected abstract TExpression GetFieldInitializer(TVariableDeclarator variable, CancellationToken cancellationToken); protected abstract TExpression GetGetterExpression(IMethodSymbol getMethod, CancellationToken cancellationToken); protected abstract TExpression GetSetterExpression(IMethodSymbol setMethod, SemanticModel semanticModel, CancellationToken cancellationToken); protected abstract SyntaxNode GetNodeToFade(TFieldDeclaration fieldDeclaration, TVariableDeclarator variableDeclarator); @@ -160,6 +162,12 @@ private void AnalyzeProperty(ConcurrentBag analysisResults, Symb return; } + var initializer = GetFieldInitializer(variableDeclarator, cancellationToken); + if (initializer != null && !SupportsPropertyInitializer(symbolContext.Compilation)) + { + return; + } + var fieldDeclaration = variableDeclarator?.Parent?.Parent as TFieldDeclaration; if (fieldDeclaration == null) { diff --git a/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs b/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs index a9f1f41f16d..db1cb9e8218 100644 --- a/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs +++ b/src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyCodeFixProvider.cs @@ -29,7 +29,7 @@ internal abstract class AbstractUseAutoPropertyCodeFixProvider UpdatePropertyAsync( Project project, Compilation compilation, IFieldSymbol fieldSymbol, IPropertySymbol propertySymbol, TPropertyDeclaration propertyDeclaration, bool isWrittenOutsideConstructor, CancellationToken cancellationToken); @@ -74,8 +74,9 @@ private async Task ProcessResult(CodeFixContext context, Diagnostic di var fieldLocations = await Renamer.GetRenameLocationsAsync(solution, fieldSymbol, solution.Workspace.Options, cancellationToken).ConfigureAwait(false); // First, create the updated property we want to replace the old property with - var updatedProperty = UpdateProperty(project, compilation, fieldSymbol, propertySymbol, property, - IsWrittenToOutsideOfConstructorOrProperty(fieldSymbol, fieldLocations, property, cancellationToken), cancellationToken); + var isWrittenToOutsideOfConstructor = IsWrittenToOutsideOfConstructorOrProperty(fieldSymbol, fieldLocations, property, cancellationToken); + var updatedProperty = await UpdatePropertyAsync(project, compilation, fieldSymbol, propertySymbol, property, + isWrittenToOutsideOfConstructor, cancellationToken).ConfigureAwait(false); // Now, rename all usages of the field to point at the property. Except don't actually // rename the field itself. We want to be able to find it again post rename. -- GitLab