fix: align compilers, analyzers and the decompiler with what the API Management gateway accepts

docs/ExpressionHelpers.md

@@ -94,6 +94,36 @@ public static class TextHelpers
 Helpers are inlined from source, so the library must be a project reference. Calling a helper, property or
 non-constant field of a compiled library is reported, although its constants can still be used.
 
+## What the analyzer checks
+
+The analyzer reports code that API Management rejects when the policy is saved: types and members outside the set it
+allows in policy expressions (`APIM001`, `APIM002`), and type names it can't resolve on their own (`APIM003`).
+
+It checks:
+
+- every helper of a policy document or fragment class, including helpers in a class nested inside it. A helper is any
+  method that returns a value, other than a policy configuration factory and a method that takes a policy section
+  context. The `[Expression]` attribute isn't needed for these.
+- methods marked `[Expression]` and every method of a class marked `[Expression]`.
+- expression lambdas passed to a policy.
+
+A helper in any other class is only checked when its class is marked `[Expression]`. Code marked `[Expression]` may
+only call document helpers and other marked helpers, while an unmarked document helper may call any helper declared in
+source.
+
+A method of a document class that isn't an expression helper is checked as well, so keep such methods in another
+class. Properties declared in source can't be used as helpers; the compiler reports them.
+
+### Ambiguous type names
+
+API Management resolves type names against all the namespaces it imports, whatever the `using` directives of the
+source file are. `Formatting` exists in both `Newtonsoft.Json` and `System.Xml`, so it has to be written with its
+namespace:
+
+```csharp
+JsonConvert.SerializeObject(value, Newtonsoft.Json.Formatting.Indented)
+```
+
 ## Named values
 
 `context.NamedValue("name")` compiles to a raw `{{name}}` token, so the named value is used as code.

src/Analyzers/ExpressionDefinition.Analyzer.cs

@@ -26,6 +26,10 @@ public override void Initialize(AnalysisContext context)
             SyntaxKind.ParenthesizedLambdaExpression);
     }
 
+    // The types an expression helper can be declared to return. The gateway types the expression itself and
+    // does not convert its value, so this follows the types it accepts as the value of a variable. object is
+    // here because the decompiler declares the helper of a set-variable value as object. Nullable and array
+    // forms and enums are handled in IsAllowedReturnType.
     private readonly static IReadOnlyCollection<string> AllowedExpressionReturnTypes = new HashSet<string>()
     {
         "System.Boolean",
@@ -39,14 +43,42 @@ public override void Initialize(AnalysisContext context)
         "System.Int16",
         "System.Int32",
         "System.Int64",
+        "System.Object",
+        "System.SByte",
+        "System.Single",
         "System.String",
+        "System.TimeSpan",
         "System.UInt16",
         "System.UInt32",
         "System.UInt64",
         "System.Uri",
+        "Newtonsoft.Json.Linq.JArray",
+        "Newtonsoft.Json.Linq.JConstructor",
+        "Newtonsoft.Json.Linq.JContainer",
         "Newtonsoft.Json.Linq.JObject",
+        "Newtonsoft.Json.Linq.JProperty",
+        "Newtonsoft.Json.Linq.JRaw",
+        "Newtonsoft.Json.Linq.JToken",
+        "Newtonsoft.Json.Linq.JValue",
     };
 
+    private static bool IsAllowedReturnType(ITypeSymbol type)
+    {
+        switch (type)
+        {
+            case { TypeKind: TypeKind.Enum }:
+                return true;
+            // the gateway accepts string[] and byte[] as the value of an expression, but not int[]
+            case IArrayTypeSymbol { Rank: 1 } array:
+                return array.ElementType.SpecialType is
+                    SpecialType.System_String or SpecialType.System_Byte or SpecialType.System_SByte;
+            case INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T } nullable:
+                return IsAllowedReturnType(nullable.TypeArguments[0]);
+            default:
+                return AllowedExpressionReturnTypes.Contains(type.ToFullyQualifiedString());
+        }
+    }
+
     private const string ContextParamType =
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IExpressionContext";
 
