Skip to content

Commit 96dce3e

Browse files
sharwellsharwell
committed
Merge pull request #1559 from Maxwe11/SA1132
2 parents e3970b8 + a07e6e3 commit 96dce3e

4 files changed

Lines changed: 152 additions & 38 deletions

File tree

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

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ namespace StyleCop.Analyzers.Test.ReadabilityRules
77
using System.Threading;
88
using System.Threading.Tasks;
99
using Analyzers.ReadabilityRules;
10+
using Microsoft.CodeAnalysis;
1011
using Microsoft.CodeAnalysis.CodeFixes;
1112
using Microsoft.CodeAnalysis.Diagnostics;
1213
using TestHelper;
@@ -34,14 +35,16 @@ public async Task TestInvalidDeclarationAsync()
3435
const string testCode = @"
3536
class Foo
3637
{
37-
private int a, b;
38+
private int a, b,/*foo*/c,d;
3839
public event System.Action aa, bb;
3940
}";
4041
const string fixedCode = @"
4142
class Foo
4243
{
4344
private int a;
44-
private int b;
45+
private int b;/*foo*/
46+
private int c;
47+
private int d;
4548
public event System.Action aa;
4649
public event System.Action bb;
4750
}";
@@ -57,6 +60,34 @@ class Foo
5760
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
5861
}
5962

