Skip to content

Commit fd13f1d

Browse files
committed
Implement SX1101 using the speculative semantic model
Thanks to @vweijsters for some of the new tests. 👍 Fixes #1706 Fixes #1707
1 parent 23fa4de commit fd13f1d

2 files changed

Lines changed: 187 additions & 20 deletions

File tree

StyleCop.Analyzers/StyleCop.Analyzers.Test/ReadabilityRules/SX1101UnitTests.cs

Lines changed: 148 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,16 +59,20 @@ public void TestMethod2()
5959
this.TestMethod1();
6060
}
6161
62+
var x = this.listeners;
6263
this.BaseTestEvent += (s, e) => { };
6364
var listeners = this.TestEvent;
6465
if (listeners != null)
6566
{
6667
listeners(this, null);
6768
}
69+
70+
var y = this.listeners;
6871
}
72+
73+
public int listeners;
6974
}
7075
";
71-
7276
var fixedTestCode = @"using System;
7377
7478
public class BaseTestClass
@@ -107,13 +111,18 @@ public void TestMethod2()
107111
TestMethod1();
108112
}
109113
114+
var x = this.listeners;
110115
BaseTestEvent += (s, e) => { };
111116
var listeners = TestEvent;
112117
if (listeners != null)
113118
{
114119
listeners(this, null);
115120
}
121+
122+
var y = this.listeners;
116123
}
124+
125+
public int listeners;
117126
}
118127
";
119128

@@ -125,8 +134,8 @@ public void TestMethod2()
125134
this.CSharpDiagnostic().WithLocation(30, 84),
126135
this.CSharpDiagnostic().WithLocation(32, 13),
127136
this.CSharpDiagnostic().WithLocation(36, 13),
128-
this.CSharpDiagnostic().WithLocation(39, 9),
129-
this.CSharpDiagnostic().WithLocation(40, 25)
137+
this.CSharpDiagnostic().WithLocation(40, 9),
138+
this.CSharpDiagnostic().WithLocation(41, 25)
130139
};
131140