@@ -74,7 +106,7 @@ private static void AnalyzeMethod(SyntaxNodeAnalysisContext context)
         else
         {
             var fullTypeName = type.ToFullyQualifiedString();
-            if (!AllowedExpressionReturnTypes.Contains(fullTypeName))
+            if (!IsAllowedReturnType(type))
             {
                 var diagnostic = Diagnostic.Create(Rules.Expression.ReturnTypeNotAllowed,
                     method.ReturnType.GetLocation(), fullTypeName);

src/Analyzers/ExpressionTypeUsed.Analyzer.cs

@@ -27,6 +27,35 @@ public override void Initialize(AnalysisContext context)
             SyntaxKind.ObjectCreationExpression,
             SyntaxKind.ObjectInitializerExpression,
             SyntaxKind.AnonymousObjectCreationExpression);
+        context.RegisterSyntaxNodeAction(AnalyzeTypeName, SyntaxKind.IdentifierName);
+    }
+
+    // API Management resolves type names against every namespace it imports, so a name declared in two of them is
+    // rejected unless it is written with its namespace, whatever the using directives of the source file are.
+    private readonly static IReadOnlyDictionary<string, IReadOnlyCollection<string>> AmbiguousTypeNames =
+        new Dictionary<string, IReadOnlyCollection<string>>()
+        {
+            { "Formatting", new HashSet<string>() { "Newtonsoft.Json.Formatting", "System.Xml.Formatting" } },
+        };
+
+    private static void AnalyzeTypeName(SyntaxNodeAnalysisContext context)
+    {
+        var name = (IdentifierNameSyntax)context.Node;
+        if (!AmbiguousTypeNames.TryGetValue(name.Identifier.ValueText, out var candidates) ||
+            name.Parent is MemberAccessExpressionSyntax memberAccess && memberAccess.Name == name ||
+            name.Parent is QualifiedNameSyntax qualifiedName && qualifiedName.Right == name ||
+            name.Parent is AliasQualifiedNameSyntax ||
+            !IsPartOfPolicyExpression(context))
+        {
+            return;
+        }
+
+        if (context.SemanticModel.GetSymbolInfo(name).Symbol is INamedTypeSymbol type &&
+            candidates.Contains(type.ToFullyQualifiedString()))
+        {
+            context.ReportDiagnostic(Diagnostic.Create(Rules.TypeUsed.AmbiguousTypeName, name.GetLocation(),
+                name.Identifier.ValueText, type.ToFullyQualifiedString()));
+        }
     }
 
     private readonly static IReadOnlyCollection<string> AllowedTypes = new HashSet<string>()
@@ -174,12 +203,21 @@ public override void Initialize(AnalysisContext context)
 
         #region Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions
 
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.Authorization",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.BasicAuthCredentials",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.ByteArrayExtensions",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.CarbonIntensityCategory",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.DictionaryExtensions",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IFoundry",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.ISustainabilityInfo",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.X509Certificate2Extensions",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IApi",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IExpressionContext",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IContextApi",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IBackend",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IDeployment",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IGateway",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IWorkspace",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IGroup",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.Jwt",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.ILastError",
@@ -188,6 +226,7 @@ public override void Initialize(AnalysisContext context)
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IOperation",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IPrivateEndpointConnection",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IProduct",
+        "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.ProductState",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IRequest",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.IResponse",
         "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring.Expressions.ISubscription",
