Skip to content

Commit d95082a

Browse files
committed
Update indentation code fix provider to adjust an entire element
This change allows for for accurate indentation adjustments of single code elements which span multiple lines. Since the fix all provider has not been updated to account for the new behavior, it is disabled for now.
1 parent 2bd4822 commit d95082a

File tree

5 files changed

+337
-54
lines changed

5 files changed

+337
-54
lines changed

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/Helpers/IndentationHelper.cs

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

44
namespace StyleCop.Analyzers.Helpers
55
{
6+
using System.Text;
67
using Microsoft.CodeAnalysis;
78
using Microsoft.CodeAnalysis.CSharp;
89
using Settings.ObjectModel;
@@ -83,6 +84,42 @@ public static string GenerateIndentationStringForSteps(IndentationSettings inden
8384
return result;
8485
}
8586

87+
/// <summary>
88+
/// Generate a new indentation string for the given indentation width.
89+
/// </summary>
90+
/// <param name="indentationSettings">The indentation settings to use.</param>
91+
/// <param name="indentationWidth">The width of the indentation string.</param>
92+
/// <returns>A string containing the whitespace needed to indent code to the specified width.</returns>
93+
public static string GenerateIndentationString(IndentationSettings indentationSettings, int indentationWidth) =>
94+
GenerateIndentationString(indentationSettings, indentationWidth, 0);
95+
96+
/// <summary>
97+
/// Generate a new indentation string for the given indentation width.
98+
/// </summary>
99+
/// <param name="indentationSettings">The indentation settings to use.</param>
100+
/// <param name="indentationWidth">The width of the indentation string.</param>
101+
/// <param name="startColumn">The starting column for the indentation.</param>
102+
/// <returns>A string containing the whitespace needed to indent code to the specified width.</returns>
103+
public static string GenerateIndentationString(IndentationSettings indentationSettings, int indentationWidth, int startColumn)
104+
{
105+
if (!indentationSettings.UseTabs)
106+
{
107+
return new string(' ', indentationWidth);
108+
}
109+
110+
// Adjust the indentation width so a narrower first tab doesn't affect the outcome
111+
indentationWidth += startColumn % indentationSettings.TabSize;
112+
113+
int tabCount = indentationWidth / indentationSettings.TabSize;
114+
int spaceCount = indentationWidth - (tabCount * indentationSettings.TabSize);
115+
116+
StringBuilder builder = StringBuilderPool.Allocate();
117+
builder.EnsureCapacity(tabCount + spaceCount);
118+
builder.Append('\t', tabCount);
119+
builder.Append(' ', spaceCount);
120+
return StringBuilderPool.ReturnAndFree(builder);
121+
}
122+
86123
/// <summary>
87124
/// Generates a whitespace trivia with the requested indentation.
88125
/// </summary>

StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/IndentationCodeFixProvider.cs

Lines changed: 147 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ namespace StyleCop.Analyzers.ReadabilityRules
1212
using Microsoft.CodeAnalysis;
1313
using Microsoft.CodeAnalysis.CodeActions;
1414
using Microsoft.CodeAnalysis.CodeFixes;
15+
using Microsoft.CodeAnalysis.CSharp;
1516
using Microsoft.CodeAnalysis.Text;
17+
using Settings.ObjectModel;
1618

1719
[ExportCodeFixProvider(LanguageNames.CSharp, Name = nameof(IndentationCodeFixProvider))]
1820
[Shared]
@@ -26,7 +28,7 @@ internal class IndentationCodeFixProvider : CodeFixProvider
2628

2729
/// <inheritdoc/>
2830
public sealed override FixAllProvider GetFixAllProvider() =>
29-
FixAll.Instance;
31+
null;
3032

3133
/// <inheritdoc/>
3234
public override Task RegisterCodeFixesAsync(CodeFixContext context)
@@ -48,26 +50,28 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
4850
{
4951
var syntaxRoot = await document.GetSyntaxRootAsync().ConfigureAwait(false);
5052

51-
TextChange textChange;
52-
if (!TryGetTextChange(diagnostic, syntaxRoot, out textChange))
53+
StyleCopSettings settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, cancellationToken);
54+
ImmutableArray<TextChange> textChanges = await GetTextChangesAsync(diagnostic, syntaxRoot, settings.Indentation, cancellationToken).ConfigureAwait(false);
55+
if (textChanges.IsEmpty)
5356
{
5457
return document;
5558
}
5659

5760
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
58-
return document.WithText(text.WithChanges(textChange));
61+
return document.WithText(text.WithChanges(textChanges));
5962
}
6063

