From c1a3e813a2dbb593fccea6eb8d09946152beaad8 Mon Sep 17 00:00:00 2001 From: CyrusNajmabadi Date: Thu, 17 Nov 2016 13:19:02 -0800 Subject: [PATCH] Extract analyzer into its own helper type. --- ...harpUseObjectInitializerCodeFixProvider.cs | 46 ++- ...pUseObjectInitializerDiagnosticAnalyzer.cs | 47 --- src/Features/Core/Portable/Features.csproj | 2 +- ...ractUseObjectInitializerCodeFixProvider.cs | 72 ++++- ...tInitializerDiagnosticAnalyzer.Analyzer.cs | 282 ----------------- ...tUseObjectInitializerDiagnosticAnalyzer.cs | 87 +----- .../Portable/UseObjectInitializer/Analyzer.cs | 291 ++++++++++++++++++ ...asicUseObjectInitializerCodeFixProvider.vb | 45 ++- ...cUseObjectInitializerDiagnosticAnalyzer.vb | 43 --- 9 files changed, 449 insertions(+), 466 deletions(-) delete mode 100644 src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.Analyzer.cs create mode 100644 src/Features/Core/Portable/UseObjectInitializer/Analyzer.cs diff --git a/src/Features/CSharp/Portable/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs b/src/Features/CSharp/Portable/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs index a6798b5bbf2..5fc0c7acc2e 100644 --- a/src/Features/CSharp/Portable/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs +++ b/src/Features/CSharp/Portable/UseObjectInitializer/CSharpUseObjectInitializerCodeFixProvider.cs @@ -1,5 +1,7 @@ // 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.Generic; +using System.Collections.Immutable; using System.Composition; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; @@ -18,9 +20,49 @@ internal class CSharpUseObjectInitializerCodeFixProvider : ExpressionStatementSyntax, VariableDeclaratorSyntax> { - public CSharpUseObjectInitializerCodeFixProvider() - : base(new CSharpUseObjectInitializerDiagnosticAnalyzer()) + protected override ObjectCreationExpressionSyntax GetNewObjectCreation( + ObjectCreationExpressionSyntax objectCreation, + ImmutableArray> matches) { + var openBrace = SyntaxFactory.Token(SyntaxKind.OpenBraceToken) + .WithTrailingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed); + var initializer = SyntaxFactory.InitializerExpression( + SyntaxKind.ObjectInitializerExpression, + CreateExpressions(matches)).WithOpenBraceToken(openBrace); + + return objectCreation.WithInitializer(initializer); + } + + private SeparatedSyntaxList CreateExpressions( + ImmutableArray> matches) + { + var nodesAndTokens = new List(); + for (int i = 0; i < matches.Length; i++) + { + var match = matches[i]; + var expressionStatement = match.Statement; + var assignment = (AssignmentExpressionSyntax)expressionStatement.Expression; + + var newAssignment = assignment.WithLeft( + match.MemberAccessExpression.Name.WithLeadingTrivia(match.MemberAccessExpression.GetLeadingTrivia())); + + if (i < matches.Length - 1) + { + nodesAndTokens.Add(newAssignment); + var commaToken = SyntaxFactory.Token(SyntaxKind.CommaToken) + .WithTriviaFrom(expressionStatement.SemicolonToken); + + nodesAndTokens.Add(commaToken); + } + else + { + newAssignment = newAssignment.WithTrailingTrivia( + expressionStatement.GetTrailingTrivia()); + nodesAndTokens.Add(newAssignment); + } + } + + return SyntaxFactory.SeparatedList(nodesAndTokens); } } } \ No newline at end of file diff --git a/src/Features/CSharp/Portable/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs index 080d27f9d8a..b261e4dbd81 100644 --- a/src/Features/CSharp/Portable/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs +++ b/src/Features/CSharp/Portable/UseObjectInitializer/CSharpUseObjectInitializerDiagnosticAnalyzer.cs @@ -1,7 +1,5 @@ // 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.Generic; -using System.Collections.Immutable; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.LanguageServices; @@ -32,50 +30,5 @@ protected override bool AreObjectInitializersSupported(SyntaxNodeAnalysisContext protected override SyntaxKind GetObjectCreationSyntaxKind() => SyntaxKind.ObjectCreationExpression; protected override ISyntaxFactsService GetSyntaxFactsService() => CSharpSyntaxFactsService.Instance; - - protected override ObjectCreationExpressionSyntax GetNewObjectCreation( - ObjectCreationExpressionSyntax objectCreation, - ImmutableArray matches) - { - var openBrace = SyntaxFactory.Token(SyntaxKind.OpenBraceToken) - .WithTrailingTrivia(SyntaxFactory.ElasticCarriageReturnLineFeed); - var initializer = SyntaxFactory.InitializerExpression( - SyntaxKind.ObjectInitializerExpression, - CreateExpressions(matches)).WithOpenBraceToken(openBrace); - - return objectCreation.WithInitializer(initializer); - } - - private SeparatedSyntaxList CreateExpressions( - ImmutableArray matches) - { - var nodesAndTokens = new List(); - for (int i = 0; i < matches.Length; i++) - { - var match = matches[i]; - var expressionStatement = match.Statement; - var assignment = (AssignmentExpressionSyntax)expressionStatement.Expression; - - var newAssignment = assignment.WithLeft( - match.MemberAccessExpression.Name.WithLeadingTrivia(match.MemberAccessExpression.GetLeadingTrivia())); - - if (i < matches.Length - 1) - { - nodesAndTokens.Add(newAssignment); - var commaToken = SyntaxFactory.Token(SyntaxKind.CommaToken) - .WithTriviaFrom(expressionStatement.SemicolonToken); - - nodesAndTokens.Add(commaToken); - } - else - { - newAssignment = newAssignment.WithTrailingTrivia( - expressionStatement.GetTrailingTrivia()); - nodesAndTokens.Add(newAssignment); - } - } - - return SyntaxFactory.SeparatedList(nodesAndTokens); - } } } \ No newline at end of file diff --git a/src/Features/Core/Portable/Features.csproj b/src/Features/Core/Portable/Features.csproj index 2bae17a5d7f..a04fce5851e 100644 --- a/src/Features/Core/Portable/Features.csproj +++ b/src/Features/Core/Portable/Features.csproj @@ -143,7 +143,7 @@ - + diff --git a/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs b/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs index e04ef9d8e4f..aee76ebc0b7 100644 --- a/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs +++ b/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerCodeFixProvider.cs @@ -1,6 +1,7 @@ // 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.Generic; using System.Collections.Immutable; using System.Linq; using System.Threading; @@ -9,6 +10,8 @@ using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Diagnostics; using Microsoft.CodeAnalysis.Editing; +using Microsoft.CodeAnalysis.Formatting; +using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; @@ -31,14 +34,6 @@ internal abstract class AbstractUseObjectInitializerCodeFixProvider< where TAssignmentStatementSyntax : TStatementSyntax where TVariableDeclaratorSyntax : SyntaxNode { - private readonly AbstractUseObjectInitializerDiagnosticAnalyzer _analyzer; - - protected AbstractUseObjectInitializerCodeFixProvider( - AbstractUseObjectInitializerDiagnosticAnalyzer analyzer) - { - _analyzer = analyzer; - } - public override ImmutableArray FixableDiagnosticIds => ImmutableArray.Create(IDEDiagnosticIds.UseObjectInitializerDiagnosticId); @@ -57,9 +52,68 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context) Document document, ImmutableArray diagnostics, SyntaxEditor editor, CancellationToken cancellationToken) { - return _analyzer.FixAllAsync(document, diagnostics, editor, cancellationToken); + // Fix-All for this feature is somewhat complicated. As Object-Initializers + // could be arbitrarily nested, we have to make sure that any edits we make + // to one Object-Initializer are seen by any higher ones. In order to do this + // we actually process each object-creation-node, one at a time, rewriting + // the tree for each node. In order to do this effectively, we use the '.TrackNodes' + // feature to keep track of all the object creation nodes as we make edits to + // the tree. If we didn't do this, then we wouldn't be able to find the + // second object-creation-node after we make the edit for the first one. + var workspace = document.Project.Solution.Workspace; + var syntaxFacts = document.GetLanguageService(); + + var originalRoot = editor.OriginalRoot; + var originalObjectCreationNodes = new Stack(); + foreach (var diagnostic in diagnostics) + { + var objectCreation = (TObjectCreationExpressionSyntax)originalRoot.FindNode( + diagnostic.AdditionalLocations[0].SourceSpan, getInnermostNodeForTie: true); + originalObjectCreationNodes.Push(objectCreation); + } + + // We're going to be continually editing this tree. Track all the nodes we + // care about so we can find them across each edit. + var currentRoot = originalRoot.TrackNodes(originalObjectCreationNodes); + + while (originalObjectCreationNodes.Count > 0) + { + var originalObjectCreation = originalObjectCreationNodes.Pop(); + var objectCreation = currentRoot.GetCurrentNodes(originalObjectCreation).Single(); + + var analyzer = new Analyzer( + syntaxFacts, objectCreation); + var matches = analyzer.Analyze(); + + if (matches == null || matches.Value.Length == 0) + { + continue; + } + + var statement = objectCreation.FirstAncestorOrSelf(); + var newStatement = statement.ReplaceNode( + objectCreation, + GetNewObjectCreation(objectCreation, matches.Value)).WithAdditionalAnnotations(Formatter.Annotation); + + var subEditor = new SyntaxEditor(currentRoot, workspace); + + subEditor.ReplaceNode(statement, newStatement); + foreach (var match in matches) + { + subEditor.RemoveNode(match.Statement); + } + + currentRoot = subEditor.GetChangedRoot(); + } + + editor.ReplaceNode(editor.OriginalRoot, currentRoot); + return SpecializedTasks.EmptyTask; } + protected abstract TObjectCreationExpressionSyntax GetNewObjectCreation( + TObjectCreationExpressionSyntax objectCreation, + ImmutableArray> matches); + private class MyCodeAction : CodeAction.DocumentChangeAction { public MyCodeAction(Func> createChangedDocument) diff --git a/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.Analyzer.cs b/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.Analyzer.cs deleted file mode 100644 index 454e58a31e6..00000000000 --- a/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.Analyzer.cs +++ /dev/null @@ -1,282 +0,0 @@ -// 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.Generic; -using Microsoft.CodeAnalysis.CodeStyle; -using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.LanguageServices; -using System.Linq; -using System.Collections.Immutable; - -namespace Microsoft.CodeAnalysis.UseObjectInitializer -{ - internal partial class AbstractUseObjectInitializerDiagnosticAnalyzer< - TSyntaxKind, - TExpressionSyntax, - TStatementSyntax, - TObjectCreationExpressionSyntax, - TMemberAccessExpressionSyntax, - TAssignmentStatementSyntax, - TVariableDeclaratorSyntax> - { - private struct Analyzer - { - private readonly ISyntaxFactsService _syntaxFacts; - private readonly TObjectCreationExpressionSyntax _objectCreationExpression; - - private TStatementSyntax _containingStatement; - private SyntaxNodeOrToken _valuePattern; - - public Analyzer( - ISyntaxFactsService syntaxFacts, - TObjectCreationExpressionSyntax objectCreationExpression) : this() - { - _syntaxFacts = syntaxFacts; - _objectCreationExpression = objectCreationExpression; - } - - internal ImmutableArray Analyze() - { - if (_syntaxFacts.GetObjectCreationInitializer(_objectCreationExpression) != null) - { - // Don't bother if this already has an initializer. - return ImmutableArray.Empty; - } - - _containingStatement = _objectCreationExpression.FirstAncestorOrSelf(); - if (_containingStatement == null) - { - return ImmutableArray.Empty; - } - - if (!TryInitializeVariableDeclarationCase() && - !TryInitializeAssignmentCase()) - { - return ImmutableArray.Empty; - } - - var containingBlock = _containingStatement.Parent; - var foundStatement = false; - - var matches = ArrayBuilder.GetInstance(); - HashSet seenNames = null; - - foreach (var child in containingBlock.ChildNodesAndTokens()) - { - if (!foundStatement) - { - if (child == _containingStatement) - { - foundStatement = true; - continue; - } - - continue; - } - - if (child.IsToken) - { - break; - } - - var statement = child.AsNode() as TAssignmentStatementSyntax; - if (statement == null) - { - break; - } - - if (!_syntaxFacts.IsSimpleAssignmentStatement(statement)) - { - break; - } - - _syntaxFacts.GetPartsOfAssignmentStatement( - statement, out var left, out var right); - - var rightExpression = right as TExpressionSyntax; - var leftMemberAccess = left as TMemberAccessExpressionSyntax; - - if (!_syntaxFacts.IsSimpleMemberAccessExpression(leftMemberAccess)) - { - break; - } - - var expression = (TExpressionSyntax)_syntaxFacts.GetExpressionOfMemberAccessExpression(leftMemberAccess); - if (!ValuePatternMatches(expression)) - { - break; - } - - // Don't offer this fix if the value we're initializing is itself referenced - // on the RHS of the assignment. For example: - // - // var v = new X(); - // v.Prop = v.Prop.WithSomething(); - // - // Or with - // - // v = new X(); - // v.Prop = v.Prop.WithSomething(); - // - // In the first case, 'v' is being initialized, and so will not be available - // in the object initializer we create. - // - // In the second case we'd change semantics because we'd access the old value - // before the new value got written. - if (ExpressionContainsValuePattern(rightExpression)) - { - break; - } - - // If we have code like "x.v = .Length.ToString()" - // then we don't want to change this into: - // - // var x = new Whatever() With { .v = .Length.ToString() } - // - // The problem here is that .Length will change it's meaning to now refer to the - // object that we're creating in our object-creation expression. - if (ImplicitMemberAccessWouldBeAffected(rightExpression)) - { - break; - } - - // found a match! - seenNames = seenNames ?? new HashSet(); - - // If we see an assignment to the same property/field, we can't convert it - // to an initializer. - var name = _syntaxFacts.GetNameOfMemberAccessExpression(leftMemberAccess); - var identifier = _syntaxFacts.GetIdentifierOfSimpleName(name); - if (!seenNames.Add(identifier.ValueText)) - { - break; - } - - matches.Add(new Match(statement, leftMemberAccess, rightExpression)); - } - - return matches.ToImmutableAndFree(); - } - - private bool ImplicitMemberAccessWouldBeAffected(SyntaxNode node) - { - if (node != null) - { - foreach (var child in node.ChildNodesAndTokens()) - { - if (child.IsNode) - { - if (ImplicitMemberAccessWouldBeAffected(child.AsNode())) - { - return true; - } - } - } - - if (_syntaxFacts.IsSimpleMemberAccessExpression(node)) - { - var expression = _syntaxFacts.GetExpressionOfMemberAccessExpression( - node, allowImplicitTarget: true); - - // If we're implicitly referencing some target that is before the - // object creation expression, then our semantics will change. - if (expression != null && expression.SpanStart < _objectCreationExpression.SpanStart) - { - return true; - } - } - } - - return false; - } - - private bool ExpressionContainsValuePattern(TExpressionSyntax expression) - { - foreach (var subExpression in expression.DescendantNodesAndSelf().OfType()) - { - if (!_syntaxFacts.IsNameOfMemberAccessExpression(subExpression)) - { - if (ValuePatternMatches(subExpression)) - { - return true; - } - } - } - - return false; - } - - private bool ValuePatternMatches(TExpressionSyntax expression) - { - if (_valuePattern.IsToken) - { - return _syntaxFacts.IsIdentifierName(expression) && - _syntaxFacts.AreEquivalent( - _valuePattern.AsToken(), - _syntaxFacts.GetIdentifierOfSimpleName(expression)); - } - else - { - return _syntaxFacts.AreEquivalent( - _valuePattern.AsNode(), expression); - } - } - - private bool TryInitializeAssignmentCase() - { - if (!_syntaxFacts.IsSimpleAssignmentStatement(_containingStatement)) - { - return false; - } - - _syntaxFacts.GetPartsOfAssignmentStatement( - _containingStatement, out var left, out var right); - if (right != _objectCreationExpression) - { - return false; - } - - _valuePattern = left; - return true; - } - - private bool TryInitializeVariableDeclarationCase() - { - if (!_syntaxFacts.IsLocalDeclarationStatement(_containingStatement)) - { - return false; - } - - var containingDeclarator = _objectCreationExpression.FirstAncestorOrSelf(); - if (containingDeclarator == null) - { - return false; - } - - if (!_syntaxFacts.IsDeclaratorOfLocalDeclarationStatement(containingDeclarator, _containingStatement)) - { - return false; - } - - _valuePattern = _syntaxFacts.GetIdentifierOfVariableDeclarator(containingDeclarator); - return true; - } - } - - internal struct Match - { - public readonly TAssignmentStatementSyntax Statement; - public readonly TMemberAccessExpressionSyntax MemberAccessExpression; - public readonly TExpressionSyntax Initializer; - - public Match( - TAssignmentStatementSyntax statement, - TMemberAccessExpressionSyntax memberAccessExpression, - TExpressionSyntax initializer) - { - Statement = statement; - MemberAccessExpression = memberAccessExpression; - Initializer = initializer; - } - } - } -} \ No newline at end of file diff --git a/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs b/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs index 73c9f1b9f05..89d6b9ea938 100644 --- a/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs +++ b/src/Features/Core/Portable/UseObjectInitializer/AbstractUseObjectInitializerDiagnosticAnalyzer.cs @@ -1,19 +1,11 @@ // 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.Generic; using System.Collections.Immutable; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; using Microsoft.CodeAnalysis.CodeStyle; using Microsoft.CodeAnalysis.Diagnostics; -using Microsoft.CodeAnalysis.Editing; -using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Options; -using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; -using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.UseObjectInitializer { @@ -70,8 +62,12 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context) return; } - var result = Analyze(objectCreationExpression); - if (result.Length == 0) + var syntaxFacts = GetSyntaxFactsService(); + var analyzer = new Analyzer( + syntaxFacts, objectCreationExpression); + var result = analyzer.Analyze(); + + if (result == null || result.Value.Length == 0) { return; } @@ -84,21 +80,13 @@ private void AnalyzeNode(SyntaxNodeAnalysisContext context) objectCreationExpression.GetLocation(), additionalLocations: locations)); - FadeOutCode(context, optionSet, result, locations); - } - - public ImmutableArray Analyze(TObjectCreationExpressionSyntax objectCreationExpression) - { - var syntaxFacts = GetSyntaxFactsService(); - var analyzer = new Analyzer(syntaxFacts, objectCreationExpression); - var result = analyzer.Analyze(); - return result; + FadeOutCode(context, optionSet, result.Value, locations); } private void FadeOutCode( SyntaxNodeAnalysisContext context, OptionSet optionSet, - ImmutableArray matches, + ImmutableArray> matches, ImmutableArray locations) { var syntaxTree = context.Node.SyntaxTree; @@ -142,64 +130,5 @@ public DiagnosticAnalyzerCategory GetAnalyzerCategory() { return DiagnosticAnalyzerCategory.SemanticDocumentAnalysis; } - - public Task FixAllAsync( - Document document, ImmutableArray diagnostics, - SyntaxEditor editor, CancellationToken cancellationToken) - { - // Fix-All for this feature is somewhat complicated. As Object-Initializers - // could be arbitrarily nested, we have to make sure that any edits we make - // to one Object-Initializer are seen by any higher ones. In order to do this - // we actually process each object-creation-node, one at a time, rewriting - // the tree for each node. In order to do this effectively, we use the '.TrackNodes' - // feature to keep track of all the object creation nodes as we make edits to - // the tree. If we didn't do this, then we wouldn't be able to find the - // second object-creation-node after we make the edit for the first one. - var workspace = document.Project.Solution.Workspace; - var syntaxFacts = document.GetLanguageService(); - - var originalRoot = editor.OriginalRoot; - var originalObjectCreationNodes = new Stack(); - foreach (var diagnostic in diagnostics) - { - var objectCreation = (TObjectCreationExpressionSyntax)originalRoot.FindNode( - diagnostic.AdditionalLocations[0].SourceSpan, getInnermostNodeForTie: true); - originalObjectCreationNodes.Push(objectCreation); - } - - // We're going to be continually editing this tree. Track all the nodes we - // care about so we can find them across each edit. - var currentRoot = originalRoot.TrackNodes(originalObjectCreationNodes); - - while (originalObjectCreationNodes.Count > 0) - { - var originalObjectCreation = originalObjectCreationNodes.Pop(); - var objectCreation = currentRoot.GetCurrentNodes(originalObjectCreation).Single(); - - var matches = this.Analyze(objectCreation); - - var statement = objectCreation.FirstAncestorOrSelf(); - var newStatement = statement.ReplaceNode( - objectCreation, - GetNewObjectCreation(objectCreation, matches)).WithAdditionalAnnotations(Formatter.Annotation); - - var subEditor = new SyntaxEditor(currentRoot, workspace); - - subEditor.ReplaceNode(statement, newStatement); - foreach (var match in matches) - { - subEditor.RemoveNode(match.Statement); - } - - currentRoot = subEditor.GetChangedRoot(); - } - - editor.ReplaceNode(editor.OriginalRoot, currentRoot); - return SpecializedTasks.EmptyTask; - } - - protected abstract TObjectCreationExpressionSyntax GetNewObjectCreation( - TObjectCreationExpressionSyntax objectCreation, - ImmutableArray matches); } } \ No newline at end of file diff --git a/src/Features/Core/Portable/UseObjectInitializer/Analyzer.cs b/src/Features/Core/Portable/UseObjectInitializer/Analyzer.cs new file mode 100644 index 00000000000..1281a631321 --- /dev/null +++ b/src/Features/Core/Portable/UseObjectInitializer/Analyzer.cs @@ -0,0 +1,291 @@ +// 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.Generic; +using System.Collections.Immutable; +using System.Linq; +using Microsoft.CodeAnalysis.LanguageServices; + +namespace Microsoft.CodeAnalysis.UseObjectInitializer +{ + internal struct Analyzer< + TExpressionSyntax, + TStatementSyntax, + TObjectCreationExpressionSyntax, + TMemberAccessExpressionSyntax, + TAssignmentStatementSyntax, + TVariableDeclaratorSyntax> + where TExpressionSyntax : SyntaxNode + where TStatementSyntax : SyntaxNode + where TObjectCreationExpressionSyntax : TExpressionSyntax + where TMemberAccessExpressionSyntax : TExpressionSyntax + where TAssignmentStatementSyntax : TStatementSyntax + where TVariableDeclaratorSyntax : SyntaxNode + { + private readonly ISyntaxFactsService _syntaxFacts; + private readonly TObjectCreationExpressionSyntax _objectCreationExpression; + + private TStatementSyntax _containingStatement; + private SyntaxNodeOrToken _valuePattern; + + public Analyzer( + ISyntaxFactsService syntaxFacts, + TObjectCreationExpressionSyntax objectCreationExpression) : this() + { + _syntaxFacts = syntaxFacts; + _objectCreationExpression = objectCreationExpression; + } + + internal ImmutableArray>? Analyze() + { + if (_syntaxFacts.GetObjectCreationInitializer(_objectCreationExpression) != null) + { + // Don't bother if this already has an initializer. + return null; + } + + _containingStatement = _objectCreationExpression.FirstAncestorOrSelf(); + if (_containingStatement == null) + { + return null; + } + + if (!TryInitializeVariableDeclarationCase() && + !TryInitializeAssignmentCase()) + { + return null; + } + + var containingBlock = _containingStatement.Parent; + var foundStatement = false; + + var matches = ArrayBuilder>.GetInstance(); + HashSet seenNames = null; + + foreach (var child in containingBlock.ChildNodesAndTokens()) + { + if (!foundStatement) + { + if (child == _containingStatement) + { + foundStatement = true; + continue; + } + + continue; + } + + if (child.IsToken) + { + break; + } + + var statement = child.AsNode() as TAssignmentStatementSyntax; + if (statement == null) + { + break; + } + + if (!_syntaxFacts.IsSimpleAssignmentStatement(statement)) + { + break; + } + + _syntaxFacts.GetPartsOfAssignmentStatement( + statement, out var left, out var right); + + var rightExpression = right as TExpressionSyntax; + var leftMemberAccess = left as TMemberAccessExpressionSyntax; + + if (!_syntaxFacts.IsSimpleMemberAccessExpression(leftMemberAccess)) + { + break; + } + + var expression = (TExpressionSyntax)_syntaxFacts.GetExpressionOfMemberAccessExpression(leftMemberAccess); + if (!ValuePatternMatches(expression)) + { + break; + } + + // Don't offer this fix if the value we're initializing is itself referenced + // on the RHS of the assignment. For example: + // + // var v = new X(); + // v.Prop = v.Prop.WithSomething(); + // + // Or with + // + // v = new X(); + // v.Prop = v.Prop.WithSomething(); + // + // In the first case, 'v' is being initialized, and so will not be available + // in the object initializer we create. + // + // In the second case we'd change semantics because we'd access the old value + // before the new value got written. + if (ExpressionContainsValuePattern(rightExpression)) + { + break; + } + + // If we have code like "x.v = .Length.ToString()" + // then we don't want to change this into: + // + // var x = new Whatever() With { .v = .Length.ToString() } + // + // The problem here is that .Length will change it's meaning to now refer to the + // object that we're creating in our object-creation expression. + if (ImplicitMemberAccessWouldBeAffected(rightExpression)) + { + break; + } + + // found a match! + seenNames = seenNames ?? new HashSet(); + + // If we see an assignment to the same property/field, we can't convert it + // to an initializer. + var name = _syntaxFacts.GetNameOfMemberAccessExpression(leftMemberAccess); + var identifier = _syntaxFacts.GetIdentifierOfSimpleName(name); + if (!seenNames.Add(identifier.ValueText)) + { + break; + } + + matches.Add(new Match( + statement, leftMemberAccess, rightExpression)); + } + + return matches.ToImmutableAndFree(); + } + + private bool ImplicitMemberAccessWouldBeAffected(SyntaxNode node) + { + if (node != null) + { + foreach (var child in node.ChildNodesAndTokens()) + { + if (child.IsNode) + { + if (ImplicitMemberAccessWouldBeAffected(child.AsNode())) + { + return true; + } + } + } + + if (_syntaxFacts.IsSimpleMemberAccessExpression(node)) + { + var expression = _syntaxFacts.GetExpressionOfMemberAccessExpression( + node, allowImplicitTarget: true); + + // If we're implicitly referencing some target that is before the + // object creation expression, then our semantics will change. + if (expression != null && expression.SpanStart < _objectCreationExpression.SpanStart) + { + return true; + } + } + } + + return false; + } + + private bool ExpressionContainsValuePattern(TExpressionSyntax expression) + { + foreach (var subExpression in expression.DescendantNodesAndSelf().OfType()) + { + if (!_syntaxFacts.IsNameOfMemberAccessExpression(subExpression)) + { + if (ValuePatternMatches(subExpression)) + { + return true; + } + } + } + + return false; + } + + private bool ValuePatternMatches(TExpressionSyntax expression) + { + if (_valuePattern.IsToken) + { + return _syntaxFacts.IsIdentifierName(expression) && + _syntaxFacts.AreEquivalent( + _valuePattern.AsToken(), + _syntaxFacts.GetIdentifierOfSimpleName(expression)); + } + else + { + return _syntaxFacts.AreEquivalent( + _valuePattern.AsNode(), expression); + } + } + + private bool TryInitializeAssignmentCase() + { + if (!_syntaxFacts.IsSimpleAssignmentStatement(_containingStatement)) + { + return false; + } + + _syntaxFacts.GetPartsOfAssignmentStatement( + _containingStatement, out var left, out var right); + if (right != _objectCreationExpression) + { + return false; + } + + _valuePattern = left; + return true; + } + + private bool TryInitializeVariableDeclarationCase() + { + if (!_syntaxFacts.IsLocalDeclarationStatement(_containingStatement)) + { + return false; + } + + var containingDeclarator = _objectCreationExpression.FirstAncestorOrSelf(); + if (containingDeclarator == null) + { + return false; + } + + if (!_syntaxFacts.IsDeclaratorOfLocalDeclarationStatement(containingDeclarator, _containingStatement)) + { + return false; + } + + _valuePattern = _syntaxFacts.GetIdentifierOfVariableDeclarator(containingDeclarator); + return true; + } + } + + internal struct Match< + TExpressionSyntax, + TStatementSyntax, + TMemberAccessExpressionSyntax, + TAssignmentStatementSyntax> + where TExpressionSyntax : SyntaxNode + where TStatementSyntax : SyntaxNode + where TMemberAccessExpressionSyntax : TExpressionSyntax + where TAssignmentStatementSyntax : TStatementSyntax + { + public readonly TAssignmentStatementSyntax Statement; + public readonly TMemberAccessExpressionSyntax MemberAccessExpression; + public readonly TExpressionSyntax Initializer; + + public Match( + TAssignmentStatementSyntax statement, + TMemberAccessExpressionSyntax memberAccessExpression, + TExpressionSyntax initializer) + { + Statement = statement; + MemberAccessExpression = memberAccessExpression; + Initializer = initializer; + } + } +} \ No newline at end of file diff --git a/src/Features/VisualBasic/Portable/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb b/src/Features/VisualBasic/Portable/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb index 93632409a19..bdbf4fbee3f 100644 --- a/src/Features/VisualBasic/Portable/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb +++ b/src/Features/VisualBasic/Portable/UseObjectInitializer/VisualBasicUseObjectInitializerCodeFixProvider.vb @@ -1,5 +1,6 @@ ' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. +Imports System.Collections.Immutable Imports System.Composition Imports Microsoft.CodeAnalysis.CodeFixes Imports Microsoft.CodeAnalysis.UseObjectInitializer @@ -17,8 +18,46 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer AssignmentStatementSyntax, VariableDeclaratorSyntax) - Public Sub New() - MyBase.New(New VisualBasicUseObjectInitializerDiagnosticAnalyzer()) - End Sub + Protected Overrides Function GetNewObjectCreation( + objectCreation As ObjectCreationExpressionSyntax, + matches As ImmutableArray(Of Match(Of ExpressionSyntax, StatementSyntax, MemberAccessExpressionSyntax, AssignmentStatementSyntax))) As ObjectCreationExpressionSyntax + + Dim initializer = SyntaxFactory.ObjectMemberInitializer( + CreateFieldInitializers(matches)) + + Return objectCreation.WithoutTrailingTrivia(). + WithInitializer(initializer). + WithTrailingTrivia(objectCreation.GetTrailingTrivia()) + End Function + + Private Function CreateFieldInitializers( + matches As ImmutableArray(Of Match(Of ExpressionSyntax, StatementSyntax, MemberAccessExpressionSyntax, AssignmentStatementSyntax))) As SeparatedSyntaxList(Of FieldInitializerSyntax) + Dim nodesAndTokens = New List(Of SyntaxNodeOrToken) + + For i = 0 To matches.Length - 1 + Dim match = matches(i) + + Dim rightValue = match.Initializer + If i < matches.Count - 1 Then + rightValue = rightValue.WithoutTrailingTrivia() + End If + + Dim initializer = SyntaxFactory.NamedFieldInitializer( + keyKeyword:=Nothing, + dotToken:=match.MemberAccessExpression.OperatorToken, + name:=DirectCast(match.MemberAccessExpression.Name, IdentifierNameSyntax), + equalsToken:=match.Statement.OperatorToken, + expression:=rightValue).WithPrependedLeadingTrivia(SyntaxFactory.ElasticMarker) + + nodesAndTokens.Add(initializer) + If i < matches.Length - 1 Then + Dim comma = SyntaxFactory.Token(SyntaxKind.CommaToken). + WithTrailingTrivia(match.Initializer.GetTrailingTrivia()) + nodesAndTokens.Add(comma) + End If + Next + + Return SyntaxFactory.SeparatedList(Of FieldInitializerSyntax)(nodesAndTokens) + End Function End Class End Namespace \ No newline at end of file diff --git a/src/Features/VisualBasic/Portable/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb b/src/Features/VisualBasic/Portable/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb index 5ade45b05df..72ff5a89e91 100644 --- a/src/Features/VisualBasic/Portable/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb +++ b/src/Features/VisualBasic/Portable/UseObjectInitializer/VisualBasicUseObjectInitializerDiagnosticAnalyzer.vb @@ -1,6 +1,5 @@ ' Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -Imports System.Collections.Immutable Imports Microsoft.CodeAnalysis.Diagnostics Imports Microsoft.CodeAnalysis.LanguageServices Imports Microsoft.CodeAnalysis.UseObjectInitializer @@ -36,47 +35,5 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseObjectInitializer Protected Overrides Function GetSyntaxFactsService() As ISyntaxFactsService Return VisualBasicSyntaxFactsService.Instance End Function - - Protected Overrides Function GetNewObjectCreation( - objectCreation As ObjectCreationExpressionSyntax, - matches As ImmutableArray(Of Match)) As ObjectCreationExpressionSyntax - - Dim initializer = SyntaxFactory.ObjectMemberInitializer( - CreateFieldInitializers(matches)) - - Return objectCreation.WithoutTrailingTrivia(). - WithInitializer(initializer). - WithTrailingTrivia(objectCreation.GetTrailingTrivia()) - End Function - - Private Function CreateFieldInitializers( - matches As ImmutableArray(Of Match)) As SeparatedSyntaxList(Of FieldInitializerSyntax) - Dim nodesAndTokens = New List(Of SyntaxNodeOrToken) - - For i = 0 To matches.Length - 1 - Dim match = matches(i) - - Dim rightValue = match.Initializer - If i < matches.Count - 1 Then - rightValue = rightValue.WithoutTrailingTrivia() - End If - - Dim initializer = SyntaxFactory.NamedFieldInitializer( - keyKeyword:=Nothing, - dotToken:=match.MemberAccessExpression.OperatorToken, - name:=DirectCast(match.MemberAccessExpression.Name, IdentifierNameSyntax), - equalsToken:=match.Statement.OperatorToken, - expression:=rightValue).WithPrependedLeadingTrivia(SyntaxFactory.ElasticMarker) - - nodesAndTokens.Add(initializer) - If i < matches.Length - 1 Then - Dim comma = SyntaxFactory.Token(SyntaxKind.CommaToken). - WithTrailingTrivia(match.Initializer.GetTrailingTrivia()) - nodesAndTokens.Add(comma) - End If - Next - - Return SyntaxFactory.SeparatedList(Of FieldInitializerSyntax)(nodesAndTokens) - End Function End Class End Namespace \ No newline at end of file -- GitLab