63+
[Fact]
64+
public async Task TestInvalidDeclarationWithTrailingTriviaAsync()
65+
{
66+
const string testCode = @"
67+
class Foo
68+
{
69+
public const int a = 1, // foo
70+
b = 2,
71+
c = 3, // bar
72+
d = 4,
73+
e = 5; /* spam */
74+
}";
75+
const string fixedCode = @"
76+
class Foo
77+
{
78+
public const int a = 1; // foo
79+
public const int b = 2;
80+
public const int c = 3; // bar
81+
public const int d = 4;
82+
public const int e = 5; /* spam */
83+
}";
84+
85+
DiagnosticResult expected = this.CSharpDiagnostic().WithLocation(4, 5);
86+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
87+
await this.VerifyCSharpDiagnosticAsync(fixedCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
88+
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
89+
}
90+
6091
[Fact]
6192
public async Task TestInvalidFieldDeclarationWithAttributesAsync()
6293
{
@@ -81,6 +112,29 @@ class Foo
81112
await this.VerifyCSharpFixAsync(testCode, fixedCode).ConfigureAwait(false);
82113
}
83114

115+
[Theory]
116+
[InlineData("CS1001", 19, "Identifier expected", "private int a,, b;")]
117+
[InlineData("CS1001", 20, "Identifier expected", "private int e, = 3;")]
118+
[InlineData("CS1002", 21, "; expected", "private int c, d")]
119+
public async Task TestDeclarationWithMissingTokensAndNodesAsync(string id, int column, string message, string declaration)
120+
{
121+
string testCode = $@"
122+
class Foo
123+
{{
124+
{declaration}
125+
}}";
126+
127+
DiagnosticResult expected = new DiagnosticResult
128+
{
129+
Id = id,
130+
Severity = DiagnosticSeverity.Error,
131+
Locations = new[] { new DiagnosticResultLocation("Test0.cs", 4, column) },
132+
Message = message
133+
};
134+
135+
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
136+
}
137+
84138
[Fact]
85139
public async Task TestInvalidEventFieldDeclarationWithAttributesAsync()
86140
{

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1132CodeFixProvider.cs‎

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

44
namespace StyleCop.Analyzers.ReadabilityRules
55
{
6+
using System;
67
using System.Collections.Generic;
78
using System.Collections.Immutable;
89
using System.Composition;
@@ -57,7 +58,7 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
5758
{
5859
var syntaxRoot = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
5960
var baseFieldDeclaration = (BaseFieldDeclarationSyntax)syntaxRoot.FindNode(diagnostic.Location.SourceSpan);
60-
List<BaseFieldDeclarationSyntax> newFieldDeclarations = SplitDeclaration(baseFieldDeclaration);
61+
List<BaseFieldDeclarationSyntax> newFieldDeclarations = SplitDeclaration(document, baseFieldDeclaration);
6162

6263
if (newFieldDeclarations != null)
6364
{
@@ -70,59 +71,80 @@ private static async Task<Document> GetTransformedDocumentAsync(Document documen
7071
return document;
7172
}
7273

73-
private static List<BaseFieldDeclarationSyntax> SplitDeclaration(BaseFieldDeclarationSyntax baseFieldDeclaration)
74+
private static List<BaseFieldDeclarationSyntax> SplitDeclaration(Document document, BaseFieldDeclarationSyntax baseFieldDeclaration)
7475
{
7576
var fieldDeclaration = baseFieldDeclaration as FieldDeclarationSyntax;
7677
if (fieldDeclaration != null)
7778
{
78-
VariableDeclarationSyntax declaration = fieldDeclaration.Declaration;
79-
SeparatedSyntaxList<VariableDeclaratorSyntax> variables = declaration.Variables;
80-
VariableDeclaratorSyntax first = variables.First();
81-
var newFieldDeclarations = new List<BaseFieldDeclarationSyntax>(variables.Count);
82-
83-
foreach (VariableDeclaratorSyntax variable in variables)
84-
{
85-
var variableDeclarator = SyntaxFactory.SingletonSeparatedList(variable);
86-
var newFieldDeclaration = fieldDeclaration.WithDeclaration(declaration.WithVariables(variableDeclarator));
87-
88-
if (variable != first)
89-
{
90-
var triviaList = newFieldDeclaration.GetLeadingTrivia().WithoutDirectiveTrivia();
91-
newFieldDeclaration = newFieldDeclaration.WithLeadingTrivia(triviaList);
92-
}
93-
94-
newFieldDeclarations.Add(newFieldDeclaration);
95-
}
96-
97-
return newFieldDeclarations;
79+
return DeclarationSplitter(
80+
document,
81+
fieldDeclaration.Declaration,
82+
fieldDeclaration.WithDeclaration,
83+
fieldDeclaration.SemicolonToken.TrailingTrivia);
9884
}
9985

10086
var eventFieldDeclaration = baseFieldDeclaration as EventFieldDeclarationSyntax;
10187
if (eventFieldDeclaration != null)
10288
{
103-
VariableDeclarationSyntax declaration = eventFieldDeclaration.Declaration;
104-
SeparatedSyntaxList<VariableDeclaratorSyntax> variables = declaration.Variables;
105-
var first = variables.First();
106-
var newEventFieldDeclarations = new List<BaseFieldDeclarationSyntax>(variables.Count);
89+
return DeclarationSplitter(
90+
document,
91+
eventFieldDeclaration.Declaration,
92+
eventFieldDeclaration.WithDeclaration,
93+
eventFieldDeclaration.SemicolonToken.TrailingTrivia);
94+
}
10795

108-
foreach (VariableDeclaratorSyntax variable in variables)
96+
return null;
97+
}
98+
99+
private static List<BaseFieldDeclarationSyntax> DeclarationSplitter(
100+
Document document,
101+
VariableDeclarationSyntax declaration,
102+
Func<VariableDeclarationSyntax, BaseFieldDeclarationSyntax> declarationFactory,
103+
SyntaxTriviaList declarationTrailingTrivia)
104+
{
105+
SeparatedSyntaxList<VariableDeclaratorSyntax> variables = declaration.Variables;
106+
VariableDeclaratorSyntax first = variables.First();
107+
BaseFieldDeclarationSyntax previous = null;
108+
var newFieldDeclarations = new List<BaseFieldDeclarationSyntax>(variables.Count);
109+
110+
foreach (SyntaxNodeOrToken nodeOrToken in variables.GetWithSeparators())
111+
{
112+
if (previous == null)
109113
{
114+
VariableDeclaratorSyntax variable = (VariableDeclaratorSyntax)nodeOrToken.AsNode();
115+
variable = variable.WithIdentifier(variable.Identifier.WithoutLeadingWhitespace());
110116
var variableDeclarator = SyntaxFactory.SingletonSeparatedList(variable);
111-
var newEventFieldDeclaration = eventFieldDeclaration.WithDeclaration(declaration.WithVariables(variableDeclarator));
117+
previous = declarationFactory(declaration.WithVariables(variableDeclarator));
112118

113119
if (variable != first)
114120
{
115-
var triviaList = newEventFieldDeclaration.GetLeadingTrivia().WithoutDirectiveTrivia();
116-
newEventFieldDeclaration = newEventFieldDeclaration.WithLeadingTrivia(triviaList);
121+
var triviaList = previous.GetLeadingTrivia().WithoutDirectiveTrivia();
122+
previous = previous.WithLeadingTrivia(triviaList);
117123
}
118-
119-
newEventFieldDeclarations.Add(newEventFieldDeclaration);
120124
}
125+
else
126+
{
127+
SyntaxToken commaToken = nodeOrToken.AsToken();
128+
SyntaxTriviaList trailingTrivia = commaToken.TrailingTrivia;
129+
if (trailingTrivia.Any())
130+
{
131+
if (!trailingTrivia.Last().IsKind(SyntaxKind.EndOfLineTrivia))
132+
{
133+
trailingTrivia = trailingTrivia.WithoutTrailingWhitespace().Add(TriviaHelper.GetNewLineTrivia(document));
134+
}
135+
}
136+
else
137+
{
138+
trailingTrivia = SyntaxTriviaList.Create(TriviaHelper.GetNewLineTrivia(document));
139+
}
121140

122-
return newEventFieldDeclarations;
141+
newFieldDeclarations.Add(previous.WithTrailingTrivia(trailingTrivia));
142+
previous = null;
143+
}
123144
}
124145

125-
return null;
146+
newFieldDeclarations.Add(previous.WithTrailingTrivia(declarationTrailingTrivia));
147+
return newFieldDeclarations;
126148
}
127149
}
128150
}

‎StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SA1132DoNotCombineFields.cs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,22 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte
4545
private static void HandleDeclaration(SyntaxNodeAnalysisContext context)
4646
{
4747
var fieldDeclaration = (BaseFieldDeclarationSyntax)context.Node;
48-
if (fieldDeclaration.Declaration.Variables.Count > 1)
48+
var variables = fieldDeclaration.Declaration.Variables;
49+
50+
if (variables.Count < 2 || fieldDeclaration.SemicolonToken.IsMissing)
4951
{
50-
context.ReportDiagnostic(Diagnostic.Create(Descriptor, fieldDeclaration.GetLocation()));
52+
return;
5153
}
54+
55+
foreach (VariableDeclaratorSyntax variable in variables)
56+
{
57+
if (variable.IsMissing || variable.Identifier.IsMissing)
58+
{
59+
return;
60+
}
61+
}
62+
63+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, fieldDeclaration.GetLocation()));
5264
}
5365
}
5466
}

