Skip to content

Commit 64016d7

Browse files
sharwellsharwell
authored
Merge pull request #2074 from vweijsters/fix-1987
Improved FixAll for TokenSpacingCodeFixProvider
2 parents 65fbf93 + ae74416 commit 64016d7

File tree

8 files changed

+61
-44
lines changed

8 files changed

+61
-44
lines changed

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/SpacingRules/TokenSpacingCodeFixProvider.cs‎

Lines changed: 33 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
namespace StyleCop.Analyzers.SpacingRules
55
{
6+
using System;
67
using System.Collections.Generic;
78
using System.Collections.Immutable;
89
using System.Composition;
@@ -123,7 +124,7 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
123124
case TokenSpacingProperties.ActionInsert:
124125
if (!replaceMap.ContainsKey(prevToken))
125126
{
126-
replaceMap[token] = token.WithLeadingTrivia(token.LeadingTrivia.Add(SyntaxFactory.Space));
127+
UpdateReplaceMap(replaceMap, token, t => t.WithLeadingTrivia(t.LeadingTrivia.Add(SyntaxFactory.Space)));
127128
}
128129

129130
break;
@@ -137,11 +138,12 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
137138
break;
138139
}
139140

140-
replaceMap[prevToken] = prevToken.WithTrailingTrivia();
141+
UpdateReplaceMap(replaceMap, prevToken, t => t.WithTrailingTrivia());
142+
141143
if ((!preserveLayout || !tokenIsFirstInLine)
142144
&& triviaList.All(i => i.IsKind(SyntaxKind.WhitespaceTrivia) || i.IsKind(SyntaxKind.EndOfLineTrivia)))
143145
{
144-
replaceMap[token] = token.WithLeadingTrivia();
146+
UpdateReplaceMap(replaceMap, token, t => t.WithLeadingTrivia());
145147
}
146148
else if (tokenIsFirstInLine && token.IsLastInLine())
147149
{
@@ -177,13 +179,13 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
177179
// firstNewLineFollowing was adjusted above to account for the missing case.
178180
trailingTrivia = trailingTrivia.AddRange(token.TrailingTrivia.Take(firstNewLineFollowing));
179181

180-
replaceMap[token] = token.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia);
182+
UpdateReplaceMap(replaceMap, token, t => t.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia));
181183
}
182184
else
183185
{
184186
// Just move the token and keep all surrounding trivia.
185187
SyntaxTriviaList trailingTrivia = triviaList.AddRange(token.TrailingTrivia);
186-
replaceMap[token] = token.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia);
188+
UpdateReplaceMap(replaceMap, token, t => t.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia));
187189
}
188190
}
189191
else
@@ -194,35 +196,23 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
194196
if (nextToken.IsKind(SyntaxKind.SemicolonToken))
195197
{
196198
// make the semicolon 'sticky'
197-
replaceMap[token] = token.WithLeadingTrivia().WithTrailingTrivia();
198-
replaceMap[nextToken] = nextToken.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia.WithoutTrailingWhitespace());
199+
UpdateReplaceMap(replaceMap, token, t => t.WithLeadingTrivia().WithTrailingTrivia());
200+
UpdateReplaceMap(replaceMap, nextToken, t => t.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia.WithoutTrailingWhitespace()));
199201
}
200202
else
201203
{
202-
replaceMap[token] = token.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia);
204+
UpdateReplaceMap(replaceMap, token, t => t.WithLeadingTrivia().WithTrailingTrivia(trailingTrivia));
203205
}
204206
}
205207

206208
break;
207209

208210
case TokenSpacingProperties.ActionRemoveImmediate:
209-
SyntaxTriviaList tokenLeadingTrivia = token.LeadingTrivia;
210-
while (tokenLeadingTrivia.Any() && tokenLeadingTrivia.Last().IsKind(SyntaxKind.WhitespaceTrivia))
211-
{
212-
tokenLeadingTrivia = tokenLeadingTrivia.RemoveAt(tokenLeadingTrivia.Count - 1);
213-
}
214-
215-
replaceMap[token] = token.WithLeadingTrivia(tokenLeadingTrivia);
211+
UpdateReplaceMap(replaceMap, token, t => t.WithLeadingTrivia(t.LeadingTrivia.WithoutTrailingWhitespace(endOfLineIsWhitespace: false)));
216212

217-
if (!tokenLeadingTrivia.Any())
213+
if (!replaceMap[token].LeadingTrivia.Any())
218214
{
219-
SyntaxTriviaList previousTrailingTrivia = prevToken.TrailingTrivia;
220-
while (previousTrailingTrivia.Any() && previousTrailingTrivia.Last().IsKind(SyntaxKind.WhitespaceTrivia))
221-
{
222-
previousTrailingTrivia = previousTrailingTrivia.RemoveAt(previousTrailingTrivia.Count - 1);
223-
}
224-
225-
replaceMap[prevToken] = prevToken.WithTrailingTrivia(previousTrailingTrivia);
215+
UpdateReplaceMap(replaceMap, prevToken, t => t.WithTrailingTrivia(t.TrailingTrivia.WithoutTrailingWhitespace(endOfLineIsWhitespace: false)));
226216
}
227217