61-
private static bool TryGetTextChange(Diagnostic diagnostic, SyntaxNode syntaxRoot, out TextChange textChange)
64+
private static async Task<ImmutableArray<TextChange>> GetTextChangesAsync(Diagnostic diagnostic, SyntaxNode syntaxRoot, IndentationSettings indentationSettings, CancellationToken cancellationToken)
6265
{
6366
string replacement;
6467
if (!diagnostic.Properties.TryGetValue(SA1137ElementsShouldHaveTheSameIndentation.ExpectedIndentationKey, out replacement))
6568
{
66-
textChange = default(TextChange);
67-
return false;
69+
return ImmutableArray<TextChange>.Empty;
6870
}
6971

70-
var trivia = syntaxRoot.FindTrivia(diagnostic.Location.SourceSpan.Start);
72+
SyntaxTrivia trivia = syntaxRoot.FindTrivia(diagnostic.Location.SourceSpan.Start);
73+
SyntaxToken token = trivia != default(SyntaxTrivia) ? trivia.Token : syntaxRoot.FindToken(diagnostic.Location.SourceSpan.Start, findInsideTrivia: true);
74+
SyntaxNode node = GetNodeForAdjustment(token);
7175

7276
TextSpan originalSpan;
7377
if (trivia == default(SyntaxTrivia))
@@ -80,8 +84,139 @@ private static bool TryGetTextChange(Diagnostic diagnostic, SyntaxNode syntaxRoo
8084
originalSpan = trivia.Span;
8185
}
8286

83-
textChange = new TextChange(originalSpan, replacement);
84-
return true;
87+
FileLinePositionSpan fullSpan = syntaxRoot.SyntaxTree.GetLineSpan(node.FullSpan, cancellationToken);
88+
if (fullSpan.StartLinePosition.Line == fullSpan.EndLinePosition.Line)
89+
{
90+
return ImmutableArray.Create(new TextChange(originalSpan, replacement));
91+
}
92+
93+
SyntaxTree tree = node.SyntaxTree;
94+
SourceText sourceText = await tree.GetTextAsync(cancellationToken).ConfigureAwait(false);
95+
96+
int originalIndentation = GetIndentationWidth(indentationSettings, sourceText.ToString(originalSpan));
97+
int newIndentation = GetIndentationWidth(indentationSettings, replacement);
98+
99+
ImmutableArray<TextSpan> excludedSpans = SyntaxTreeHelpers.GetExcludedSpans(node);
100+
TextLineCollection lines = sourceText.Lines;
101+
102+
// For each line in the full span of the syntax node:
103+
// 1. If the line is indented less than originalIndentation, ignore the line
104+
// 2. If the indentation characters are not located within the full span, ignore the line
105+
// 2. If the indentation characters of the line overlap with an excluded span, ignore the line
106+
// 3. Replace the first original.Length characters on the line with replacement
107+
ImmutableArray<TextChange>.Builder builder = ImmutableArray.CreateBuilder<TextChange>();
108+
for (int i = fullSpan.StartLinePosition.Line; i <= fullSpan.EndLinePosition.Line; i++)
109+
{
110+
TextLine line = lines[i];
111+
string lineText = sourceText.ToString(line.Span);
112+
113+
int indentationCount;
114+
int indentationWidth = GetIndentationWidth(indentationSettings, lineText, out indentationCount);
115+
if (indentationWidth < originalIndentation)
116+
{
117+
continue;
118+
}
119+
120+
if (indentationCount == line.Span.Length)
121+
{
122+
// The line is just whitespace
123+
continue;
124+
}
125+
126+
TextSpan indentationSpan = new TextSpan(line.Start, indentationCount);
127+
if (indentationSpan.Start >= node.FullSpan.End)
128+
{
129+
// The line does not contain any non-whitespace content which is part of the full span of the node
130+
continue;
131+
}
132+
133+
if (!node.FullSpan.Contains(indentationSpan))
134+
{
135+
// The indentation of the line is not part of the full span of the node
136+
continue;
137+
}
138+
139+
if (IsExcluded(excludedSpans, indentationSpan))
140+
{
141+
// The line indentation is partially- or fully-excluded from adjustments
142+
continue;
143+
}
144+
145+
if (originalIndentation == indentationWidth)
146+
{
147+
builder.Add(new TextChange(indentationSpan, replacement));
148+
}
149+
else if (newIndentation > originalIndentation)
150+
{
151+
// TODO: This needs to handle UseTabs setting
152+
builder.Add(new TextChange(new TextSpan(indentationSpan.End, 0), new string(' ', newIndentation - originalIndentation)));
153+
}
154+
else if (newIndentation < originalIndentation)
155+
{
156+
builder.Add(new TextChange(indentationSpan, IndentationHelper.GenerateIndentationString(indentationSettings, indentationWidth + (newIndentation - originalIndentation))));
157+
}
158+
}
159+
160+
return builder.ToImmutable();
161+
}
162+
163+
private static SyntaxNode GetNodeForAdjustment(SyntaxToken token)
164+
{
165+
return token.Parent;
166+
}
167+
168+
private static int GetIndentationWidth(IndentationSettings indentationSettings, string text)
169+
{
170+
int ignored;
171+
return GetIndentationWidth(indentationSettings, text, out ignored);
172+
}
173+
174+
private static int GetIndentationWidth(IndentationSettings indentationSettings, string text, out int count)
175+
{
176+
int tabSize = indentationSettings.TabSize;
177+
int indentationWidth = 0;
178+
for (int i = 0; i < text.Length; i++)
179+
{
180+
switch (text[i])
181+
{
182+
case ' ':
183+
indentationWidth++;
184+
break;
185+
186+
case '\t':
187+
indentationWidth = tabSize * ((indentationWidth / tabSize) + 1);
188+
break;
189+
190+
default:
191+
count = i;
192+
return indentationWidth;
193+
}
194+
}
195+
196+
count = text.Length;
197+
return indentationWidth;
198+
}
199+
200+
private static bool IsExcluded(ImmutableArray<TextSpan> excludedSpans, TextSpan textSpan)
201+
{
202+
int index = excludedSpans.BinarySearch(textSpan);
203+
if (index > 0)
204+
{
205+
return true;
206+
}
207+
208+
int nextLarger = ~index;
209+
if (nextLarger > 0 && excludedSpans[nextLarger - 1].OverlapsWith(textSpan))
210+
{
211+
return true;
212+
}
213+
214+
if (nextLarger < excludedSpans.Length - 1 && excludedSpans[nextLarger].OverlapsWith(textSpan))
215+
{
216+
return true;
217+
}
218+
219+
return false;
85220
}
86221

