Query the operations with the XPath queries of MA0240

[meziantou]

The XPath queries of the banned syntax files could only be evaluated on the syntax tree. Boxing, an implicit conversion, a virtual call or a constant folded by the compiler are properties of the operations, and a syntactic query can only approximate them. This exposes the operations as a second document, queryable with the same file format, the same severities and the same MA0241 validation.

//operation:Conversion[@IsImplicit='true' and @TypeMetadataName='System.Object']; Do not box values
//operation:Invocation[@TargetMethodDocumentationId='M:System.Console.WriteLine(System.String)']; Use the logger
//operation:Loop//operation:Invocation[@TargetMethodName='Query']; Do not query in a loop

What changed

The operation document. Each operation is an element of the operation namespace named after its OperationKind, and the children of an element are the child operations. The operations of a file form a forest (method bodies, field/property/parameter initializers, attributes), so the document has one element per root.

The attributes are unprefixed and named after the properties of the operation interfaces (@IsImplicit, @OperatorKind, @TargetMethod). They are discovered by reflecting over the interfaces the operation implements, because every implementation in Microsoft.CodeAnalysis is an internal type. A property returning a type expands to the seven suffixes the semantic: attributes already use, another symbol to five, and ConstantValue to @HasConstantValue / @ConstantValue.

semantic: is deliberately not exposed on the operations: that vocabulary was built on GetTypeInfo / GetSymbolInfo of a syntax node and half of it has no meaning there. A conversion is a node, so ConvertedType has no equivalent, and Type would get a second name. An entry is evaluated on one document, and using both prefixes is reported by MA0241.

The syntax function returns the syntax nodes of the operations it is given, so a query on the operations can continue on the syntax tree and reach what only the syntax has:

syntax(//operation:Invocation)//StringLiteralExpression/@Token[starts-with(., '@')]
//operation:Invocation[syntax(.)//InterpolatedStringExpression]; Do not interpolate the arguments

A function call cannot be a step of a path, so the function takes the operations as its argument and the path continues after the call. syntax(.) in a predicate is the node of the operation being tested.

A Name attribute was added everywhere a symbol is exposed, on both documents: semantic:TypeName, semantic:ConvertedTypeName, semantic:ReturnTypeName, semantic:ContainingTypeName, semantic:SymbolName, semantic:ContainingSymbolName, and @TargetMethodName, @TypeName and friends on the operations.

Notes for the reviewer

Two findings are worth a look, as they are behaviour and not refactoring.

OperationKind has members that share a value, and the name ToString returns for such a value is not the same in every version of Roslyn: OperationKind.Binary.ToString() is "BinaryOperator" on Roslyn 4.8 and "Binary" on 5.9. The elements are therefore named after the current name of the kind, derived from the members of the enumeration, so //operation:Binary means the same thing on every version. The obsolete alias is reported by MA0241 instead of silently selecting nothing. The multi-version CI is what caught this.

An expression that is not compiled with an XsltContext cannot use a function of its own, whatever the context it is evaluated with, so the queries are now compiled with one. A consequence is that this context resolves the prefixes when the query is evaluated rather than when it is compiled, so an undefined prefix no longer throws at compile time: it is now reported by the scanner, which already walks every prefix, with a better message than the engine produced.

Cost. The rule still does nothing for the projects without a banned syntax file, and the operations are only built for the files analysed by an entry that uses the operation prefix. The navigator of the syntax tree used by the syntax function is only built when an entry calls it. The attributes of an operation are computed once per file, and the reflection is done once per operation type per process.

Refactoring. SyntaxNodeXPathNavigator lost the pieces the two navigators share (the attribute record, the type names, the formatters, the namespaces), which moved to their own files. Its behaviour is unchanged, and the existing tests of MA0240 are the regression signal.

Validation

• dotnet test on the five Roslyn versions: 24 335 tests, all passing.
• 47 tests added to DoNotUseBannedSyntaxAnalyzerTests, including a smoke test that computes the attributes of every kind of operation over a file exercising every node kind, and a test for each query printed in the documentation.
• dotnet run --project src/DocumentationGenerator reports no pending change.