Add granular opt-in property for interceptors (#69712)

dotnet · Aug 28, 2023 · 9ca8c49 · 9ca8c49
1 parent 9d937b4
commit 9ca8c49
Show file tree

Hide file tree

Showing 25 changed files with 329 additions and 36 deletions.
diff --git a/docs/features/interceptors.md b/docs/features/interceptors.md
@@ -191,7 +191,12 @@ Interceptors are treated like a post-compilation step in this design. Diagnostic
 
 ### User opt-in
 
-Interceptors will require a feature flag during the experimental phase. The flag can be enabled with `/features=InterceptorsPreview` on the command line or `<Features>InterceptorsPreview</Features>` in msbuild.
+To use interceptors, the user project must specify the property `<InterceptorsPreviewNamespaces>`. This is a list of namespaces which are allowed to contain interceptors.
+```xml
+<InterceptorsPreviewNamespaces>$(InterceptorsPreviewNamespaces);Microsoft.AspNetCore.Http.Generated;MyLibrary.Generated</InterceptorsPreviewNamespaces>
+```
+
+It's expected that each entry in the `InterceptorsPreviewNamespaces` list roughly corresponds to one source generator. Well-behaved components are expected to not insert interceptors into namespaces they do not own.
 
 ### Implementation strategy
 

diff --git a/src/Compilers/CSharp/Portable/CSharpParseOptions.cs b/src/Compilers/CSharp/Portable/CSharpParseOptions.cs
@@ -21,6 +21,7 @@ public sealed class CSharpParseOptions : ParseOptions, IEquatable<CSharpParseOpt
         public static CSharpParseOptions Default { get; } = new CSharpParseOptions();
 
         private ImmutableDictionary<string, string> _features;
+        private ImmutableArray<ImmutableArray<string>> _interceptorsPreviewNamespaces;
 
         /// <summary>
         /// Gets the effective language version, which the compiler uses to select the
@@ -175,6 +176,32 @@ public override IReadOnlyDictionary<string, string> Features
             }
         }
 
