Skip to content

Commit 9e2bda8

Browse files
Copilotdrewnoakes
Copilot
and
drewnoakes
committed
Add diagnostic for invalid CompletedTaskAttribute usage and support for init accessors
Co-authored-by: drewnoakes <[email protected]>
1 parent a593dc1 commit 9e2bda8

File tree

3 files changed

+256
-9
lines changed

3 files changed

+256
-9
lines changed

‎src/Microsoft.VisualStudio.Threading.Analyzers.CSharp/VSTHRD003UseJtfRunAsyncAnalyzer.cs‎

Lines changed: 83 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,15 @@ public class VSTHRD003UseJtfRunAsyncAnalyzer : DiagnosticAnalyzer
4040
{
4141
public const string Id = "VSTHRD003";
4242

43+
public static readonly DiagnosticDescriptor InvalidAttributeUseDescriptor = new DiagnosticDescriptor(
44+
id: Id,
45+
title: new LocalizableResourceString(nameof(Strings.VSTHRD003InvalidAttributeUse_Title), Strings.ResourceManager, typeof(Strings)),
46+
messageFormat: new LocalizableResourceString(nameof(Strings.VSTHRD003InvalidAttributeUse_MessageFormat), Strings.ResourceManager, typeof(Strings)),
47+
helpLinkUri: Utils.GetHelpLink(Id),
48+
category: "Usage",
49+
defaultSeverity: DiagnosticSeverity.Warning,
50+
isEnabledByDefault: true);
51+
4352
internal static readonly DiagnosticDescriptor Descriptor = new DiagnosticDescriptor(
4453
id: Id,
4554
title: new LocalizableResourceString(nameof(Strings.VSTHRD003_Title), Strings.ResourceManager, typeof(Strings)),
@@ -54,7 +63,7 @@ public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics
5463
{
5564
get
5665
{
57-
return ImmutableArray.Create(Descriptor);
66+
return ImmutableArray.Create(InvalidAttributeUseDescriptor, Descriptor);
5867
}
5968
}
6069

@@ -69,6 +78,7 @@ public override void Initialize(AnalysisContext context)
6978
context.RegisterSyntaxNodeAction(Utils.DebuggableWrapper(this.AnalyzeArrowExpressionClause), SyntaxKind.ArrowExpressionClause);
7079
context.RegisterSyntaxNodeAction(Utils.DebuggableWrapper(this.AnalyzeLambdaExpression), SyntaxKind.SimpleLambdaExpression);
7180
context.RegisterSyntaxNodeAction(Utils.DebuggableWrapper(this.AnalyzeLambdaExpression), SyntaxKind.ParenthesizedLambdaExpression);
81+
context.RegisterSymbolAction(Utils.DebuggableWrapper(this.AnalyzeSymbolForInvalidAttributeUse), SymbolKind.Field, SymbolKind.Property, SymbolKind.Method);
7282
}
7383

7484
private static bool IsSymbolAlwaysOkToAwait(ISymbol? symbol, Compilation compilation)
@@ -95,9 +105,22 @@ private static bool IsSymbolAlwaysOkToAwait(ISymbol? symbol, Compilation compila
95105
else if (symbol is IPropertySymbol propertySymbol)
96106
{
97107
// Properties must not have non-private setters
98-
if (propertySymbol.SetMethod is not null && propertySymbol.SetMethod.DeclaredAccessibility != Accessibility.Private)
108+
// Init accessors are only allowed if the property itself is private
109+
if (propertySymbol.SetMethod is not null)
99110
{
100-
return false;
111+
if (propertySymbol.SetMethod.IsInitOnly)
112+
{
113+
// Init accessor - only allowed if property is private
114+
if (propertySymbol.DeclaredAccessibility != Accessibility.Private)
115+
{
116+
return false;
117+
}
118+
}
119+
else if (propertySymbol.SetMethod.DeclaredAccessibility != Accessibility.Private)
120+
{
121+
// Regular setter must be private
122+
return false;
123+
}
101124
}
102125
}
103126

@@ -186,6 +209,63 @@ private static string GetFullyQualifiedName(ISymbol symbol)
186209
return string.Join(".", parts);
187210
}
188211

212+
private void AnalyzeSymbolForInvalidAttributeUse(SymbolAnalysisContext context)
213+
{
214+
ISymbol symbol = context.Symbol;
215+
216+
// Check if the symbol has the CompletedTaskAttribute
217+
AttributeData? completedTaskAttr = symbol.GetAttributes().FirstOrDefault(attr =>
218+
attr.AttributeClass?.Name == Types.CompletedTaskAttribute.TypeName &&
219+
attr.AttributeClass.BelongsToNamespace(Types.CompletedTaskAttribute.Namespace));
220+
221+
if (completedTaskAttr is null)
222+
{
223+
return;
224+
}
225+
226+
string? errorMessage = null;
227+
228+
if (symbol is IFieldSymbol fieldSymbol)
229+
{
230+
// Fields must be readonly
231+
if (!fieldSymbol.IsReadOnly)
232+
{
233+
errorMessage = "Fields must be readonly.";
234+
}
235+
}
236+
else if (symbol is IPropertySymbol propertySymbol)
237+
{
238+
// Check for init accessor (which is a special kind of setter)
239+
if (propertySymbol.SetMethod is not null)
240+
{
241+
// Init accessors are only allowed if the property itself is private
242+
if (propertySymbol.SetMethod.IsInitOnly)
243+
{
244+
if (propertySymbol.DeclaredAccessibility != Accessibility.Private)
245+
{
246+
errorMessage = "Properties with init accessors must be private.";
247+
}
248+
}
249+
else if (propertySymbol.SetMethod.DeclaredAccessibility != Accessibility.Private)
250+
{
251+
// Non-private setters are not allowed
252+
errorMessage = "Properties must not have non-private setters.";
253+
}
254+
}
255+
}
256+
257+
// Methods are always allowed
258+
if (errorMessage is not null)
259+
{
260+
// Report diagnostic on the attribute location
261+
Location? location = completedTaskAttr.ApplicationSyntaxReference?.GetSyntax(context.CancellationToken).GetLocation();
262+
if (location is not null)
263+
{
264+
context.ReportDiagnostic(Diagnostic.Create(InvalidAttributeUseDescriptor, location, errorMessage));
265+
}
266+
}
267+
}
268+
189269
private void AnalyzeArrowExpressionClause(SyntaxNodeAnalysisContext context)
190270
{
191271
var arrowExpressionClause = (ArrowExpressionClauseSyntax)context.Node;

‎src/Microsoft.VisualStudio.Threading.Analyzers/Strings.resx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,4 +353,10 @@ Start the work within this context, or use JoinableTaskFactory.RunAsync to start
353353
<data name="VSTHRD115_CodeFix_UseFactory_Title" xml:space="preserve">
354354
<value>Use 'JoinableTaskContext.CreateNoOpContext' instead.</value>
355355
</data>
356+
<data name="VSTHRD003InvalidAttributeUse_Title" xml:space="preserve">
357+
<value>Invalid use of CompletedTaskAttribute</value>
358+
</data>
359+
<data name="VSTHRD003InvalidAttributeUse_MessageFormat" xml:space="preserve">
360+
<value>CompletedTaskAttribute can only be applied to readonly fields, properties without non-private setters, or methods. {0}</value>
361+
</data>
356362
</root>

‎test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD003UseJtfRunAsyncAnalyzerTests.cs‎

Lines changed: 167 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1868,7 +1868,7 @@ internal sealed class CompletedTaskAttribute : System.Attribute
18681868
18691869
class Tests
18701870
{
1871-
[Microsoft.VisualStudio.Threading.CompletedTask]
1871+
[{|#0:Microsoft.VisualStudio.Threading.CompletedTask|}]
18721872
private static Task MyTask = Task.CompletedTask; // Not readonly
18731873
18741874
async Task GetTask()
@@ -1877,7 +1877,10 @@ async Task GetTask()
18771877
}
18781878
}
18791879
";
1880-
await CSVerify.VerifyAnalyzerAsync(test);
1880+
DiagnosticResult expected = new DiagnosticResult(Microsoft.VisualStudio.Threading.Analyzers.VSTHRD003UseJtfRunAsyncAnalyzer.InvalidAttributeUseDescriptor)
1881+
.WithLocation(0)
1882+
.WithArguments("Fields must be readonly.");
1883+
await CSVerify.VerifyAnalyzerAsync(test, expected);
18811884
}
18821885

18831886
[Fact]
@@ -1896,7 +1899,7 @@ internal sealed class CompletedTaskAttribute : System.Attribute
18961899
18971900
class Tests
18981901
{
1899-
[Microsoft.VisualStudio.Threading.CompletedTask]
1902+
[{|#0:Microsoft.VisualStudio.Threading.CompletedTask|}]
19001903
public static Task MyTask { get; set; } = Task.CompletedTask; // Public setter
19011904
19021905
async Task GetTask()
@@ -1905,7 +1908,10 @@ async Task GetTask()
19051908
}
19061909
}
19071910
";
1908-
await CSVerify.VerifyAnalyzerAsync(test);
1911+
DiagnosticResult expected = new DiagnosticResult(Microsoft.VisualStudio.Threading.Analyzers.VSTHRD003UseJtfRunAsyncAnalyzer.InvalidAttributeUseDescriptor)
1912+
.WithLocation(0)
1913+
.WithArguments("Properties must not have non-private setters.");
1914+
await CSVerify.VerifyAnalyzerAsync(test, expected);
19091915
}
19101916

19111917
[Fact]
@@ -1924,7 +1930,7 @@ internal sealed class CompletedTaskAttribute : System.Attribute
19241930
19251931
class Tests
19261932
{
1927-
[Microsoft.VisualStudio.Threading.CompletedTask]
1933+
[{|#0:Microsoft.VisualStudio.Threading.CompletedTask|}]
19281934
public static Task MyTask { get; internal set; } = Task.CompletedTask; // Internal setter
19291935
19301936
async Task GetTask()
@@ -1933,7 +1939,10 @@ async Task GetTask()
19331939
}
19341940
}
19351941
";
1936-
await CSVerify.VerifyAnalyzerAsync(test);
1942+
DiagnosticResult expected = new DiagnosticResult(Microsoft.VisualStudio.Threading.Analyzers.VSTHRD003UseJtfRunAsyncAnalyzer.InvalidAttributeUseDescriptor)
1943+
.WithLocation(0)
1944+
.WithArguments("Properties must not have non-private setters.");
1945+
await CSVerify.VerifyAnalyzerAsync(test, expected);
19371946
}
19381947

19391948
[Fact]
@@ -1992,6 +2001,158 @@ async Task GetTask()
19922001
await CSVerify.VerifyAnalyzerAsync(test);
19932002
}
19942003

2004+
[Fact]
2005+
public async Task ReportDiagnosticWhenCompletedTaskAttributeOnPropertyWithPublicInit()
2006+
{
2007+
var test = @"
2008+
using System.Threading.Tasks;
2009+
2010+
namespace Microsoft.VisualStudio.Threading
2011+
{
2012+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
2013+
internal sealed class CompletedTaskAttribute : System.Attribute
2014+
{
2015+
}
2016+
}
2017+
2018+
class Tests
2019+
{
2020+
[{|#0:Microsoft.VisualStudio.Threading.CompletedTask|}]
2021+
public static Task MyTask { get; init; } = Task.CompletedTask; // Public init
2022+
2023+
async Task GetTask()
2024+
{
2025+
await [|MyTask|];
2026+
}
2027+
}
2028+
";
2029+
DiagnosticResult expected = new DiagnosticResult(Microsoft.VisualStudio.Threading.Analyzers.VSTHRD003UseJtfRunAsyncAnalyzer.InvalidAttributeUseDescriptor)
2030+
.WithLocation(0)
2031+
.WithArguments("Properties with init accessors must be private.");
2032+
await CSVerify.VerifyAnalyzerAsync(test, expected);
2033+
}
2034+
2035+
[Fact]
2036+
public async Task DoNotReportWarningWhenCompletedTaskAttributeOnPrivatePropertyWithInit()
2037+
{
2038+
var test = @"
2039+
using System.Threading.Tasks;
2040+
2041+
namespace Microsoft.VisualStudio.Threading
2042+
{
2043+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
2044+
internal sealed class CompletedTaskAttribute : System.Attribute
2045+
{
2046+
}
2047+
}
2048+
2049+
class Tests
2050+
{
2051+
[Microsoft.VisualStudio.Threading.CompletedTask]
2052+
private static Task MyTask { get; init; } = Task.CompletedTask; // Private init is OK
2053+
2054+
async Task GetTask()
2055+
{
2056+
await MyTask;
2057+
}
2058+
}
2059+
";
2060+
await CSVerify.VerifyAnalyzerAsync(test);
2061+
}
2062+
2063+
[Fact]
2064+
public async Task ReportDiagnosticWhenCompletedTaskAttributeOnNonReadonlyFieldWithDiagnosticOnAttribute()
2065+
{
2066+
var test = @"
2067+
using System.Threading.Tasks;
2068+
2069+
namespace Microsoft.VisualStudio.Threading
2070+
{
2071+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
2072+
internal sealed class CompletedTaskAttribute : System.Attribute
2073+
{
2074+
}
2075+
}
2076+
2077+
class Tests
2078+
{
2079+
[{|#0:Microsoft.VisualStudio.Threading.CompletedTask|}]
2080+
private static Task MyTask = Task.CompletedTask; // Not readonly
2081+
2082+
async Task GetTask()
2083+
{
2084+
await [|MyTask|];
2085+
}
2086+
}
2087+
";
2088+
DiagnosticResult expected = new DiagnosticResult(Microsoft.VisualStudio.Threading.Analyzers.VSTHRD003UseJtfRunAsyncAnalyzer.InvalidAttributeUseDescriptor)
2089+
.WithLocation(0)
2090+
.WithArguments("Fields must be readonly.");
2091+
await CSVerify.VerifyAnalyzerAsync(test, expected);
2092+
}
2093+
2094+
[Fact]
2095+
public async Task ReportDiagnosticWhenCompletedTaskAttributeOnPropertyWithPublicSetterWithDiagnosticOnAttribute()
2096+
{
2097+
var test = @"
2098+
using System.Threading.Tasks;
2099+
2100+
namespace Microsoft.VisualStudio.Threading
2101+
{
2102+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
2103+
internal sealed class CompletedTaskAttribute : System.Attribute
2104+
{
2105+
}
2106+
}
2107+
2108+
class Tests
2109+
{
2110+
[{|#0:Microsoft.VisualStudio.Threading.CompletedTask|}]
2111+
public static Task MyTask { get; set; } = Task.CompletedTask; // Public setter
2112+
2113+
async Task GetTask()
2114+
{
2115+
await [|MyTask|];
2116+
}
2117+
}
2118+
";
2119+
DiagnosticResult expected = new DiagnosticResult(Microsoft.VisualStudio.Threading.Analyzers.VSTHRD003UseJtfRunAsyncAnalyzer.InvalidAttributeUseDescriptor)
2120+
.WithLocation(0)
2121+
.WithArguments("Properties must not have non-private setters.");
2122+
await CSVerify.VerifyAnalyzerAsync(test, expected);
2123+
}
2124+
2125+
[Fact]
2126+
public async Task ReportDiagnosticWhenCompletedTaskAttributeOnPropertyWithInternalSetterWithDiagnosticOnAttribute()
2127+
{
2128+
var test = @"
2129+
using System.Threading.Tasks;
2130+
2131+
namespace Microsoft.VisualStudio.Threading
2132+
{
2133+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
2134+
internal sealed class CompletedTaskAttribute : System.Attribute
2135+
{
2136+
}
2137+
}
2138+
2139+
class Tests
2140+
{
2141+
[{|#0:Microsoft.VisualStudio.Threading.CompletedTask|}]
2142+
public static Task MyTask { get; internal set; } = Task.CompletedTask; // Internal setter
2143+
2144+
async Task GetTask()
2145+
{
2146+
await [|MyTask|];
2147+
}
2148+
}
2149+
";
2150+
DiagnosticResult expected = new DiagnosticResult(Microsoft.VisualStudio.Threading.Analyzers.VSTHRD003UseJtfRunAsyncAnalyzer.InvalidAttributeUseDescriptor)
2151+
.WithLocation(0)
2152+
.WithArguments("Properties must not have non-private setters.");
2153+
await CSVerify.VerifyAnalyzerAsync(test, expected);
2154+
}
2155+
19952156
private DiagnosticResult CreateDiagnostic(int line, int column, int length) =>
19962157
CSVerify.Diagnostic().WithSpan(line, column, line, column + length);
19972158
}

0 commit comments

Comments
 (0)