Skip to content

Expose more of Roslyn to the XPath queries of MA0240 - #1544

Merged
meziantou merged 2 commits into
mainfrom
feature/roslyn-xpath-exposure-5f6e66
Sep 22, 2026
Merged

meziantou merged 2 commits into
mainfrom
feature/roslyn-xpath-exposure-5f6e66

Conversation

@meziantou

Copy link
Copy Markdown
Owner

What

Adds the members that were missing from the data an MA0240 banned syntax query can select, and fixes two inconsistencies in how the operations are exposed.

The kind of a type

Each of the four type prefixes gains a …Kind suffix (semantic:TypeKind, semantic:ConvertedTypeKind, semantic:ReturnTypeKind, semantic:ContainingTypeKind, and @PKind for the type properties of the operations). Only IsValueType and SpecialType were exposed, so a query could not select the interfaces, the enumerations or the delegates:

//Parameter/*[@semantic:TypeKind='Interface']; Do not take an interface

The attribute is not present for the types that do not compile (Unknown and Error), like semantic:TypeSpecialType is not present for the types that are not special.

The modifiers of a symbol

IsAbstract, IsVirtual, IsOverride, IsSealed, IsAsync, IsExtensionMethod and Arity, on the semantic attributes and on the symbol properties of the operations (@TargetMethodIsAbstract, @TargetMethodArity, ...). Only IsStatic was exposed:

//MethodDeclaration[@semantic:IsAsync='true' and @semantic:ReturnTypeSpecialType='System_Void']; Do not write async void
//operation:Invocation[@TargetMethodIsExtensionMethod='true']

The modifiers that only a method or a type has are not exposed for the other symbols, so [not(@semantic:IsAsync)] selects the nodes whose symbol is not a method, whereas [@semantic:IsAsync='false'] selects the ones whose symbol is a method that is not async. The logic is shared by the two navigators (XPathAttributeFormatter.AddSymbolModifiers), so the two lists cannot drift apart.

Two inconsistencies of the operations

  • A property that returns several symbols, such as Locals or InitializedFields, only had @P and @PName. It now also has @PDocumentationId.
  • A property that returns several types was exposed as the symbols that are not types, so it had no metadata name and no reference id. ImmutableArray<ITypeSymbol> is now matched before ImmutableArray<ISymbol> and exposes @PName, @PMetadataName, @PDocumentationId and @PReferenceId.
  • CommonConversion was dropped by CreateProperty, so IConversionOperation.Conversion was unreachable and a query could not tell a user-defined conversion from a numeric one. It now exposes @PExists, @PIsIdentity, @PIsImplicit, @PIsNullable, @PIsNumeric, @PIsReference, @PIsUserDefined, and the operator as the symbol @PMethod:
//operation:Conversion[@ConversionIsUserDefined='true']; Do not rely on the implicit operators

This covers IConversionOperation.Conversion, the two conversions of IArgumentOperation and of ICompoundAssignmentOperation, ICoalesceOperation.ValueConversion, and ISpreadOperation.ElementConversion on the versions of Roslyn that have it. IsUnion is not exposed, as it only exists from Roslyn 5.9 and an entry must mean the same thing in every supported version.

Why

The operations are exposed by reflection, so they follow the API of Roslyn, but the semantic attributes are a hand-written list that was missing the modifiers and the kind of a type. The attributes are only computed when a query can select them (XPathAttributeFilter), so the entries that do not use the new attributes pay nothing for them. The list properties declare only the names they can produce, so asking for @PIsStatic on a Locals no longer costs a reflection call.

Breaking change

@TypeArguments becomes @TypeArgumentsMetadataName, as a list of types is now exposed like a single type, which has no bare @P. It only applies to IDynamicMemberReferenceOperation.TypeArguments, an attribute that was introduced recently and that operation:DynamicMemberReference is the only element to have.

Testing

  • 18 new tests in DoNotUseBannedSyntaxAnalyzerTests, which bring the MA0240/MA0241 tests to 153. They pass on roslyn4.8, roslyn4.14, roslyn5.0, roslyn5.6 and roslyn5.9.
  • The whole roslyn5.9 test project passes (4953 tests).
  • dotnet run --project src/DocumentationGenerator exits 0 and changes no markdown file beyond the edits to docs/Rules/MA0240.md that are in this PR.

Add the members that were missing from the data the banned syntax
queries can select. The attributes are only computed when a query can
select them, so the new ones cost nothing to the entries that do not
use them.

- Expose the kind of a type as the '...Kind' suffix, so a query can
  select the interfaces, the enumerations or the delegates instead of
  only telling a value type from a reference type.
- Expose the modifiers of a symbol: IsAbstract, IsVirtual, IsOverride,
  IsSealed, IsAsync, IsExtensionMethod and Arity. The modifiers that
  only a method or a type has are not exposed for the other symbols, so
  an attribute that is not present means that the modifier does not
  apply.
- Expose the documentation comment ids of the properties that return
  several symbols, which only had the qualified names and the names.
- Expose a property that returns several types with the names a type
  has, as the types were exposed as the symbols that are not types.
  '@TypeArguments' is therefore '@TypeArgumentsMetadataName'.
- Expose the CommonConversion of an operation, which was dropped, so a
  query can tell a user-defined conversion from a numeric one and reach
  the operator it calls. The members that are not in every supported
  version of Roslyn, such as IsUnion, are not exposed.
The tests that use a source generator of the .NET reference pack failed
on a run with a ReflectionTypeLoadException:

  Could not load file or assembly 'Microsoft.Interop.SourceGeneration'

GeneratorAssemblyLoader registered the path of an assembly just before
loading it, so an assembly could only resolve the dependencies that were
loaded before it. Microsoft.Interop.ComInterfaceGenerator and
Microsoft.Interop.LibraryImportGenerator depend on
Microsoft.Interop.SourceGeneration, which sorts after them, and the
assemblies are enumerated with Directory.EnumerateFiles, whose order
depends on the file system. The load therefore succeeded or failed
depending on the machine, which is why only one leg of the CI matrix
failed.

Register the paths of the whole pack in the constructor, before any
assembly is loaded, so the resolution no longer depends on the order.
@meziantou
meziantou merged commit 38a4584 into main Sep 22, 2026
14 checks passed
@meziantou
meziantou deleted the feature/roslyn-xpath-exposure-5f6e66 branch September 22, 2026 20:36
This was referenced Sep 22, 2026
This was referenced Sep 28, 2026
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.

1 participant