Fix RCS1077 formatting when converting Select().ToList() to ConvertAll() - #1742
Fix RCS1077 formatting when converting Select().ToList() to ConvertAll()#1742MattFromRVA wants to merge 8 commits into
Conversation
josefpihrt
left a comment
There was a problem hiding this comment.
Nice fix — the root cause (the new node inheriting the Select invocation's trailing trivia, which carried the newline before .ToList()) is correctly addressed by anchoring to toListInvocation.GetTrailingTrivia(), and it stays backward-compatible with the single-line case. A couple of inline comments below; one I'd suggest resolving before merge.
Test coverage: could we add a case for a single-line // comment in the removed segment (see the inline comment on the if), and optionally one with an IReadOnlyList<...> target to mirror the exact repro in #1728? The current tests only cover /* */, which is what lets the // case slip through.
| TextSpan.FromBounds(selectInvocation.Span.End, toListInvocation.Span.End)); | ||
|
|
||
| if (removedTrivia.Any(f => !f.IsWhitespaceOrEndOfLineTrivia())) | ||
| { |
There was a problem hiding this comment.
This branch strips all whitespace/EOL trivia and reattaches surviving comments inline. That's fine for block comments, but a single-line // comment has no terminating newline of its own — the newline is separate EndOfLineTrivia, which gets stripped here. So this input:
var x = items
.Select(f => f.ToString()) // keep this
.ToList();would become:
var x = items
.ConvertAll(f => f.ToString())// keep this;…where the ; is now commented out → compile error. Could we preserve a trailing newline when the salvaged trivia ends in a single-line comment (or otherwise guard against it)? A //-comment variant of Test_CallConvertAllInsteadOfSelectAndToList_PreservesCommentInRemovedSegment would reproduce it.
| removedTrivia.Where(f => !f.IsWhitespaceOrEndOfLineTrivia()).Concat(toListInvocation.GetTrailingTrivia())); | ||
| } | ||
| else | ||
| { |
There was a problem hiding this comment.
Nit: both branches converge on ... .Concat(toListInvocation.GetTrailingTrivia()) — when there are no comments the Where yields nothing, so this collapses to a single path:
IEnumerable<SyntaxTrivia> comments = removedTrivia.Where(f => !f.IsWhitespaceOrEndOfLineTrivia());
newInvocationExpression = newInvocationExpression.WithTrailingTrivia(
comments.Concat(toListInvocation.GetTrailingTrivia()));Behavior-equivalent and drops the branch. Also worth materializing removedTrivia with .ToList() since it's currently enumerated multiple times.
Fixes #1728