WI00547434 - Concat analyzer

[yaakov-h]

I also just realised that we may need a bulk-fixer for (new[] { 1 }).Concat(new[] { 2 }).Concat(new[] { 3 }) and similar cases - add a test with it and see if it passes or fails.

[theawesomew]

I also just realised that we may need a bulk-fixer for (new[] { 1 }).Concat(new[] { 2 }).Concat(new[] { 3 }) and similar cases - add a test with it and see if it passes or fails.

I'm fairly confident that something like that would end up as new [] { 1, 2 }.Append(3); in its current state because it will encounter the (new[] { 1 }).Concat(new[] { 2 }).Concat SimpleMemberAccessExpression earlier in the SyntaxTree and it won't evaluate it correctly. ~Maybe I could just change the ordering to getting the Last() in the list of diagnostics and it would evaluate it correctly.~ Nope... it'll evaluate them as different warnings and therefore treat them differently upon getting to the CodeFixer

[brian-reichle]

I'm fairly confident that something like that would end up as new [] { 1, 2 }.Append(3); in its current state because it will encounter the (new[] { 1 }).Concat(new[] { 2 }).Concat SimpleMemberAccessExpression earlier in the SyntaxTree and it won't evaluate it correctly.

When Roslyn is asked to bulk fix a set of diagnostics, it will get the 'bulk fixer' from the fix provider and ask it to apply the fixes. IIRC, the default fixer implementation will attempt to fix all the failures separately and in parallel and then try to merge the fixes. If two of the fixes are too close to each other, they cause a conflict and only one of them is retained. There are a number of reasons why we might want to create a bulk fixer, eg:

• The nature of the fixes allow for them to get close enough to cause problems. Earlier versions couldn't handle changes on neighboring lines; it's improved since then but my guess is that it's not enough for the sample that Yaakov provided.
• Some forms of fixes can be expensive (particularly if you need to touch the semantic model). Creating a custom bulk fixer can allow for some performance improvements.
• Sometimes a group of diagnostics fall into a "one or the other, but not both" bucket, the bulk fixer needs to decide which one to fix and discard the other. (eg.)

Whether its needed or not, you should have a sample containing the expression that Yaakov provided. We already have tests that assert the fixed code matches the result, both when fixing the failures individually and when bulk fixing them all at the same time.

[brian-reichle]

I used BulkAnalysisRunner to run this new rule across part of our code base and auto-fix any issues, then I looked over the changes made. Some of the changes were incorrect.

• new List<object>() { MainData }.Concat(otherData ?? Enumerable.Empty<object>())

  was changed to

  otherData ?? Enumerable.Empty<object>().Prepend(MainData)

  when it should have been

  (otherData ?? Enumerable.Empty<object>()).Prepend(MainData)

  The solution would be to wrap the argument in a ParenthesizedExpression with the simplifier annotation.

  SyntaxFactory
      .ParenthesizedExpression(argumentExpression.WithoutTrivia())
      .WithTriviaFrom(argumentExpression)
      .WithAdditionalAnnotations(Simplifier.Annotation)

•     entityType
          .GetInterfaces()
          .Concat(new[] { entityType })
          .Distinct()

  was changed to

      entityType
          .GetInterfaces()
  .Append(entityType)
          .Distinct()

  when it should have been

      entityType
          .GetInterfaces()
          .Append(entityType)
          .Distinct()

  It seems that not all the trivia is retained correctly. In particular, the leading trivia on the 'dot' token. I would probably just re-use the existing 'dot' token when creating the new call method.

  InvocationExpression(
      MemberAccessExpression(
          SyntaxKind.SimpleMemberAccessExpression,
          m.Expression,
          m.OperatorToken,
          IdentifierName(nameof(Enumerable.Append)).WithTriviaFrom(m.Name)),
      ArgumentList(SeparatedList(listOfArgumentsAndSeparators)))
      .WithTriviaFrom(invocation)