87222
private class FixAll : DocumentBasedFixAllProvider
@@ -100,16 +235,13 @@ protected override async Task<SyntaxNode> FixAllInDocumentAsync(FixAllContext fi
100235
}
101236

102237
var syntaxRoot = await document.GetSyntaxRootAsync().ConfigureAwait(false);
238+
StyleCopSettings settings = SettingsHelper.GetStyleCopSettings(document.Project.AnalyzerOptions, fixAllContext.CancellationToken);
103239

104240
List<TextChange> changes = new List<TextChange>();
105241

106242
foreach (var diagnostic in diagnostics)
107243
{
108-
TextChange textChange;
109-
if (TryGetTextChange(diagnostic, syntaxRoot, out textChange))
110-
{
111-
changes.Add(textChange);
112-
}
244+
changes.AddRange(await GetTextChangesAsync(diagnostic, syntaxRoot, settings.Indentation, fixAllContext.CancellationToken).ConfigureAwait(false));
113245
}
114246

115247
changes.Sort((left, right) => left.Span.Start.CompareTo(right.Span.Start));

StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1137UnitTests.cs

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -331,37 +331,37 @@ class MyAttribute : Attribute { }
331331
using System;
332332
enum Enum1
333333
{
334-
/// <summary>
335-
/// Summary.
336-
/// </summary>
334+
/// <summary>
335+
/// Summary.
336+
/// </summary>
337337
[My]
338338
Element1,
339339
340-
/// <summary>
341-
/// Summary.
342-
/// </summary>
340+
/// <summary>
341+
/// Summary.
342+
/// </summary>
343343
Element2,
344344
}
345345
346346
enum Enum2
347347
{
348-
/// <summary>
349-
/// Summary.
350-
/// </summary>
348+
/// <summary>
349+
/// Summary.
350+
/// </summary>
351351
[My]
352352
Element1,
353353
354-
/// <summary>
355-
/// Summary.
356-
/// </summary>
354+
/// <summary>
355+
/// Summary.
356+
/// </summary>
357357
Element2,
358358
}
359359
360360
enum Enum3
361361
{
362-
/// <summary>
363-
/// Summary.
364-
/// </summary>
362+
/// <summary>
363+
/// Summary.
364+
/// </summary>
365365
[My] Element1,
366366
367367
/// <summary>
@@ -1396,13 +1396,13 @@ void MethodName()
13961396
label3:
13971397
while (true)
13981398
{
1399-
label4a:
1400-
label4b:
1401-
int x;
1399+
label4a:
1400+
label4b:
1401+
int x;
14021402
1403-
label5a:
1404-
label5b:
1405-
int y;
1403+
label5a:
1404+
label5b:
1405+
int y;
14061406
}
14071407
}
14081408
}

0 commit comments

Comments
 (0)