Skip to content

Match configured generic types by metadata name - #1631

Open
Andrew Arnott (AArnott) wants to merge 7 commits into
mainfrom
aarnott-use-metadata-type-names
Open

Match configured generic types by metadata name#1631
Andrew Arnott (AArnott) wants to merge 7 commits into
mainfrom
aarnott-use-metadata-type-names

Conversation

@AArnott

@AArnott Andrew Arnott (AArnott) commented Aug 7, 2026

Copy link
Copy Markdown
Member

VSTHRD103 exclusions could not distinguish a non-generic type from a generic type with the same simple name, and CLR metadata names such as DbSet``1 did not match generic symbols.

This change adds per-file compatibility modes for analyzer configuration:

  • Files containing a backtick use exact metadata type names, including generic arity.
  • Legacy files without a backtick continue matching simple type names across all arities.
  • Version detection is performed independently for each AdditionalFile, without materializing the whole SourceText as a string.

Tests cover exact generic/non-generic separation, legacy matching across arities, and mixed legacy/v2 files in one compilation. The configuration documentation now explains both modes and includes an Entity Framework DbSet example.

Preserve legacy arity-agnostic matching for configuration files without backticks while allowing exact metadata-name matching in versioned files.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates analyzer configuration parsing so configured type matches can distinguish generic vs non-generic types (and match CLR metadata names that include generic arity, e.g. DbSet`1). It introduces per-additional-file compatibility: files containing a backtick opt into exact metadata-name matching, while legacy files without backticks continue matching simple type names across all arities.

Changes:

  • Add per-additional-file “legacy vs metadata-name” detection based on whether the file contains a backtick, without materializing SourceText to a single string.
  • Update type matching to use MetadataName for exact matching and expand built-in known types to include generic metadata forms (e.g., Task/Task\1`).
  • Add tests and documentation covering generic/non-generic separation and legacy matching behavior.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/VSTHRD103UseAsyncOptionAnalyzerTests.cs Adds tests validating generic vs non-generic exclusion behavior and legacy matching across arities.
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.mocks.txt Updates mock exclusions to include a generic type entry using metadata arity.
test/Microsoft.VisualStudio.Threading.Analyzers.Tests/AdditionalFiles/vs-threading.SyncMethodsToExcludeFromVSTHRD103.legacy.txt Adds a legacy-mode exclusions file (no backticks) to validate “match all arities” behavior.
src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs Implements per-file mode detection and metadata-name matching; updates parsing/matching utilities accordingly.
docfx/analyzers/configuration.md Documents the two compatibility modes and provides a generic type example using metadata arity.
Suppressed comments (1)

src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs:500

  • The member-match branch in TypeMatchSpec.IsMatch also hard-codes typeSymbol.MetadataName == this.Type.Name, which again ignores matchAnyArity from the additional file. Switching to this.Type.IsMatch(typeSymbol) keeps namespace checking and applies the correct name comparison for legacy vs metadata-name modes.
            if (this.IsMember
                && memberSymbol?.Name == this.Member.Name
                && typeSymbol.MetadataName == this.Type.Name
                && typeSymbol.BelongsToNamespace(this.Type.Namespace))
            {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs
Comment thread src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs
Honor legacy arity matching for all configured type paths and surface unreadable additional files explicitly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 18:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs:331

  • ParseAdditionalFileMethodLine(string) now hard-codes matchAnyArity: false, which changes behavior for callers that parse additional-file lines without the per-file backtick scan (e.g., CommonFixes.ReadMethodsAsync). This will stop legacy entries like [Namespace.Type]::Method from matching generic arities (since QualifiedType.IsMatch now compares MetadataName). Consider inferring matchAnyArity in this wrapper when no backtick is present, so legacy callers keep matching across arities while ReadMethods/ReadTypesAndMembers still enforce per-file mode.
    public static QualifiedMember ParseAdditionalFileMethodLine(string line)
        => ParseAdditionalFileMethodLine(line, matchAnyArity: false);

Apply per-file metadata-name mode detection when code fixes read analyzer configuration, while retaining legacy behavior for individual line parsing.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 18:31

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs:60

  • SyncBlockingProperties still includes non-generic Task/ValueTask entries for the Result member. Since QualifiedType now defaults to exact MetadataName matching, these entries can never match (only Task1/ValueTask1 define Result). Keeping them is dead configuration and adds confusion/extra matching work.
        new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task)), nameof(Task<int>.Result)), null),
        new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task) + "`1"), nameof(Task<int>.Result)), null),
        new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(ValueTask)), nameof(ValueTask<int>.Result)), null),
        new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(ValueTask) + "`1"), nameof(ValueTask<int>.Result)), null),

Reuse the allocation-free SourceText search helper across analyzer and code-fix configuration readers.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 18:42
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs:332

  • ParseAdditionalFileMethodLine(string line) now infers matchAnyArity from whether that line contains a backtick. That’s inconsistent with the new per-file compatibility mode (a backtick anywhere in the file opts the whole file into metadata-name matching), and it also changes the historical behavior of this public helper to start matching simple names across all arities when the line has no backtick. Consider keeping this overload as the strict/metadata default and requiring callers to pass the per-file matchAnyArity explicitly via the new overload.
    public static QualifiedMember ParseAdditionalFileMethodLine(string line)
        => ParseAdditionalFileMethodLine(line, matchAnyArity: line.IndexOf('`') < 0);

Reserve exact metadata-name matching for callers that can provide the per-file mode explicitly.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 18:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Suppressed comments (2)

src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs:60

  • SyncBlockingProperties includes entries for Task and ValueTask with member Result. With the updated QualifiedType.IsMatch using MetadataName by default, these entries can no longer match Task<TResult>.Result / ValueTask<TResult>.Result (and the non-generic types don’t define Result anyway), so they are now dead and potentially misleading. Consider removing the non-generic entries and keeping only the *1` metadata-name entries.
        new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task)), nameof(Task<int>.Result)), null),
        new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(Task) + "`1"), nameof(Task<int>.Result)), null),
        new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(ValueTask)), nameof(ValueTask<int>.Result)), null),
        new SyncBlockingMethod(new QualifiedMember(new QualifiedType(Namespaces.SystemThreadingTasks, nameof(ValueTask) + "`1"), nameof(ValueTask<int>.Result)), null),

src/Microsoft.VisualStudio.Threading.Analyzers/CommonInterest.cs:355

  • Contains(SourceText text, char value) is a new public API but doesn’t guard against text == null, which will currently surface as a NullReferenceException. Other public helpers in this type (e.g. ReadLinesFromAdditionalFile) throw ArgumentNullException for null inputs; consider doing the same here for consistency and clearer failures.
    public static bool Contains(SourceText text, char value)
    {
        for (int i = 0; i < text.Length; i++)
        {
            if (text[i] == value)

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 7, 2026 18:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants