IDE0304 should preserve the leading comment

[Bartleby2718]

Version Used:

.NET SDK 8.0.205

Steps to Reproduce:

1. Add the example code from https://learn.microsoft.com/en-us/dotnet/fundamentals/code-analysis/style-rules/ide0304#example to a .cs file

// Code with violation.
var builder = ImmutableArray.CreateBuilder<int>();
builder.Add(1);
builder.AddRange(new int[] { 5, 6, 7 });
ImmutableArray<int> i = builder.ToImmutable();

2. Use the suggested codefix for IDE0304

sharplab repro: https://sharplab.io/#gist:9c306bb8a2c25fa09562b016a4d5a550

Diagnostic Id: IDE0304 (Use collection expression for builder)

Expected Behavior: The code should be updated to

// Code with violation.
ImmutableArray<int> i = [1, .. new int[] { 5, 6, 7 }];

Actual Behavior: The code is updated to

ImmutableArray<int> i = [1, .. new int[] { 5, 6, 7 }];

Note the absence of the comment // Code with violation.

[DannySliekers]

Hey, can i pick this up if possible?

[CyrusNajmabadi]

@DannySliekers Absolutely!

[DannySliekers]

So I did some investigating and found that the issue seems to lie at the removal of the LocalDeclarationStatement, as none of the leading and trailing trivia gets preserved:

roslyn/src/Analyzers/CSharp/CodeFixes/UseCollectionExpression/CSharpUseCollectionExpressionForBuilderCodeFixProvider.cs

Lines 78 to 79 in 2b1fb9b

	// Remove the actual declaration of the builder.
	subEditor.RemoveNode(analysisResult.LocalDeclarationStatement);

For the case of leading trivia i assume we want to include all leading trivia in the fixed code, right?
Example with trivia on same line:

// Code with violation.
/* test */ var builder = ImmutableArray.CreateBuilder<int>();
builder.Add(1);
builder.AddRange(new int[] { 5, 6, 7 });
ImmutableArray<int> i = builder.ToImmutable();

// Desired code
/* test */ ImmutableArray<int> i = [1, .. new int[] { 5, 6, 7 }];

For the case in which the declaration of the builder contains trailing trivia seems to be a little bit more complicated.
This seems like a desired result to me:

// Code with violation.
var builder = ImmutableArray.CreateBuilder<int>(); // test
builder.Add(1);
builder.AddRange(new int[] { 5, 6, 7 });
ImmutableArray<int> i = builder.ToImmutable();

//Desired code
ImmutableArray<int> i = [1, .. new int[] { 5, 6, 7 }]; // test

If the fix in the code above is desired, there is a case where there would be multiple options of preserving trivia:

var builder = ImmutableArray.CreateBuilder<int>(); // test
builder.Add(1);
builder.AddRange(new int[] { 5, 6, 7 });
ImmutableArray<int> i = builder.ToImmutable(); // test2

//Desired code #1? 
ImmutableArray<int> i = [1, .. new int[] { 5, 6, 7 }]; // test2 // test

//Desired code #2? 
ImmutableArray<int> i = [1, .. new int[] { 5, 6, 7 }]; // test // test2

Seems to me like there are multiple options in the case of trailing trivia at the declaration of the builder, and I would like to know what would be desirable.

[CyrusNajmabadi]

For the case of leading trivia i assume we want to include all leading trivia in the fixed code, right?

We try to handle reasonable code patterns, and we don't expend effort on uncommon ones.

So, for example, this is not common: /* test */ var builder = ImmutableArray.CreateBuilder<int>();.

[CyrusNajmabadi]

@DannySliekers i would just work on a fix that resolves exactly the case reported.