@@ -204,6 +243,9 @@ public override void Initialize(AnalysisContext context)
         new Dictionary<string, IReadOnlyCollection<string>>()
         {
             { "Newtonsoft.Json.JsonConvert", new HashSet<string>() { "SerializeObject", "DeserializeObject" } },
+            // The gateway accepts the type but none of its members: ToString(), Equals() or GetType() called on
+            // a value typed object, or on a type that doesn't declare them itself, is rejected.
+            { "System.Object", new HashSet<string>() },
             {
                 "System.DateTime", new HashSet<string>()
                 {
@@ -287,10 +329,7 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
     {
         var node = context.Node;
 
-        var underUnderExpressionMethod = node.IsPartOfPolicyExpressionMethod(context.SemanticModel);
-        var underUnderExpressionLambda = node.IsPartOfPolicyExpressionDelegate(context.SemanticModel);
-
-        if (!underUnderExpressionMethod && !underUnderExpressionLambda)
+        if (!IsPartOfPolicyExpression(context))
         {
             return;
         }
@@ -307,6 +346,16 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
             return;
         }
 
+        // Invoking a delegate typed member, like context.Trace("message"), is a use of that member and not of the
+        // delegate type.
+        if (nodeSymbol is IMethodSymbol { MethodKind: MethodKind.DelegateInvoke } &&
+            node is InvocationExpressionSyntax delegateInvocation &&
+            context.SemanticModel.GetSymbolInfo(delegateInvocation.Expression).Symbol is
+                { Kind: SymbolKind.Property or SymbolKind.Field } delegateMember)
+        {
+            nodeSymbol = delegateMember;
+        }
+
         // Members of an expression helper library ([Expression] on the class) are analysed in their own project, so
         // calls to its methods and uses of its constants are allowed, from source or a referenced assembly.
         if (nodeSymbol.IsExpressionLibraryMember() &&
@@ -316,10 +365,15 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
             return;
         }
 
-        // Calls to [Expression] helpers (also from a referenced assembly) are expanded by the compiler, and source
-        // constants are folded into literals, except values of source enums, which don't exist in API Management. A
-        // helper passed as a method group isn't expanded.
-        if (nodeSymbol is IMethodSymbol && node is InvocationExpressionSyntax && nodeSymbol.HasExpressionAttribute() ||
+        // Calls to [Expression] helpers (also from a referenced assembly) and to the helpers of a policy document are
+        // expanded by the compiler, and source constants are folded into literals, except values of source enums,
+        // which don't exist in API Management. A helper passed as a method group isn't expanded. A document helper
+        // that isn't marked [Expression] may call any helper declared in source, as it could before it was analysed.
+        if (nodeSymbol is IMethodSymbol && node is InvocationExpressionSyntax &&
+            (nodeSymbol.HasExpressionAttribute() || nodeSymbol.IsDocumentMember() ||
+             !nodeSymbol.DeclaringSyntaxReferences.IsDefaultOrEmpty &&
+             !node.IsPartOfMarkedPolicyExpressionMethod(context.SemanticModel) &&
+             !node.IsPartOfPolicyExpressionDelegate(context.SemanticModel)) ||
             !nodeSymbol.DeclaringSyntaxReferences.IsDefaultOrEmpty &&
             nodeSymbol is IFieldSymbol { IsConst: true, ContainingType.TypeKind: not TypeKind.Enum } field &&
             !(field.Type.TypeKind == TypeKind.Enum && !field.Type.DeclaringSyntaxReferences.IsDefaultOrEmpty))
@@ -333,9 +387,15 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
             return;
         }
 
+        // API Management accepts anonymous types: new { a = 1 }.a
+        if (symbol.IsAnonymousType)
+        {
+            return;
+        }
+
         // Indexers are named "this[]" in Roslyn; the allow lists use their metadata name ("Item").
         var memberName = nodeSymbol is IPropertySymbol { IsIndexer: true } ? nodeSymbol.MetadataName : nodeSymbol.Name;
-        var typeName = (symbol.IsGenericType ? symbol.OriginalDefinition : symbol)?.ToFullyQualifiedString() ?? "";
+        var typeName = GetTypeName(symbol);
         if (AllowedTypes.Contains(typeName))
         {
             if (AllowedInTypes.TryGetValue(typeName, out var allowed) && !allowed.Contains(memberName))
@@ -355,4 +415,24 @@ private static void AnalyzeNode(SyntaxNodeAnalysisContext context)
             context.ReportDiagnostic(Diagnostic.Create(Rules.TypeUsed.DisallowedType, node.GetLocation(), typeName));
         }
     }
+
+    private static bool IsPartOfPolicyExpression(SyntaxNodeAnalysisContext context)
+    {
+        return context.Node.IsPartOfPolicyExpressionMethod(context.SemanticModel) ||
+               context.Node.IsPartOfPolicyExpressionDelegate(context.SemanticModel);
+    }
+
+    private static string GetTypeName(INamedTypeSymbol symbol)
+    {
+        if (!symbol.IsGenericType)
+        {
+            return symbol.ToFullyQualifiedString();
+        }
+
+        // System.Nullable and System.Tuple are listed without type parameters and are allowed with any number of them.
+        var nameWithoutTypeParameters = symbol.OriginalDefinition.ToFullyQualifiedStringWithoutTypeParameters();
+        return AllowedTypes.Contains(nameWithoutTypeParameters)
+            ? nameWithoutTypeParameters
+            : symbol.OriginalDefinition.ToFullyQualifiedString();
+    }
 }

src/Analyzers/ExpressionTypeUsed.Rules.cs

@@ -33,9 +33,19 @@ public static class TypeUsed
             helpLinkUri: "TODO",
             customTags: new[] { "APIM", "ApiManagement" });
 
+        public readonly static DiagnosticDescriptor AmbiguousTypeName = new DiagnosticDescriptor(
+            "APIM003",
+            "Ambiguous type name used",
+            "Type name '{0}' is ambiguous in API Management, use '{1}'",
+            "Type",
+            DiagnosticSeverity.Error,
+            isEnabledByDefault: true,
+            description: "Description.",
+            helpLinkUri: "TODO",
+            customTags: new[] { "APIM", "ApiManagement" });
 
         public readonly static ImmutableArray<DiagnosticDescriptor> All = ImmutableArray.Create(
-            DisallowedType, DisallowedMember
+            DisallowedType, DisallowedMember, AmbiguousTypeName
         );
     }
 }