132141
await this.VerifyCSharpDiagnosticAsync(testCode, expected, CancellationToken.None).ConfigureAwait(false);
@@ -211,6 +220,11 @@ public class TestClass
211220
{
212221
private int test;
213222
223+
public TestClass(int test)
224+
{
225+
this.test = test;
226+
}
227+
214228
public void TestMethod(int test)
215229
{
216230
this.test = test;
@@ -221,6 +235,137 @@ public void TestMethod(int test)
221235
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
222236
}
223237

238+
/// <summary>
239+
/// Verifies that a necessary field reference prefix will not produce any diagnostics.
240+
/// </summary>
241+
/// <returns>A <see cref="Task"/> representing the asynchronous unit test.</returns>
242+
[Fact]
243+
public async Task VerifyThatNecessaryFieldReferencePrefixWillNotProduceDiagnosticAsync()
244+
{
245+
var testCode = @"
246+
public class TestClass
247+
{
248+
private int test;
249+
250+
public TestClass()
251+
{
252+
var test = this.test;
253+
}
254+
255+
public void TestMethod()
256+
{
257+
var test = this.test;
258+
}
259+
}
260+
";
261+
262+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
263+
}
264+
265+
[Fact]
266+
public async Task TestExtensionMethodAsync()
267+
{
268+
var testCode = @"
269+
class ClassName
270+
{
271+
string P => string.Empty;
272+
273+
public void Method()
274+
{
275+
int value = this.P();
276+
}
277+
}
278+
279+
static class ClassNameExtensions
280+
{
281+
public static int P(this ClassName instance) => 3;
282+
}
283+
";
284+
285+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
286+
}
287+
288+
[Fact]
289+
public async Task TestTypeArgumentCollisionAsync()
290+
{
291+
var testCode = @"
292+
class BaseClass
293+
{
294+
protected int Options => 3;
295+
}
296+
297+
class DerivedClass : BaseClass
298+
{
299+
void Method<Options>()
300+
{
301+
int options = this.Options;
302+
}
303+
}
304+
";
305+
306+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
307+
}
308+
309+
[Fact]
310+
public async Task TestPropertyValueKeywordCollisionAsync()
311+
{
312+
var testCode = @"
313+
class ClassName
314+
{
315+
int value;
316+
317+
int Value
318+
{
319+
get { return value; }
320+
set { this.value = value; }
321+
}
322+
}
323+
";
324+
325+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
326+
}
327+
328+
[Fact]
329+
public async Task TestEscapedParameterCollisionAsync()
330+
{
331+
var testCode = @"
332+
class ClassName
333+
{
334+
int @object;
335+
336+
ClassName(int @object)
337+
{
338+
this.@object = @object;
339+
}
340+
}
341+
";
342+
343+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
344+
}
345+
346+
[Fact]
347+
public async Task TestFieldUseBeforeVariableDefinitionAsync()
348+
{
349+
var testCode = @"
350+
class ClassName
351+
{
352+
object p;
353+
354+
object Property
355+
{
356+
get
357+
{
358+
if (this.p != null) return this.p;
359+
object p = new object();
360+
return this.p = p;
361+
}
362+
}
363+
}
364+
";
365+
366+
await this.VerifyCSharpDiagnosticAsync(testCode, EmptyDiagnosticResults, CancellationToken.None).ConfigureAwait(false);
367+
}
368+
224369
/// <inheritdoc/>
225370
protected override IEnumerable<DiagnosticAnalyzer> GetCSharpDiagnosticAnalyzers()
226371
{

StyleCop.Analyzers/StyleCop.Analyzers/ReadabilityRules/SX1101DoNotPrefixLocalMembersWithThis.cs

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
namespace StyleCop.Analyzers.ReadabilityRules
55
{
66
using System;
7-
using System.Collections.Generic;
87
using System.Collections.Immutable;
98
using System.Linq;
109
using Microsoft.CodeAnalysis;
@@ -47,33 +46,56 @@ private static void HandleCompilationStart(CompilationStartAnalysisContext conte
4746

4847
private static void HandleThisExpression(SyntaxNodeAnalysisContext context)
4948
{
50-
if (context.Node.Parent.IsKind(SyntaxKind.SimpleMemberAccessExpression))
49+
if (!context.Node.Parent.IsKind(SyntaxKind.SimpleMemberAccessExpression))
5150
{
52-
var memberAccessExpression = (MemberAccessExpressionSyntax)context.Node.Parent;
53-
var memberName = memberAccessExpression.Name.ToString();
51+
return;
52+
}
53+
54+
var memberAccessExpression = (MemberAccessExpressionSyntax)context.Node.Parent;
55+
var originalSymbolInfo = context.SemanticModel.GetSymbolInfo(memberAccessExpression, context.CancellationToken);
56+
if (originalSymbolInfo.Symbol == null)
57+
{
58+
return;
59+
}
5460

55-
var parameters = GetMethodParameters(context.Node);
56-
if (!parameters.Contains(memberName))
61+
SyntaxNode speculationRoot;
62+
var annotation = new SyntaxAnnotation();
63+
SemanticModel speculativeModel;
64+
65+
StatementSyntax statement = context.Node.FirstAncestorOrSelf<StatementSyntax>();
66+
if (statement != null)
67+
{
68+
var modifiedStatement = statement.ReplaceNode(memberAccessExpression, memberAccessExpression.Name.WithAdditionalAnnotations(annotation));
69+
speculationRoot = modifiedStatement;
70+
if (!context.SemanticModel.TryGetSpeculativeSemanticModel(statement.SpanStart, modifiedStatement, out speculativeModel))
5771
{
58-
context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation()));
72+
return;
5973
}
6074
}
61-
}
62-
63-
private static IList<string> GetMethodParameters(SyntaxNode node)
64-
{
65-
while ((node != null) && !node.IsKind(SyntaxKind.MethodDeclaration))
75+
else
6676
{
67-
node = node.Parent;
77+
ArrowExpressionClauseSyntax arrowExpressionClause = context.Node.FirstAncestorOrSelf<ArrowExpressionClauseSyntax>();
78+
if (arrowExpressionClause == null)
79+
{
80+
return;
81+
}
82+
83+
var modifiedArrowExpressionClause = arrowExpressionClause.ReplaceNode(memberAccessExpression, memberAccessExpression.Name.WithAdditionalAnnotations(annotation));
84+
speculationRoot = modifiedArrowExpressionClause;
85+
if (!context.SemanticModel.TryGetSpeculativeSemanticModel(arrowExpressionClause.SpanStart, modifiedArrowExpressionClause, out speculativeModel))
86+
{
87+
return;
88+
}
6889
}
6990

70-
if (node == null)
91+
SyntaxNode mappedNode = speculationRoot.GetAnnotatedNodes(annotation).Single();
92+
var newSymbolInfo = speculativeModel.GetSymbolInfo(mappedNode, context.CancellationToken);
93+
if (!Equals(originalSymbolInfo.Symbol, newSymbolInfo.Symbol))
7194
{
72-
return new List<string>();
95+
return;
7396
}
7497

75-
var methodDeclaration = (MethodDeclarationSyntax)node;
76-
return methodDeclaration.ParameterList.Parameters.Select(p => p.Identifier.ValueText).ToList();
98+
context.ReportDiagnostic(Diagnostic.Create(Descriptor, context.Node.GetLocation()));
7799
}
78100
}
79101
}

0 commit comments

Comments
 (0)