228218
break;
@@ -241,7 +231,7 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
241231
// processed during an earlier step in the fix all process, so no additional processing is needed.
242232
if (!replaceMap.ContainsKey(token) || (replaceMap[token].TrailingTrivia.Count == 0))
243233
{
244-
replaceMap[token] = token.WithTrailingTrivia(token.TrailingTrivia.Insert(0, SyntaxFactory.Space));
234+
UpdateReplaceMap(replaceMap, token, t => t.WithTrailingTrivia(t.TrailingTrivia.Insert(0, SyntaxFactory.Space)));
245235
}
246236
}
247237

@@ -250,15 +240,32 @@ private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replac
250240
case TokenSpacingProperties.ActionRemove:
251241
triviaList = token.TrailingTrivia.AddRange(nextToken.LeadingTrivia);
252242

253-
replaceMap[token] = token.WithTrailingTrivia();
254-
replaceMap[nextToken] = nextToken.WithLeadingTrivia(triviaList.WithoutLeadingWhitespace(true));
243+
UpdateReplaceMap(replaceMap, token, t => t.WithTrailingTrivia());
244+
UpdateReplaceMap(replaceMap, nextToken, t => t.WithLeadingTrivia(triviaList.WithoutLeadingWhitespace(true)));
255245
break;
256246
}
257247

258248
break;
259249
}
260250
}
261251

252+
private static void UpdateReplaceMap(Dictionary<SyntaxToken, SyntaxToken> replaceMap, SyntaxToken token, Func<SyntaxToken, SyntaxToken> action)
253+
{
254+
SyntaxToken existingReplacement;
255+
SyntaxToken newReplacement;
256+
257+
if (replaceMap.TryGetValue(token, out existingReplacement))
258+
{
259+
newReplacement = action(existingReplacement);
260+
}
261+
else
262+
{
263+
newReplacement = action(token);
264+
}
265+
266+
replaceMap[token] = newReplacement;
267+
}
268+
262269
private class FixAll : DocumentBasedFixAllProvider
263270
{
264271
public static FixAllProvider Instance { get; } = new FixAll();

‎StyleCop.Analyzers/StyleCop.Analyzers.Test.CSharp7/SpacingRules/SA1008CSharp7UnitTests.cs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public class TestClass
133133

134134
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
135135
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
136-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
136+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
137137
}
138138

139139
[Fact]
@@ -195,7 +195,7 @@ public class TestClass
195195

196196
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
197197
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
198-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
198+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
199199
}
200200

201201
[Fact]
@@ -260,7 +260,7 @@ public void TestMethod4()
260260

261261
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
262262
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
263-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
263+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
264264
}
265265

266266
/// <summary>
@@ -397,7 +397,7 @@ public class TestClass
397397

398398
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
399399
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
400-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
400+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
401401
}
402402

403403
[Fact]
@@ -441,7 +441,7 @@ public class TestClass
441441

442442
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostic, CancellationToken.None).ConfigureAwait(false);
443443
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
444-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
444+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
445445
}
446446

447447
/// <summary>
@@ -717,7 +717,7 @@ public void TestMethod()
717717

718718
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostics, CancellationToken.None).ConfigureAwait(false);
719719
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
720-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
720+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
721721
}
722722

723723
/// <summary>
@@ -775,7 +775,7 @@ public class TestClass
775775

776776
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostics, CancellationToken.None).ConfigureAwait(false);
777777
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
778-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
778+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
779779
}
780780

781781
/// <summary>
@@ -823,7 +823,7 @@ public void TestMethod()
823823

824824
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostics, CancellationToken.None).ConfigureAwait(false);
825825
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
826-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
826+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
827827
}
828828

829829
/// <summary>
@@ -871,7 +871,7 @@ public void TestMethod()
871871

872872
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostics, CancellationToken.None).ConfigureAwait(false);
873873
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
874-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2).ConfigureAwait(false);
874+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
875875
}
876876

877877
/// <summary>

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/SpacingRules/NumberSignSpacingTestBase.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ void Foo()
194194
};
195195

196196
await this.VerifyCSharpDiagnosticAsync(test, expected, CancellationToken.None).ConfigureAwait(false);
197-
await this.VerifyCSharpFixAsync(test, fixedTest, numberOfFixAllIterations: 2, cancellationToken: CancellationToken.None).ConfigureAwait(false);
197+
await this.VerifyCSharpFixAsync(test, fixedTest, cancellationToken: CancellationToken.None).ConfigureAwait(false);
198198
}
199199

200200
[Fact]

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/SpacingRules/SA1008UnitTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1454,7 +1454,7 @@ public void TestMethod(int x, int y)
14541454