+        internal ImmutableArray<ImmutableArray<string>> InterceptorsPreviewNamespaces
+        {
+            get
+            {
+                if (!_interceptorsPreviewNamespaces.IsDefault)
+                {
+                    return _interceptorsPreviewNamespaces;
+                }
+
+                // e.g. [["System", "Threading"], ["System", "Collections"]]
+                ImmutableArray<ImmutableArray<string>> previewNamespaces;
+                if (Features.TryGetValue("InterceptorsPreviewNamespaces", out var namespaces))
+                {
+                    previewNamespaces = namespaces
+                        .Split(';')
+                        .SelectAsArray(segment => segment.Split('.').ToImmutableArray());
+                }
+                else
+                {
+                    previewNamespaces = ImmutableArray<ImmutableArray<string>>.Empty;
+                }
+                ImmutableInterlocked.InterlockedInitialize(ref _interceptorsPreviewNamespaces, previewNamespaces);
+                return previewNamespaces;
+            }
+        }
+
         internal override void ValidateOptions(ArrayBuilder<Diagnostic> builder)
         {
             ValidateOptions(builder, MessageProvider.Instance);

diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx
@@ -7599,7 +7599,7 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
     <value>A switch expression arm does not begin with a 'case' keyword.</value>
   </data>
   <data name="ERR_InterceptorsFeatureNotEnabled" xml:space="preserve">
-    <value>The 'interceptors' experimental feature is not enabled. Add '&lt;Features&gt;InterceptorsPreview&lt;/Features&gt;' to your project.</value>
+    <value>The 'interceptors' experimental feature is not enabled. Add '{0}' to your project.</value>
   </data>
   <data name="ERR_InterceptorContainingTypeCannotBeGeneric" xml:space="preserve">
     <value>Method '{0}' cannot be used as an interceptor because its containing type has type parameters.</value>

diff --git a/src/Compilers/CSharp/Portable/CommandLine/CSharpCommandLineParser.cs b/src/Compilers/CSharp/Portable/CommandLine/CSharpCommandLineParser.cs
@@ -267,7 +267,10 @@ internal sealed override CommandLineArguments CommonParse(IEnumerable<string> ar
                         }
                         else
                         {
-                            features.Add(value);
+                            // When a features value like "InterceptorsPreviewNamespaces=NS1;NS2" is provided,
+                            // the build system will quote it so that splitting doesn't occur in the wrong layer.
+                            // We need to unquote here so that subsequent layers can properly identify the feature name and value.
+                            features.Add(value.Unquote());
                         }
                         continue;
 

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceMethodSymbolWithAttributes.cs
@@ -13,6 +13,7 @@
 using System.Threading;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Collections;
 using Roslyn.Utilities;
 
@@ -961,12 +962,30 @@ private void DecodeInterceptsLocationAttribute(DecodeWellKnownAttributeArguments
             const int lineNumberParameterIndex = 1;
             const int characterNumberParameterIndex = 2;
 
-            if (!attributeSyntax.SyntaxTree.Options.Features.ContainsKey("InterceptorsPreview"))
+            var interceptorsNamespaces = ((CSharpParseOptions)attributeSyntax.SyntaxTree.Options).InterceptorsPreviewNamespaces;
+            if (!attributeSyntax.SyntaxTree.Options.Features.ContainsKey("InterceptorsPreview") && interceptorsNamespaces.IsEmpty)
             {
-                diagnostics.Add(ErrorCode.ERR_InterceptorsFeatureNotEnabled, attributeSyntax);
+                // InterceptorsPreview feature flag wasn't specified, and a non-empty value for InterceptorsPreviewNamespaces wasn't specified.
+                var namespaceNames = getNamespaceNames();
+                reportFeatureNotEnabled(diagnostics, attributeSyntax, namespaceNames);
+                namespaceNames.Free();
                 return;
             }
 
+            if (!interceptorsNamespaces.IsEmpty)
+            {
+                // when InterceptorsPreviewNamespaces are present, ensure the interceptor is within one of the indicated namespaces
+                var thisNamespaceNames = getNamespaceNames();
+                var foundAnyMatch = interceptorsNamespaces.Any(ns => isDeclaredInNamespace(thisNamespaceNames, ns));
+                if (!foundAnyMatch)
+                {
+                    reportFeatureNotEnabled(diagnostics, attributeSyntax, thisNamespaceNames);
+                    thisNamespaceNames.Free();
+                    return;
+                }
+                thisNamespaceNames.Free();
+            }
+
             var attributeFilePath = (string?)attributeArguments[0].Value;
             if (attributeFilePath is null)
             {
@@ -1105,6 +1124,45 @@ static string mapPath(SourceReferenceResolver? referenceResolver, SyntaxTree tre
             {
                 return referenceResolver?.NormalizePath(tree.FilePath, baseFilePath: null) ?? tree.FilePath;
             }
+
+            // Caller must free the returned builder.
+            ArrayBuilder<string> getNamespaceNames()
+            {
+                var namespaceNames = ArrayBuilder<string>.GetInstance();
+                for (var containingNamespace = ContainingNamespace; containingNamespace?.IsGlobalNamespace == false; containingNamespace = containingNamespace.ContainingNamespace)
+                    namespaceNames.Add(containingNamespace.Name);
+                // order outermost->innermost
+                // e.g. for method MyApp.Generated.Interceptors.MyInterceptor(): ["MyApp", "Generated", "Interceptors"]
+                namespaceNames.ReverseContents();
+                return namespaceNames;
+            }
+
+            static bool isDeclaredInNamespace(ArrayBuilder<string> thisNamespaceNames, ImmutableArray<string> namespaceSegments)
+            {
+                Debug.Assert(namespaceSegments.Length > 0);
+                if (namespaceSegments.Length > thisNamespaceNames.Count)
+                {
+                    // the enabled NS has more components than interceptor's NS, so it will never match.
+                    return false;
+                }
+
+                for (var i = 0; i < namespaceSegments.Length; i++)
+                {
+                    if (namespaceSegments[i] != thisNamespaceNames[i])
+                    {
+                        return false;
+                    }
+                }
+                return true;
+            }
+
+            static void reportFeatureNotEnabled(BindingDiagnosticBag diagnostics, AttributeSyntax attributeSyntax, ArrayBuilder<string> namespaceNames)
+            {
+                var suggestedProperty = namespaceNames.Count == 0
+                    ? "<Features>$(Features);InterceptorsPreview</Features>"
+                    : $"<InterceptorsPreviewNamespaces>$(InterceptorsPreviewNamespaces);{string.Join(".", namespaceNames)}</InterceptorsPreviewNamespaces>";
+                diagnostics.Add(ErrorCode.ERR_InterceptorsFeatureNotEnabled, attributeSyntax, suggestedProperty);
+            }
         }
 
         private void DecodeUnmanagedCallersOnlyAttribute(ref DecodeWellKnownAttributeArguments<AttributeSyntax, CSharpAttributeData, AttributeLocation> arguments)

diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.cs.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.de.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.es.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.fr.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.it.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ja.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ko.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pl.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.pt-BR.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.ru.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.tr.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hans.xlf
diff --git a/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf b/src/Compilers/CSharp/Portable/xlf/CSharpResources.zh-Hant.xlf