src/Analyzers/Extensions/SymbolExtensions.cs

@@ -14,5 +14,11 @@ public static class SymbolExtensions
         miscellaneousOptions: SymbolDisplayMiscellaneousOptions.EscapeKeywordIdentifiers
     );
 
+    private readonly static SymbolDisplayFormat FormatWithoutTypeParameters =
+        Format.WithGenericsOptions(SymbolDisplayGenericsOptions.None);
+
     public static string ToFullyQualifiedString(this ISymbol symbol) => symbol.ToDisplayString(Format);
+
+    public static string ToFullyQualifiedStringWithoutTypeParameters(this ISymbol symbol) =>
+        symbol.ToDisplayString(FormatWithoutTypeParameters);
 }

src/Analyzers/Extensions/SyntaxExtensions.cs

@@ -36,14 +36,65 @@ public static bool HasExpressionAttribute(this ISymbol symbol)
             .Any(attribute => attribute.AttributeClass?.ToFullyQualifiedString() == ExpressionAttribute);
     }
 
-    // A node inside an [Expression] method, or inside any method of an expression helper library (by symbol, so
-    // every part of a partial class is included).
+    // A node inside an [Expression] method, inside any method of an expression helper library (by symbol, so
+    // every part of a partial class is included), or inside a helper of a policy document or fragment, which the
+    // compiler turns into a policy expression without the attribute.
     public static bool IsPartOfPolicyExpressionMethod(this SyntaxNode syntax, SemanticModel model)
     {
         return syntax.Ancestors()
             .OfType<MethodDeclarationSyntax>()
-            .Any(method => method.AttributeLists.ContainsExpressionAttribute(model) ||
-                           model.GetDeclaredSymbol(method)?.IsExpressionLibraryMember() == true);
+            .Any(method => IsMarkedExpressionMethod(method, model) ||
+                           model.GetDeclaredSymbol(method) is IMethodSymbol symbol &&
+                           symbol.IsDocumentMember() && !IsSectionOrConfigurationFactory(symbol));
+    }
+
+    // A node inside a method that is expression code because it, or its class, is marked [Expression].
+    public static bool IsPartOfMarkedPolicyExpressionMethod(this SyntaxNode syntax, SemanticModel model)
+    {
+        return syntax.Ancestors()
+            .OfType<MethodDeclarationSyntax>()
+            .Any(method => IsMarkedExpressionMethod(method, model));
+    }
+
+    private static bool IsMarkedExpressionMethod(MethodDeclarationSyntax method, SemanticModel model)
+    {
+        return method.AttributeLists.ContainsExpressionAttribute(model) ||
+               model.GetDeclaredSymbol(method)?.IsExpressionLibraryMember() == true;
+    }
+
+    private const string Authoring = "Microsoft.Azure.ApiManagement.PolicyToolkit.Authoring";
+    private const string SectionContext = Authoring + ".IHaveExpressionContext";
+
+    // The methods of a document that hold policies rather than expression code: sections and fragments return
+    // nothing and take a section context, policy configuration factories return a configuration.
+    private static bool IsSectionOrConfigurationFactory(IMethodSymbol method)
+    {
+        return method.ReturnsVoid ||
+               method.ReturnType.ContainingNamespace?.ToDisplayString() == Authoring ||
+               method.Parameters.Any(parameter =>
+                   parameter.Type.AllInterfaces.Any(type => type.ToFullyQualifiedString() == SectionContext));
+    }
+
+    private const string Document = Authoring + ".IDocument";
+    private const string Fragment = Authoring + ".IFragment";
+
+    // A member declared in the source of a policy document or fragment class, or of a class nested in one.
+    public static bool IsDocumentMember(this ISymbol symbol)
+    {
+        if (symbol.DeclaringSyntaxReferences.IsDefaultOrEmpty)
+        {
+            return false;
+        }
+
+        for (var type = symbol.ContainingType; type is not null; type = type.ContainingType)
+        {
+            if (type.AllInterfaces.Any(implemented => implemented.ToFullyQualifiedString() is Document or Fragment))
+            {
+                return true;
+            }
+        }
+
+        return false;
     }
 
     // A member of a class marked [Expression] (an expression helper library), from source or metadata.