Skip to content
Draft
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<?xml version="1.0" encoding="utf-8" ?>
<diagnostics severity="Info">
<group id="WTG3014" message="Don't use Concat when prepending a single element to an enumerable.">
<diagnostic>
<location>Test0.cs: (15, 3-36)</location>
</diagnostic>
<diagnostic>
<location>Test0.cs: (16, 3-20)</location>
</diagnostic>
</group>
<group id="WTG3013" message="Don't use Concat when appending a single element to an enumerable.">
<diagnostic>
<location>Test0.cs: (17, 3-25)</location>
</diagnostic>
<diagnostic>
<location>Test0.cs: (18, 3-20)</location>
</diagnostic>
</group>
</diagnostics>
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using System.Collections.Generic;
using System.Linq;

public class ViewModel

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@copilot
ViewModel should be placed after Bob so that future changes are less likely to require location updates in Diagnostics.xml.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in ba72978. Moved ViewModel after Bob in both Source.cs and Result.cs.

{
public IEnumerable<ViewModel> Items { get; set; }
}

public class Bob
{
public void Method()
{
var viewModel = new ViewModel();

viewModel.Items.Prepend<object>(viewModel);
Enumerable.Prepend<object>(viewModel.Items, viewModel);
viewModel.Items.Append<object>(viewModel);
Enumerable.Append<object>(viewModel.Items, viewModel);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
using System.Collections.Generic;
using System.Linq;

public class ViewModel
{
public IEnumerable<ViewModel> Items { get; set; }
}

public class Bob
{
public void Method()
{
var viewModel = new ViewModel();

new object[] { viewModel }.Concat(viewModel.Items);
Enumerable.Concat(new object[] { viewModel }, viewModel.Items);
viewModel.Items.Concat(new object[] { viewModel });
Enumerable.Concat(viewModel.Items, new object[] { viewModel });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public override Task RegisterCodeFixesAsync(CodeFixContext context)
public static async Task<Document> ReplaceWithAppropriateMethod(Document document, Diagnostic diagnostic, CancellationToken c)
{
var root = await document.RequireSyntaxRootAsync(c).ConfigureAwait(true);
var semanticModel = await document.RequireSemanticModelAsync(c).ConfigureAwait(true);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@copilot, using SemanticModel is expensive, so we should avoid it where possible. In this case, we should be able to take the TypeSyntax from the new array expression and use that as the type argument (after trimming trivia and adding Simplifier.Annotation).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done in ba72978. Replaced SemanticModel usage with syntactic extraction of the TypeSyntax from the array/object creation expression. The type argument is now annotated with Simplifier.Annotation so Roslyn removes it when it's redundant (e.g., new int[] { 5 }.Concat(a) still produces a.Prepend(5)).


var memberAccessExpression = (MemberAccessExpressionSyntax)root.FindNode(diagnostic.Location.SourceSpan, getInnermostNodeForTie: true);

Expand All @@ -75,7 +76,7 @@ public static async Task<Document> ReplaceWithAppropriateMethod(Document documen
return document;
}

var newNode = FixMemberAccessExpression(memberAccessExpression, diagnostic);
var newNode = FixMemberAccessExpression(memberAccessExpression, diagnostic, semanticModel);

if (newNode == null)
{
Expand All @@ -86,18 +87,18 @@ public static async Task<Document> ReplaceWithAppropriateMethod(Document documen
memberAccessExpression.Parent, newNode));
}

public static SyntaxNode? FixMemberAccessExpression(MemberAccessExpressionSyntax m, Diagnostic d)
public static SyntaxNode? FixMemberAccessExpression(MemberAccessExpressionSyntax m, Diagnostic d, SemanticModel semanticModel)
{
return d.Id switch
{
Rules.DontUseConcatWhenAppendingSingleElementToEnumerablesDiagnosticID => FixConcatWithAppendMethod(m),
Rules.DontUseConcatWhenPrependingSingleElementToEnumerablesDiagnosticID => FixConcatWithPrependMethod(m),
Rules.DontUseConcatWhenAppendingSingleElementToEnumerablesDiagnosticID => FixConcatWithAppendMethod(m, semanticModel),
Rules.DontUseConcatWhenPrependingSingleElementToEnumerablesDiagnosticID => FixConcatWithPrependMethod(m, semanticModel),
Rules.DontConcatTwoCollectionsDefinedWithLiteralsDiagnosticID => FixConcatWithNewCollection(m),
_ => null,
};
}

public static SyntaxNode FixConcatWithAppendMethod(MemberAccessExpressionSyntax m)
public static SyntaxNode FixConcatWithAppendMethod(MemberAccessExpressionSyntax m, SemanticModel semanticModel)
{
var invocation = (InvocationExpressionSyntax?)m.Parent;
NRT.Assert(invocation != null, "MemberAccessExpression should have a parent.");
Expand Down Expand Up @@ -125,15 +126,15 @@ public static SyntaxNode FixConcatWithAppendMethod(MemberAccessExpressionSyntax
.WithTriviaFrom(m.Expression)
.WithAdditionalAnnotations(Simplifier.Annotation),
m.OperatorToken,
IdentifierName(nameof(Enumerable.Append))
GetMethodName(nameof(Enumerable.Append), invocation, semanticModel)
.WithTriviaFrom(m.Name)))
.WithArgumentList(
ArgumentList(
SeparatedList<ArgumentSyntax>(listOfArgumentsAndSeparators)))
.WithTriviaFrom(invocation);
}

public static SyntaxNode? FixConcatWithPrependMethod(MemberAccessExpressionSyntax m)
public static SyntaxNode? FixConcatWithPrependMethod(MemberAccessExpressionSyntax m, SemanticModel semanticModel)
{
var invocation = (InvocationExpressionSyntax?)m.Parent;
NRT.Assert(invocation != null, "MemberAccessExpression should have a parent.");
Expand Down Expand Up @@ -166,7 +167,7 @@ public static SyntaxNode FixConcatWithAppendMethod(MemberAccessExpressionSyntax
SyntaxKind.SimpleMemberAccessExpression,
member,
m.OperatorToken,
IdentifierName(nameof(Enumerable.Prepend))
GetMethodName(nameof(Enumerable.Prepend), invocation, semanticModel)
.WithTriviaFrom(m.Name)))
.WithArgumentList(
ArgumentList(
Expand Down Expand Up @@ -213,5 +214,97 @@ public static SyntaxNode FixConcatWithNewCollection(MemberAccessExpressionSyntax
.WithTriviaFrom(invocation)
.WithAdditionalAnnotations(Simplifier.Annotation);
}

static SimpleNameSyntax GetMethodName(string methodName, InvocationExpressionSyntax invocation, SemanticModel semanticModel)
{
if (NeedsExplicitTypeArgument(invocation, semanticModel, out var typeArgument))
{
return GenericName(Identifier(methodName))
.WithTypeArgumentList(
TypeArgumentList(
SingletonSeparatedList(typeArgument)));
}

return IdentifierName(methodName);
}

static bool NeedsExplicitTypeArgument(InvocationExpressionSyntax invocation, SemanticModel semanticModel, out TypeSyntax typeArgument)
{
var methodSymbol = semanticModel.GetSymbolInfo(invocation).Symbol as IMethodSymbol;

if (methodSymbol != null && methodSymbol.TypeArguments.Length == 1)
{
var concatTypeArg = methodSymbol.TypeArguments[0];
var elementExpression = GetElementExpression(invocation);

if (elementExpression != null)
{
var elementType = semanticModel.GetTypeInfo(elementExpression).Type;

if (elementType != null && !SymbolEqualityComparer.Default.Equals(elementType, concatTypeArg))
{
typeArgument = ParseTypeName(concatTypeArg.ToMinimalDisplayString(semanticModel, invocation.SpanStart));
return true;
}
}
}

typeArgument = null!;
return false;
}

static ExpressionSyntax? GetElementExpression(InvocationExpressionSyntax invocation)
{
var memberAccess = invocation.Expression as MemberAccessExpressionSyntax;

if (memberAccess == null)
{
return null;
}

var arguments = invocation.ArgumentList.Arguments;

if (arguments.Count == 1)
{
// Extension method style: collection.Concat(enumerable) or enumerable.Concat(collection)
// For Prepend: new T[] { element }.Concat(enumerable) - element is in m.Expression
// For Append: enumerable.Concat(new T[] { element }) - element is in arguments[0]
var receiverExpr = memberAccess.Expression.TryGetExpressionFromParenthesizedExpression();
var argExpr = arguments[0].Expression.TryGetExpressionFromParenthesizedExpression();

// Check which one is the single-element collection
var receiverFirstValue = LinqEnumerableUtils.GetFirstValue(receiverExpr);
if (receiverFirstValue != null)
{
return receiverFirstValue;
}

var argFirstValue = LinqEnumerableUtils.GetFirstValue(argExpr);
if (argFirstValue != null)
{
return argFirstValue;
}
}
else if (arguments.Count == 2)
{
// Static method style: Enumerable.Concat(collection, enumerable)
var arg0Expr = arguments[0].Expression.TryGetExpressionFromParenthesizedExpression();
var arg1Expr = arguments[1].Expression.TryGetExpressionFromParenthesizedExpression();

var arg0FirstValue = LinqEnumerableUtils.GetFirstValue(arg0Expr);
if (arg0FirstValue != null)
{
return arg0FirstValue;
}

var arg1FirstValue = LinqEnumerableUtils.GetFirstValue(arg1Expr);
if (arg1FirstValue != null)
{
return arg1FirstValue;
}
}

return null;
}
}
}
Loading