Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement CodeFix for CA2246 #3694

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
// 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.Composition;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using Analyzer.Utilities;
using Analyzer.Utilities.Extensions;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.CodeActions;
using Microsoft.CodeAnalysis.CodeFixes;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;

#nullable enable
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved

namespace Microsoft.CodeQuality.Analyzers.QualityGuidelines
Youssef1313 marked this conversation as resolved.
Show resolved Hide resolved
{
[ExportCodeFixProvider(LanguageNames.CSharp, Name = AssigningSymbolAndItsMemberInSameStatement.RuleId), Shared]
public sealed class AssigningSymbolAndItsMemberInSameStatementFixer : CodeFixProvider
{
public override ImmutableArray<string> FixableDiagnosticIds => ImmutableArray.Create(AssigningSymbolAndItsMemberInSameStatement.RuleId);

public override async Task RegisterCodeFixesAsync(CodeFixContext context)
{
// For something like `a.x = a = b;`, we offer 3 code fixes:
// First:
// a = b;
// a.x = b;
// Second:
// a = b;
// a.x = a;
// Third: (Not currently implemented)
// var temp = a;
// a = b;
// temp.x = b;

var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
if (root.FindNode(context.Span).Parent is not AssignmentExpressionSyntax assignment)
{
return;
}

var members = GetAssignmentMembers(assignment);
if (members.Count < 3)
{
return;
}

var mostRightMember = members.Peek(); // Don't move this near its usage. The result will be different.
var leadingTrivia = assignment.Parent.GetLeadingTrivia();
var trailingTrivia = assignment.Parent.GetTrailingTrivia();
var replacements = new List<SyntaxNode>(members.Count - 1);

while (members.Count > 2)
{
replacements.Add(GetAssignmentExpressionStatement(members, leadingTrivia, trailingTrivia));
trailingTrivia = SyntaxTriviaList.Empty; // Take the trailing trivia on the first assignment only.
}

var title = MicrosoftCodeQualityAnalyzersResources.AssigningSymbolAndItsMemberInSameStatementTitle;

var replacements1 = replacements.Concat(GetAssignmentExpressionStatement(assignment.Left, mostRightMember, leadingTrivia, trailingTrivia));
var replacements2 = replacements.Concat(GetAssignmentExpressionStatement(members, leadingTrivia, trailingTrivia));

#pragma warning disable RS1010 // Provide an explicit value for EquivalenceKey - false positve
var nestedCodeAction = CodeAction.Create(title, ImmutableArray.Create<CodeAction>(
new MyCodeAction($"{title} 1", ct => GetDocument(context.Document, root, assignment.Parent, replacements1)),
new MyCodeAction($"{title} 2", ct => GetDocument(context.Document, root, assignment.Parent, replacements2))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The title parameter is what get displayed so it feels off to have XXX 1 and XXX 2.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Evangelink I have this temporary until I can get with good titles. Do you have any suggestions? Not sure how to describe the code fixes.

), isInlinable: false);
#pragma warning restore RS1010

context.RegisterCodeFix(nestedCodeAction, context.Diagnostics);
}

/// <summary>
/// If the assignment expression is: a = b = c = d
/// Return a stack containing a, b, c, d with `a` at the bottom and `d` at the top.
/// </summary>
private static Stack<ExpressionSyntax> GetAssignmentMembers(AssignmentExpressionSyntax node)
{
var stack = new Stack<ExpressionSyntax>();
ExpressionSyntax current = node;
while (current is AssignmentExpressionSyntax assignment)
{
stack.Push(assignment.Left);
current = assignment.Right;
}
stack.Push(current);
return stack;
}

private static ExpressionStatementSyntax GetAssignmentExpressionStatement(Stack<ExpressionSyntax> stack, SyntaxTriviaList leadingTrivia, SyntaxTriviaList trailingTrivia)
{
var right = stack.Pop();
var left = stack.Peek();
return GetAssignmentExpressionStatement(left, right, leadingTrivia, trailingTrivia);
}

private static ExpressionStatementSyntax GetAssignmentExpressionStatement(ExpressionSyntax left, ExpressionSyntax right, SyntaxTriviaList leadingTrivia, SyntaxTriviaList trailingTrivia)
=> SyntaxFactory.ExpressionStatement(
SyntaxFactory.AssignmentExpression(SyntaxKind.SimpleAssignmentExpression, left, right))
.WithLeadingTrivia(leadingTrivia).WithTrailingTrivia(trailingTrivia);

private static Task<Document> GetDocument(Document document, SyntaxNode root, SyntaxNode oldNode, IEnumerable<SyntaxNode> replacements)
=> Task.FromResult(document.WithSyntaxRoot(root.ReplaceNode(oldNode, replacements)));

public override FixAllProvider GetFixAllProvider()
=> WellKnownFixAllProviders.BatchFixer;

private class MyCodeAction : DocumentChangeAction
{
public MyCodeAction(string title, Func<CancellationToken, Task<Document>> createChangedDocument)
: base(title, createChangedDocument, title)
{
}
}
}
}