diff --git a/docfx/analyzers/configuration.md b/docfx/analyzers/configuration.md index 77acf4d82..8af4563a2 100644 --- a/docfx/analyzers/configuration.md +++ b/docfx/analyzers/configuration.md @@ -25,6 +25,16 @@ all use `vs-threading.TopicA.txt` as the filename for their `AdditionalFiles` it These files may contain blank lines or comments that start with the `#` character. +Files that contain a backtick use metadata type names. In these files, generic types include +a backtick followed by the number of type parameters. For example, the metadata name for +`DbSet` is ``DbSet`1``. Including the arity distinguishes a generic type from a +non-generic type with the same name. + +For backward compatibility, files without any backticks match type names without regard +to generic arity. For example, `DbSet` in such a file matches both a non-generic `DbSet` +and every generic `DbSet` type. Add a backtick anywhere in the file to opt into +exact metadata-name matching for every entry in that file. + ## Methods that assert the main thread Code may assert it is running on the main thread by calling a method that is designed @@ -92,3 +102,5 @@ excluded from VSTHRD103 analysis by specifying them in a configuration file. **Line format:** `[Namespace.TypeName]::MethodName` **Sample:** `[System.Data.SqlClient.SqlDataReader]::Read` + +**Generic sample:** ``[Microsoft.EntityFrameworkCore.DbSet`1]::Add`` diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/CommonFixes.cs b/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/CommonFixes.cs index 79015ac8d..1679cf0d3 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/CommonFixes.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers.CodeFixes/CommonFixes.cs @@ -26,15 +26,19 @@ internal static class CommonFixes internal static async Task> ReadMethodsAsync(CodeFixContext codeFixContext, Regex fileNamePattern, CancellationToken cancellationToken) { ImmutableArray.Builder? result = ImmutableArray.CreateBuilder(); - foreach (string line in await ReadAdditionalFilesAsync(codeFixContext.Document.Project.AdditionalDocuments, fileNamePattern, cancellationToken)) + foreach (SourceText text in await ReadAdditionalFileTextsAsync(codeFixContext.Document.Project.AdditionalDocuments, fileNamePattern, cancellationToken)) { - result.Add(ParseAdditionalFileMethodLine(line)); + bool matchAnyArity = !Contains(text, '`'); + foreach (string line in ReadLinesFromAdditionalFile(text)) + { + result.Add(ParseAdditionalFileMethodLine(line, matchAnyArity)); + } } return result.ToImmutable(); } - internal static async Task> ReadAdditionalFilesAsync(IEnumerable additionalFiles, Regex fileNamePattern, CancellationToken cancellationToken) + internal static async Task> ReadAdditionalFileTextsAsync(IEnumerable additionalFiles, Regex fileNamePattern, CancellationToken cancellationToken) { if (additionalFiles is null) { @@ -50,11 +54,11 @@ internal static async Task> ReadAdditionalFilesAsync(IEnu let fileName = Path.GetFileName(doc.Name) where fileNamePattern.IsMatch(fileName) select doc; - ImmutableArray.Builder? result = ImmutableArray.CreateBuilder(); + ImmutableArray.Builder? result = ImmutableArray.CreateBuilder(); foreach (TextDocument? doc in docs) { - SourceText? text = await doc.GetTextAsync(cancellationToken); - result.AddRange(ReadLinesFromAdditionalFile(text)); + SourceText text = await doc.GetTextAsync(cancellationToken); + result.Add(text); } return result.ToImmutable(); diff --git a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs index b4fdfc07a..282353e1c 100644 --- a/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs +++ b/src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs @@ -40,7 +40,9 @@ public static class CommonInterest new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task)), nameof(Task.WaitAny)), null), new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemRuntimeCompilerServices, nameof(ConfiguredTaskAwaitable.ConfiguredTaskAwaiter)), nameof(ConfiguredTaskAwaitable.ConfiguredTaskAwaiter.GetResult)), null), new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemRuntimeCompilerServices, nameof(TaskAwaiter)), nameof(TaskAwaiter.GetResult)), null), + new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemRuntimeCompilerServices, nameof(TaskAwaiter) + "`1"), nameof(TaskAwaiter.GetResult)), null), new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemRuntimeCompilerServices, nameof(ValueTaskAwaiter)), nameof(ValueTaskAwaiter.GetResult)), null), + new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemRuntimeCompilerServices, nameof(ValueTaskAwaiter) + "`1"), nameof(ValueTaskAwaiter.GetResult)), null), new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemRuntimeCompilerServices, nameof(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter)), nameof(ConfiguredValueTaskAwaitable.ConfiguredValueTaskAwaiter.GetResult)), null), ]; @@ -53,7 +55,9 @@ public static class CommonInterest public static readonly ImmutableArray SyncBlockingProperties = [ new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task)), nameof(Task.Result)), null), + new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task) + "`1"), nameof(Task.Result)), null), new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(ValueTask)), nameof(ValueTask.Result)), null), + new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(ValueTask) + "`1"), nameof(ValueTask.Result)), null), ]; public static readonly IEnumerable ThreadAffinityTestingMethods = @@ -63,6 +67,7 @@ public static class CommonInterest public static readonly ImmutableArray TaskConfigureAwait = ImmutableArray.Create( new QualifiedMember(new QualifiedType(Types.Task.Namespace, Types.Task.TypeName), nameof(Task.ConfigureAwait)), + new QualifiedMember(new QualifiedType(Types.Task.Namespace, Types.Task.TypeName + "`1"), nameof(Task.ConfigureAwait)), new QualifiedMember(new QualifiedType(Types.AwaitExtensions.Namespace, Types.AwaitExtensions.TypeName), Types.AwaitExtensions.ConfigureAwaitRunInline)); private const RegexOptions FileNamePatternRegexOptions = RegexOptions.IgnoreCase | RegexOptions.Singleline; @@ -71,25 +76,33 @@ public static class CommonInterest public static IEnumerable ReadMethods(AnalyzerOptions analyzerOptions, Regex fileNamePattern, CancellationToken cancellationToken) { - foreach (string line in ReadAdditionalFiles(analyzerOptions, fileNamePattern, cancellationToken)) + foreach (SourceText text in ReadAdditionalFileTexts(analyzerOptions, fileNamePattern, cancellationToken)) { - yield return ParseAdditionalFileMethodLine(line); + bool matchAnyArity = !Contains(text, '`'); + foreach (string line in ReadLinesFromAdditionalFile(text)) + { + yield return ParseAdditionalFileMethodLine(line, matchAnyArity); + } } } public static IEnumerable ReadTypesAndMembers(AnalyzerOptions analyzerOptions, Regex fileNamePattern, CancellationToken cancellationToken) { - foreach (string line in ReadAdditionalFiles(analyzerOptions, fileNamePattern, cancellationToken)) + foreach (SourceText text in ReadAdditionalFileTexts(analyzerOptions, fileNamePattern, cancellationToken)) { - if (!CommonInterestParsing.TryParseNegatableTypeOrMemberReference(line, out bool negated, out ReadOnlyMemory typeNameMemory, out string? memberNameValue)) + bool matchAnyArity = !Contains(text, '`'); + foreach (string line in ReadLinesFromAdditionalFile(text)) { - throw new InvalidOperationException($"Parsing error on line: {line}"); - } + if (!CommonInterestParsing.TryParseNegatableTypeOrMemberReference(line, out bool negated, out ReadOnlyMemory typeNameMemory, out string? memberNameValue)) + { + throw new InvalidOperationException($"Parsing error on line: {line}"); + } - (ImmutableArray containingNamespace, string? typeName) = SplitQualifiedIdentifier(typeNameMemory); - var type = new QualifiedType(containingNamespace, typeName); - QualifiedMember member = memberNameValue is not null ? new QualifiedMember(type, memberNameValue) : default(QualifiedMember); - yield return new TypeMatchSpec(type, member, negated); + (ImmutableArray containingNamespace, string? typeName) = SplitQualifiedIdentifier(typeNameMemory); + var type = new QualifiedType(containingNamespace, typeName, matchAnyArity); + QualifiedMember member = memberNameValue is not null ? new QualifiedMember(type, memberNameValue) : default(QualifiedMember); + yield return new TypeMatchSpec(type, member, negated); + } } } @@ -105,12 +118,7 @@ public static IEnumerable ReadAdditionalFiles(AnalyzerOptions analyzerOp throw new ArgumentNullException(nameof(fileNamePattern)); } - IEnumerable? docs = from file in analyzerOptions.AdditionalFiles.OrderBy(x => x.Path, StringComparer.Ordinal) - let fileName = Path.GetFileName(file.Path) - where fileNamePattern.IsMatch(fileName) - let text = file.GetText(cancellationToken) - select text; - return docs.SelectMany(ReadLinesFromAdditionalFile); + return ReadAdditionalFileTexts(analyzerOptions, fileNamePattern, cancellationToken).SelectMany(ReadLinesFromAdditionalFile); } public static bool Contains(this ImmutableArray methods, ISymbol symbol) @@ -320,6 +328,9 @@ public static IEnumerable ReadLinesFromAdditionalFile(SourceText text) } public static QualifiedMember ParseAdditionalFileMethodLine(string line) + => ParseAdditionalFileMethodLine(line, matchAnyArity: true); + + public static QualifiedMember ParseAdditionalFileMethodLine(string line, bool matchAnyArity) { if (!CommonInterestParsing.TryParseMemberReference(line, out ReadOnlyMemory typeNameMemory, out string? memberName)) { @@ -327,10 +338,34 @@ public static QualifiedMember ParseAdditionalFileMethodLine(string line) } (ImmutableArray containingNamespace, string? typeName) = SplitQualifiedIdentifier(typeNameMemory); - var containingType = new QualifiedType(containingNamespace, typeName); + var containingType = new QualifiedType(containingNamespace, typeName, matchAnyArity); return new QualifiedMember(containingType, memberName!); } + /// + /// Determines whether a character appears in source text without materializing the text as a string. + /// + /// The source text to search. + /// The character to find. + /// if appears in ; otherwise, . + public static bool Contains(SourceText text, char value) + { + if (text is null) + { + throw new ArgumentNullException(nameof(text)); + } + + for (int i = 0; i < text.Length; i++) + { + if (text[i] == value) + { + return true; + } + } + + return false; + } + /// /// Splits a qualified type name (e.g. My.Namespace.MyType) into its containing namespace /// segments and the simple type name, without allocating an intermediate joined string. @@ -364,6 +399,25 @@ private static (ImmutableArray ContainingNamespace, string TypeName) Spl return (nsBuilder.ToImmutable(), typeName); } + private static IEnumerable ReadAdditionalFileTexts(AnalyzerOptions analyzerOptions, Regex fileNamePattern, CancellationToken cancellationToken) + { + if (analyzerOptions is null) + { + throw new ArgumentNullException(nameof(analyzerOptions)); + } + + if (fileNamePattern is null) + { + throw new ArgumentNullException(nameof(fileNamePattern)); + } + + return from file in analyzerOptions.AdditionalFiles.OrderBy(x => x.Path, StringComparer.Ordinal) + let fileName = Path.GetFileName(file.Path) + where fileNamePattern.IsMatch(fileName) + let text = file.GetText(cancellationToken) ?? throw new InvalidOperationException($"Unable to read additional file: {file.Path}") + select text; + } + private static bool TestGetAwaiterMethod(IMethodSymbol getAwaiterMethod) { if (getAwaiterMethod.IsExtensionMethod) @@ -444,16 +498,14 @@ public bool IsMatch([NotNullWhen(true)] ITypeSymbol? typeSymbol, ISymbol? member } if (!this.IsMember - && (this.IsWildcard || typeSymbol.Name == this.Type.Name) - && typeSymbol.BelongsToNamespace(this.Type.Namespace)) + && ((this.IsWildcard && typeSymbol.BelongsToNamespace(this.Type.Namespace)) || this.Type.IsMatch(typeSymbol))) { return true; } if (this.IsMember && memberSymbol?.Name == this.Member.Name - && typeSymbol.Name == this.Type.Name - && typeSymbol.BelongsToNamespace(this.Type.Namespace)) + && this.Type.IsMatch(typeSymbol)) { return true; } @@ -465,18 +517,33 @@ public bool IsMatch([NotNullWhen(true)] ITypeSymbol? typeSymbol, ISymbol? member public readonly struct QualifiedType { public QualifiedType(ImmutableArray containingTypeNamespace, string typeName) + : this(containingTypeNamespace, typeName, matchAnyArity: false) + { + } + + /// + /// Initializes a new instance of the struct. + /// + /// The namespace containing the type. + /// The simple or metadata name of the type. + /// to match the simple name across all generic arities; otherwise, to match the metadata name exactly. + internal QualifiedType(ImmutableArray containingTypeNamespace, string typeName, bool matchAnyArity) { this.Namespace = containingTypeNamespace; this.Name = typeName; + this.MatchAnyArity = matchAnyArity; } public ImmutableArray Namespace { get; } public string Name { get; } + private bool MatchAnyArity { get; } + public bool IsMatch(ISymbol symbol) { - return symbol?.Name == this.Name + return symbol is not null + && (this.MatchAnyArity ? symbol.Name : symbol.MetadataName) == this.Name && symbol.BelongsToNamespace(this.Namespace); } diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.legacy.txt b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.legacy.txt new file mode 100644 index 000000000..c43316883 --- /dev/null +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.legacy.txt @@ -0,0 +1,2 @@ +# This file has no metadata arity markers, so its type names match every arity. +[TestNamespace.LegacyTestClass]::SlowSyncMethod diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.mocks.txt b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.mocks.txt index 491c94c55..6ab299c75 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.mocks.txt +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.mocks.txt @@ -1,2 +1,3 @@ # Test exclusions for VSTHRD103 analyzer -[TestNamespace.TestClass]::SlowSyncMethod \ No newline at end of file +[TestNamespace.TestClass]::SlowSyncMethod +[TestNamespace.GenericTestClass`1]::SlowSyncMethod \ No newline at end of file diff --git a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs index 0e27c2c55..524957dd8 100644 --- a/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs +++ b/test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs @@ -1363,6 +1363,92 @@ public static void SlowSyncMethod() { } await CSVerify.VerifyAnalyzerAsync(test); } + [Fact] + public async Task GenericTypeExclusion_DoesNotExcludeNonGenericType() + { + string test = """ + using System.Threading.Tasks; + + class Test { + async Task T() { + TestNamespace.GenericTestClass.SlowSyncMethod(); + TestNamespace.GenericTestClass.{|#0:SlowSyncMethod|}(); + } + } + + namespace TestNamespace { + class GenericTestClass { + public static void SlowSyncMethod() { } + public static Task SlowSyncMethodAsync() => Task.CompletedTask; + } + + class GenericTestClass { + public static void SlowSyncMethod() { } + public static Task SlowSyncMethodAsync() => Task.CompletedTask; + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(test, CSVerify.Diagnostic(Descriptor).WithLocation(0).WithArguments("SlowSyncMethod", "SlowSyncMethodAsync")); + } + + [Fact] + public async Task NonGenericTypeExclusion_DoesNotExcludeGenericType() + { + string test = """ + using System.Threading.Tasks; + + class Test { + async Task T() { + TestNamespace.TestClass.{|#0:SlowSyncMethod|}(); + } + } + + namespace TestNamespace { + class TestClass { + public static void SlowSyncMethod() { } + public static Task SlowSyncMethodAsync() => Task.CompletedTask; + } + + class TestClass { + public static void SlowSyncMethod() { } + public static Task SlowSyncMethodAsync() => Task.CompletedTask; + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(test, CSVerify.Diagnostic(Descriptor).WithLocation(0).WithArguments("SlowSyncMethod", "SlowSyncMethodAsync")); + } + + [Fact] + public async Task LegacyFileExclusion_MatchesAllArities() + { + string test = """ + using System.Threading.Tasks; + + class Test { + async Task T() { + TestNamespace.LegacyTestClass.SlowSyncMethod(); + TestNamespace.LegacyTestClass.SlowSyncMethod(); + } + } + + namespace TestNamespace { + class LegacyTestClass { + public static void SlowSyncMethod() { } + public static Task SlowSyncMethodAsync() => Task.CompletedTask; + } + + class LegacyTestClass { + public static void SlowSyncMethod() { } + public static Task SlowSyncMethodAsync() => Task.CompletedTask; + } + } + """; + + await CSVerify.VerifyAnalyzerAsync(test); + } + [Fact] public async Task SyncMethodCallInAsyncMethod_NotExcludedViaAdditionalFiles_GeneratesWarning() {