14551455
await this.VerifyCSharpDiagnosticAsync(testCode, expectedDiagnostics, CancellationToken.None).ConfigureAwait(false);
14561456
await this.VerifyCSharpDiagnosticAsync(fixedTestCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
1457-
await this.VerifyCSharpFixAsync(testCode, fixedTestCode, numberOfFixAllIterations: 2, cancellationToken: CancellationToken.None).ConfigureAwait(false);
1457+
await this.VerifyCSharpFixAsync(testCode, fixedTestCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
14581458
}
14591459

14601460
/// <summary>

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/SpacingRules/SA1012UnitTests.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public class TestClass
158158

159159
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
160160
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
161-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2, cancellationToken: CancellationToken.None).ConfigureAwait(false);
161+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
162162
}
163163

164164
/// <summary>
@@ -218,7 +218,7 @@ public void TestMethod()
218218

219219
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
220220
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
221-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2, cancellationToken: CancellationToken.None).ConfigureAwait(false);
221+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
222222
}
223223

224224
[Fact]

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/SpacingRules/SA1013UnitTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ public void TestMethod()
186186

187187
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
188188
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
189-
await this.VerifyCSharpFixAsync(testCode, fixedCode, numberOfFixAllIterations: 2, cancellationToken: CancellationToken.None).ConfigureAwait(false);
189+
await this.VerifyCSharpFixAsync(testCode, fixedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
190190
}
191191

192192
/// <summary>

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/SpacingRules/SA1024UnitTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ private int Bar(int value)
329329

330330
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
331331
await this.VerifyCSharpDiagnosticAsync(ExpectedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
332-
await this.VerifyCSharpFixAsync(testCode, ExpectedCode, numberOfFixAllIterations: 2, cancellationToken: CancellationToken.None).ConfigureAwait(false);
332+
await this.VerifyCSharpFixAsync(testCode, ExpectedCode, cancellationToken: CancellationToken.None).ConfigureAwait(false);
333333
}
334334

335335
/// <inheritdoc/>

‎StyleCop.Analyzers/StyleCop.Analyzers/Helpers/TriviaHelper.cs‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,10 @@ internal static int IndexOfFirstNonBlankLineTrivia<T>(T triviaList)
8080
/// </summary>
8181
/// <param name="triviaList">The trivia list to process.</param>
8282
/// <typeparam name="T">The type of the trivia list.</typeparam>
83+
/// <param name="endOfLineIsWhitespace"><see langword="true"/> to treat <see cref="SyntaxKind.EndOfLineTrivia"/>
84+
/// as whitespace; otherwise, <see langword="false"/>.</param>
8385
/// <returns>The index where the trailing whitespace starts, or -1 if there is no trailing whitespace.</returns>
84-
internal static int IndexOfTrailingWhitespace<T>(T triviaList)
86+
internal static int IndexOfTrailingWhitespace<T>(T triviaList, bool endOfLineIsWhitespace = true)
8587
where T : IReadOnlyList<SyntaxTrivia>
8688
{
8789
var done = false;
@@ -94,6 +96,12 @@ internal static int IndexOfTrailingWhitespace<T>(T triviaList)
9496
switch (currentTrivia.Kind())
9597
{
9698
case SyntaxKind.EndOfLineTrivia:
99+
if (!endOfLineIsWhitespace)
100+
{
101+
done = true;
102+
break;
103+
}
104+
97105
whiteSpaceStartIndex = index;
98106
previousTriviaWasEndOfLine = true;
99107
break;
@@ -206,10 +214,12 @@ internal static int LastIndexOf(this SyntaxTriviaList list, SyntaxKind kind)
206214
/// Strips all trailing whitespace trivia from the trivia list until a non-whitespace trivia is encountered.
207215
/// </summary>
208216
/// <param name="triviaList">The trivia list to strip of its trailing whitespace.</param>
217+
/// <param name="endOfLineIsWhitespace"><see langword="true"/> to treat <see cref="SyntaxKind.EndOfLineTrivia"/>
218+
/// as whitespace; otherwise, <see langword="false"/>.</param>
209219
/// <returns>The modified triviaList.</returns>
210-
internal static SyntaxTriviaList WithoutTrailingWhitespace(this SyntaxTriviaList triviaList)
220+
internal static SyntaxTriviaList WithoutTrailingWhitespace(this SyntaxTriviaList triviaList, bool endOfLineIsWhitespace = true)
211221
{
212-
var trailingWhitespaceIndex = IndexOfTrailingWhitespace(triviaList);
222+
var trailingWhitespaceIndex = IndexOfTrailingWhitespace(triviaList, endOfLineIsWhitespace);
213223
return (trailingWhitespaceIndex >= 0) ? SyntaxFactory.TriviaList(triviaList.Take(trailingWhitespaceIndex)) : triviaList;
214224
}
215225

0 commit comments

Comments
 (0)