‎StyleCop.Analyzers/StyleCop.Analyzers/SpacingRules/SpacingExtensions.cs‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,32 @@ public static bool IsMissingOrDefault(this SyntaxToken token)
2020
|| token.IsMissing;
2121
}
2222

23+
public static SyntaxToken WithoutLeadingWhitespace(this SyntaxToken token, bool removeEndOfLineTrivia = false)
24+
{
25+
if (!token.HasLeadingTrivia)
26+
{
27+
return token;
28+
}
29+
30+
return token.WithLeadingTrivia(token.LeadingTrivia.WithoutWhitespace(removeEndOfLineTrivia));
31+
}
32+
33+
public static SyntaxTriviaList WithoutWhitespace(this SyntaxTriviaList syntaxTriviaList, bool removeEndOfLineTrivia = false)
34+
{
35+
if (syntaxTriviaList.Count == 0)
36+
{
37+
return syntaxTriviaList;
38+
}
39+
40+
var trivia = syntaxTriviaList.Where(i => !i.IsKind(SyntaxKind.WhitespaceTrivia));
41+
if (removeEndOfLineTrivia)
42+
{
43+
trivia = trivia.Where(i => !i.IsKind(SyntaxKind.EndOfLineTrivia));
44+
}
45+
46+
return SyntaxFactory.TriviaList(trivia);
47+
}
48+
2349
/// <summary>
2450
/// Removes the leading and trailing trivia associated with a syntax token.
2551
/// </summary>

0 commit comments

Comments
 (0)