Skip to content

Commit d2db3e8

Browse files
sharwellsharwell
committed
Fix SA1106 implicit reliance on default line endings
1 parent de3f675 commit d2db3e8

5 files changed

Lines changed: 102 additions & 38 deletions

File tree

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/LayoutRules/SA1501CodeFixProvider.cs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ private static int DetermineIndentationLevel(IndentationSettings indentationSett
133133
}
134134
else
135135
{
136-
parentIndentationLevel = IndentationHelper.GetIndentationSteps(indentationSettings, GetFirstOnLineParent(parent));
136+
parentIndentationLevel = IndentationHelper.GetIndentationSteps(indentationSettings, parent.GetFirstOnLineAncestorOrSelf());
137137
}
138138

139139
return parentIndentationLevel;
@@ -275,18 +275,6 @@ private static SyntaxNode GetStatementParent(SyntaxNode node)
275275
return statementSyntax;
276276
}
277277

278-
private static SyntaxNode GetFirstOnLineParent(SyntaxNode parent)
279-
{
280-
// if the parent is not the first on a line, find the parent that is.
281-
// This mainly happens for 'else if' statements.
282-
while (!parent.GetFirstToken().IsFirstInLine())
283-
{
284-
parent = parent.Parent;
285-
}
286-
287-
return parent;
288-
}
289-
290278
private class FixAll : DocumentBasedFixAllProvider
291279
{
292280
public static FixAllProvider Instance { get; } =

‎StyleCop.Analyzers/StyleCop.Analyzers.CodeFixes/ReadabilityRules/SA1106CodeFixProvider.cs‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,23 @@ private static async Task<Document> RemoveEmptyStatementAsync(Document document,
8181
case SyntaxKind.WhileStatement:
8282
case SyntaxKind.DoStatement:
8383
// these cases are always replaced with an empty block
84-
newRoot = root.ReplaceNode(node, SyntaxFactory.Block().WithTriviaFrom(node));
84+
var text = await document.GetTextAsync(cancellationToken).ConfigureAwait(false);
85+
var options = document.Project.Solution.Workspace.Options;
86+
var endOfLine = FormattingHelper.GetEndOfLineForCodeFix(node.SemicolonToken, text, options);
87+
var settings = SettingsHelper.GetStyleCopSettingsInCodeFix(document.Project.AnalyzerOptions, root.SyntaxTree, cancellationToken);
88+
var indentSteps = IndentationHelper.GetIndentationSteps(settings.Indentation, node.Parent.GetFirstOnLineAncestorOrSelf());
89+
var indentTrivia = IndentationHelper.GenerateWhitespaceTrivia(settings.Indentation, indentSteps);
90+
var block = SyntaxFactory.Block(
91+
SyntaxFactory.Token(
92+
node.GetLeadingTrivia().WithoutTrailingWhitespace().Add(indentTrivia),
93+
SyntaxKind.OpenBraceToken,
94+
SyntaxFactory.TriviaList(endOfLine)),
95+
SyntaxFactory.List<StatementSyntax>(),
96+
SyntaxFactory.Token(
97+
SyntaxFactory.TriviaList(indentTrivia),
98+
SyntaxKind.CloseBraceToken,
99+
node.GetTrailingTrivia()));
100+
newRoot = root.ReplaceNode(node, block);
85101
return document.WithSyntaxRoot(newRoot);
86102

87103
case SyntaxKind.LabeledStatement:

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/Helpers/CommonData.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
2+
// Licensed under the MIT License. See LICENSE in the project root for license information.
3+
4+
namespace StyleCop.Analyzers.Test.Helpers
5+
{
6+
using Xunit;
7+
8+
internal static class CommonData
9+
{
10+
public static TheoryData<string> EndOfLineSequences
11+
{
12+
get
13+
{
14+
var result = new TheoryData<string>();
15+
result.Add("\n");
16+
result.Add("\r\n");
17+
return result;
18+
}
19+
}
20+
}
21+
}

‎StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SA1106UnitTests.cs‎

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
// Copyright (c) Tunnel Vision Laboratories, LLC. All Rights Reserved.
22
// Licensed under the MIT License. See LICENSE in the project root for license information.
33

4-
#nullable disable
5-
64
namespace StyleCop.Analyzers.Test.ReadabilityRules
75
{
6+
using System.Linq;
87
using System.Threading;
98
using System.Threading.Tasks;
109
using Microsoft.CodeAnalysis.Testing;
@@ -16,23 +15,38 @@ namespace StyleCop.Analyzers.Test.ReadabilityRules
1615

1716
public class SA1106UnitTests
1817
{
18+
public static TheoryData<string, string> StatementAsBlock
19+
{
20+
get
21+
{
22+
var result = new TheoryData<string, string>();
23+
foreach (var lineEndingData in CommonData.EndOfLineSequences)
24+
{
25+
var lineEnding = (string)lineEndingData.Single();
26+
result.Add("if (true)", lineEnding);
27+
result.Add("if (true) { } else", lineEnding);
28+
result.Add("for (int i = 0; i < 10; i++)", lineEnding);
29+
result.Add("foreach (int i in new int[] { 0 })", lineEnding);
30+
result.Add("while (true)", lineEnding);
31+
}
32+
33+
return result;
34+
}
35+
}
36+
1937
[Theory]
20-
[InlineData("if (true)")]
21-
[InlineData("if (true) { } else")]
22-
[InlineData("for (int i = 0; i < 10; i++)")]
23-
[InlineData("foreach (int i in new int[] { 0 })")]
24-
[InlineData("while (true)")]
25-
public async Task TestEmptyStatementAsBlockAsync(string controlFlowConstruct)
38+
[MemberData(nameof(StatementAsBlock))]
39+
public async Task TestEmptyStatementAsBlockAsync(string controlFlowConstruct, string lineEnding)
2640
{
2741
var testCode = $@"
2842
class TestClass
2943
{{
3044
public void TestMethod()
3145
{{
3246
{controlFlowConstruct}
33-
;
47+
{{|#0:;|}}
3448
}}
35-
}}";
49+
}}".ReplaceLineEndings(lineEnding);
3650
var fixedCode = $@"
3751
class TestClass
3852
{{
@@ -42,26 +56,28 @@ public void TestMethod()
4256
{{
4357
}}
4458
}}
45-
}}";
59+
}}".ReplaceLineEndings(lineEnding);
4660

47-
DiagnosticResult expected = Diagnostic().WithLocation(7, 13);
61+
DiagnosticResult expected = Diagnostic().WithLocation(0);
4862

4963
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
5064
}
5165

