Skip to content

Commit a593dc1

Browse files
Copilotdrewnoakes
andcommitted
Add validation for CompletedTaskAttribute on non-readonly fields and properties with non-private setters
Co-authored-by: drewnoakes <350947+drewnoakes@users.noreply.github.com>
1 parent d067b66 commit a593dc1

3 files changed

Lines changed: 162 additions & 0 deletions

File tree

docfx/analyzers/VSTHRD003.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,10 @@ async Task MyMethodAsync()
2626
}
2727
```
2828

29+
**Important restrictions:**
30+
- Fields must be marked `readonly` when using this attribute
31+
- Properties must not have non-private setters (getter-only or private setters are allowed)
32+
2933
### Marking external types
3034

3135
You can also apply the attribute at the assembly level to mark members in external types that you don't control:

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,24 @@ private static bool IsSymbolAlwaysOkToAwait(ISymbol? symbol, Compilation compila
8383
attr.AttributeClass?.Name == Types.CompletedTaskAttribute.TypeName &&
8484
attr.AttributeClass.BelongsToNamespace(Types.CompletedTaskAttribute.Namespace)))
8585
{
86+
// Validate that the attribute is used correctly
87+
if (symbol is IFieldSymbol fieldSymbol)
88+
{
89+
// Fields must be readonly
90+
if (!fieldSymbol.IsReadOnly)
91+
{
92+
return false;
93+
}
94+
}
95+
else if (symbol is IPropertySymbol propertySymbol)
96+
{
97+
// Properties must not have non-private setters
98+
if (propertySymbol.SetMethod is not null && propertySymbol.SetMethod.DeclaredAccessibility != Accessibility.Private)
99+
{
100+
return false;
101+
}
102+
}
103+
86104
return true;
87105
}
88106

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

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1852,6 +1852,146 @@ async Task TestMethod()
18521852
await CSVerify.VerifyAnalyzerAsync(test);
18531853
}
18541854

1855+
[Fact]
1856+
public async Task ReportWarningWhenCompletedTaskAttributeOnNonReadonlyField()
1857+
{
1858+
var test = @"
1859+
using System.Threading.Tasks;
1860+
1861+
namespace Microsoft.VisualStudio.Threading
1862+
{
1863+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
1864+
internal sealed class CompletedTaskAttribute : System.Attribute
1865+
{
1866+
}
1867+
}
1868+
1869+
class Tests
1870+
{
1871+
[Microsoft.VisualStudio.Threading.CompletedTask]
1872+
private static Task MyTask = Task.CompletedTask; // Not readonly
1873+
1874+
async Task GetTask()
1875+
{
1876+
await [|MyTask|];
1877+
}
1878+
}
1879+
";
1880+
await CSVerify.VerifyAnalyzerAsync(test);
1881+
}
1882+
1883+
[Fact]
1884+
public async Task ReportWarningWhenCompletedTaskAttributeOnPropertyWithPublicSetter()
1885+
{
1886+
var test = @"
1887+
using System.Threading.Tasks;
1888+
1889+
namespace Microsoft.VisualStudio.Threading
1890+
{
1891+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
1892+
internal sealed class CompletedTaskAttribute : System.Attribute
1893+
{
1894+
}
1895+
}
1896+
1897+
class Tests
1898+
{
1899+
[Microsoft.VisualStudio.Threading.CompletedTask]
1900+
public static Task MyTask { get; set; } = Task.CompletedTask; // Public setter
1901+
1902+
async Task GetTask()
1903+
{
1904+
await [|MyTask|];
1905+
}
1906+
}
1907+
";
1908+
await CSVerify.VerifyAnalyzerAsync(test);
1909+
}
1910+
1911+
[Fact]
1912+
public async Task ReportWarningWhenCompletedTaskAttributeOnPropertyWithInternalSetter()
1913+
{
1914+
var test = @"
1915+
using System.Threading.Tasks;
1916+
1917+
namespace Microsoft.VisualStudio.Threading
1918+
{
1919+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
1920+
internal sealed class CompletedTaskAttribute : System.Attribute
1921+
{
1922+
}
1923+
}
1924+
1925+
class Tests
1926+
{
1927+
[Microsoft.VisualStudio.Threading.CompletedTask]
1928+
public static Task MyTask { get; internal set; } = Task.CompletedTask; // Internal setter
1929+
1930+
async Task GetTask()
1931+
{
1932+
await [|MyTask|];
1933+
}
1934+
}
1935+
";
1936+
await CSVerify.VerifyAnalyzerAsync(test);
1937+
}
1938+
1939+
[Fact]
1940+
public async Task DoNotReportWarningWhenCompletedTaskAttributeOnPropertyWithPrivateSetter()
1941+
{
1942+
var test = @"
1943+
using System.Threading.Tasks;
1944+
1945+
namespace Microsoft.VisualStudio.Threading
1946+
{
1947+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
1948+
internal sealed class CompletedTaskAttribute : System.Attribute
1949+
{
1950+
}
1951+
}
1952+
1953+
class Tests
1954+
{
1955+
[Microsoft.VisualStudio.Threading.CompletedTask]
1956+
public static Task MyTask { get; private set; } = Task.CompletedTask; // Private setter is OK
1957+
1958+
async Task GetTask()
1959+
{
1960+
await MyTask;
1961+
}
1962+
}
1963+
";
1964+
await CSVerify.VerifyAnalyzerAsync(test);
1965+
}
1966+
1967+
[Fact]
1968+
public async Task DoNotReportWarningWhenCompletedTaskAttributeOnPropertyWithGetterOnly()
1969+
{
1970+
var test = @"
1971+
using System.Threading.Tasks;
1972+
1973+
namespace Microsoft.VisualStudio.Threading
1974+
{
1975+
[System.AttributeUsage(System.AttributeTargets.Property | System.AttributeTargets.Method | System.AttributeTargets.Field, Inherited = false, AllowMultiple = false)]
1976+
internal sealed class CompletedTaskAttribute : System.Attribute
1977+
{
1978+
}
1979+
}
1980+
1981+
class Tests
1982+
{
1983+
[Microsoft.VisualStudio.Threading.CompletedTask]
1984+
public static Task MyTask { get; } = Task.CompletedTask; // Getter-only is OK
1985+
1986+
async Task GetTask()
1987+
{
1988+
await MyTask;
1989+
}
1990+
}
1991+
";
1992+
await CSVerify.VerifyAnalyzerAsync(test);
1993+
}
1994+
18551995
private DiagnosticResult CreateDiagnostic(int line, int column, int length) =>
18561996
CSVerify.Diagnostic().WithSpan(line, column, line, column + length);
18571997
}

0 commit comments

Comments
 (0)