52-
[Fact]
53-
public async Task TestEmptyStatementAsBlockInDoWhileAsync()
66+
[Theory]
67+
[InlineData("\n")]
68+
[InlineData("\r\n")]
69+
public async Task TestEmptyStatementAsBlockInDoWhileAsync(string lineEnding)
5470
{
5571
var testCode = @"
5672
class TestClass
5773
{
5874
public void TestMethod()
5975
{
6076
do
61-
;
77+
{|#0:;|}
6278
while (false);
6379
}
64-
}";
80+
}".ReplaceLineEndings(lineEnding);
6581
var fixedCode = @"
6682
class TestClass
6783
{
@@ -72,9 +88,9 @@ public void TestMethod()
7288
}
7389
while (false);
7490
}
75-
}";
91+
}".ReplaceLineEndings(lineEnding);
7692

77-
DiagnosticResult expected = Diagnostic().WithLocation(7, 13);
93+
DiagnosticResult expected = Diagnostic().WithLocation(0);
7894

7995
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
8096
}
@@ -128,26 +144,28 @@ public void TestMethod()
128144
await VerifyCSharpDiagnosticAsync(testCode, DiagnosticResult.EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
129145
}
130146

131-
[Fact]
132-
public async Task TestEmptyStatementAsync()
147+
[Theory]
148+
[InlineData("\n")]
149+
[InlineData("\r\n")]
150+
public async Task TestEmptyStatementAsync(string lineEnding)
133151
{
134152
var testCode = @"
135153
class TestClass
136154
{
137155
public void TestMethod()
138156
{
139-
;
157+
{|#0:;|}
140158
}
141-
}";
159+
}".ReplaceLineEndings(lineEnding);
142160
var fixedCode = @"
143161
class TestClass
144162
{
145163
public void TestMethod()
146164
{
147165
}
148-
}";
166+
}".ReplaceLineEndings(lineEnding);
149167

150-
DiagnosticResult expected = Diagnostic().WithLocation(6, 9);
168+
DiagnosticResult expected = Diagnostic().WithLocation(0);
151169

152170
await VerifyCSharpFixAsync(testCode, expected, fixedCode, CancellationToken.None).ConfigureAwait(false);
153171
}

‎StyleCop.Analyzers/StyleCop.Analyzers/Helpers/SyntaxNodeExtensions.cs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,5 +86,26 @@ public static bool IsAnyLambda(this SyntaxNode? node)
8686
return node.IsKind(SyntaxKind.ParenthesizedLambdaExpression)
8787
|| node.IsKind(SyntaxKind.SimpleLambdaExpression);
8888
}
89+
90+
/// <summary>
91+
/// Finds the nearest ancestor of the specified syntax node that is the first token on its line.
92+
/// </summary>
93+
/// <remarks><para>This method is useful for scenarios such as handling 'else if' statements, where the
94+
/// initial node may not be the first on its line. The search ascends the syntax tree until a suitable ancestor
95+
/// is found.</para></remarks>
96+
/// <param name="parent">The syntax node from which to begin searching for an ancestor that starts a line.</param>
97+
/// <returns>A syntax node that is the first token on its line. If the specified node is already the first on its line,
98+
/// returns the node itself.</returns>
99+
public static SyntaxNode GetFirstOnLineAncestorOrSelf(this SyntaxNode parent)
100+
{
101+
// if the parent is not the first on a line, find the parent that is.
102+
// This mainly happens for 'else if' statements.
103+
while (!parent.GetFirstToken().IsFirstInLine())
104+
{
105+
parent = parent.Parent;
106+
}
107+
108+
return parent;
109+
}
89110
}
90111
}

0 commit comments

Comments
 (0)