Allow users to define EmbeddedAttribute (#76523)

* Work on embedded attribute recognition in source * Add tests * Additional testing * Refactoring for simplicity * Always validate in source, and document breaking change * Typo * Update test * Synthesize an application of EmbeddedAttribute if EmbeddedAttribute itself didn't have one applied to it. * Add VB validation implementation. * More PR feedback. * PR feedback * Minor typo and feedback
dotnet · Jan 3, 2025 · 0396afe · 0396afe
1 parent 692a6cc
commit 0396afe
Show file tree

Hide file tree

Showing 43 changed files with 1,159 additions and 104 deletions.
diff --git a/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md b/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md
@@ -156,3 +156,25 @@ struct S
     public static void M([UnscopedRef] ref int x) { }
 }
 ```
+
+## `Microsoft.CodeAnalysis.EmbeddedAttribute` is validated on declaration
+
+***Introduced in Visual Studio 2022 version 17.13***
+
+The compiler now validates the shape of `Microsoft.CodeAnalysis.EmbeddedAttribute` when declared in source. Previously, the compiler
+would allow user-defined declarations of this attribute, but only when it didn't need to generate one itself. We now validate that:
+
+1. It must be internal
+2. It must be a class
+3. It must be sealed
+4. It must be non-static
+5. It must have an internal or public parameterless constructor
+6. It must inherit from System.Attribute.
+7. It must be allowed on any type declaration (class, struct, interface, enum, or delegate)
+
+```cs
+namespace Microsoft.CodeAnalysis;
+
+// Previously, sometimes allowed. Now, CS9271
+public class EmbeddedAttribute : Attribute {}
+```
diff --git a/docs/compilers/Visual Basic/Compiler Breaking Changes - DotNet 10.md b/docs/compilers/Visual Basic/Compiler Breaking Changes - DotNet 10.md
@@ -35,3 +35,27 @@ Class C
 End Class
 ```
 
+## `Microsoft.CodeAnalysis.EmbeddedAttribute` is validated on declaration
+
+***Introduced in Visual Studio 2022 version 17.13***
+
+The compiler now validates the shape of `Microsoft.CodeAnalysis.EmbeddedAttribute` when declared in source. Previously, the compiler
+would allow user-defined declarations of this attribute with any shape. We now validate that:
+
+1. It must be Friend
+2. It must be a Class
+3. It must be NotInheritable
+4. It must not be a Module
+5. It must have a Public parameterless constructor
+6. It must inherit from System.Attribute.
+7. It must be allowed on any type declaration (Class, Structure, Interface, Enum, or Delegate)
+
+```vb
+Namespace Microsoft.CodeAnalysis
+
+    ' Previously allowed. Now, BC37335
+    Public Class EmbeddedAttribute
+        Inherits Attribute
+    End Class
+End Namespace
+```
diff --git a/src/Compilers/CSharp/Portable/CSharpResources.resx b/src/Compilers/CSharp/Portable/CSharpResources.resx
@@ -5908,6 +5908,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_TypeReserved" xml:space="preserve">
     <value>The type name '{0}' is reserved to be used by the compiler.</value>
   </data>
+  <data name="ERR_EmbeddedAttributeMustFollowPattern" xml:space="preserve">
+    <value>The type 'Microsoft.CodeAnalysis.EmbeddedAttribute' must be non-generic, internal, sealed, non-static, have a parameterless constructor, inherit from System.Attribute, and be able to be applied to any type.</value>
+  </data>
   <data name="ERR_InExtensionMustBeValueType" xml:space="preserve">
     <value>The first 'in' or 'ref readonly' parameter of the extension method '{0}' must be a concrete (non-generic) value type.</value>
   </data>

diff --git a/src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs b/src/Compilers/CSharp/Portable/Emitter/Model/PEAssemblyBuilder.cs
@@ -10,6 +10,7 @@
 using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
+using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.Emit;
 using Microsoft.CodeAnalysis.PooledObjects;
@@ -32,7 +33,7 @@ internal abstract class PEAssemblyBuilderBase : PEModuleBuilder, Cci.IAssemblyRe
         /// <summary>This is a cache of a subset of <seealso cref="_lazyFiles"/>. We don't include manifest resources in ref assemblies</summary>
         private ImmutableArray<Cci.IFileReference> _lazyFilesWithoutManifestResources;
 
-        private SynthesizedEmbeddedAttributeSymbol _lazyEmbeddedAttribute;
+        private NamedTypeSymbol _lazyEmbeddedAttribute;
         private SynthesizedEmbeddedAttributeSymbol _lazyIsReadOnlyAttribute;
         private SynthesizedEmbeddedAttributeSymbol _lazyRequiresLocationAttribute;
         private SynthesizedEmbeddedAttributeSymbol _lazyParamCollectionAttribute;
@@ -94,7 +95,11 @@ internal sealed override ImmutableArray<NamedTypeSymbol> GetEmbeddedTypes(Bindin
 
             CreateEmbeddedAttributesIfNeeded(diagnostics);
 
-            builder.AddIfNotNull(_lazyEmbeddedAttribute);
+            if (_lazyEmbeddedAttribute is SynthesizedEmbeddedAttributeSymbol)
+            {
+                builder.Add(_lazyEmbeddedAttribute);
+            }
+
             builder.AddIfNotNull(_lazyIsReadOnlyAttribute);
             builder.AddIfNotNull(_lazyRequiresLocationAttribute);
             builder.AddIfNotNull(_lazyParamCollectionAttribute);
@@ -387,7 +392,8 @@ private void CreateEmbeddedAttributesIfNeeded(BindingDiagnosticBag diagnostics)
                 ref _lazyEmbeddedAttribute,
                 diagnostics,
                 AttributeDescription.CodeAnalysisEmbeddedAttribute,
-                createParameterlessEmbeddedAttributeSymbol);
+                createParameterlessEmbeddedAttributeSymbol,
+                allowUserDefinedAttribute: true);
 
             if ((needsAttributes & EmbeddableAttributes.IsReadOnlyAttribute) != 0)
             {
@@ -544,16 +550,32 @@ private SynthesizedEmbeddedRefSafetyRulesAttributeSymbol CreateRefSafetyRulesAtt
                     GetWellKnownType(WellKnownType.System_Attribute, diagnostics),
                     GetSpecialType(SpecialType.System_Int32, diagnostics));
 
+#nullable enable
         private void CreateAttributeIfNeeded<T>(
             ref T symbol,
             BindingDiagnosticBag diagnostics,
             AttributeDescription description,
-            Func<string, NamespaceSymbol, BindingDiagnosticBag, T> factory)
-            where T : SynthesizedEmbeddedAttributeSymbolBase
+            Func<string, NamespaceSymbol, BindingDiagnosticBag, T> factory,
+            bool allowUserDefinedAttribute = false)
+            where T : NamedTypeSymbol
         {
+            Debug.Assert(!allowUserDefinedAttribute || typeof(T) == typeof(NamedTypeSymbol));
             if (symbol is null)
             {
-                AddDiagnosticsForExistingAttribute(description, diagnostics);
+                var userDefinedAttribute = getExistingType(description);
+
+                if (userDefinedAttribute is not null)
+                {
+                    if (allowUserDefinedAttribute)
+                    {
+                        symbol = (T)userDefinedAttribute;
+                        return;
+                    }
+                    else
+                    {
+                        diagnostics.Add(ErrorCode.ERR_TypeReserved, userDefinedAttribute.GetFirstLocation(), description.FullName);
+                    }
+                }
 
                 var containingNamespace = GetOrSynthesizeNamespace(description.Namespace);
 
@@ -567,23 +589,17 @@ private void CreateAttributeIfNeeded<T>(
 
                 AddSynthesizedDefinition(containingNamespace, symbol);
             }
-        }
-
-#nullable enable
 
-        private void AddDiagnosticsForExistingAttribute(AttributeDescription description, BindingDiagnosticBag diagnostics)
-        {
-            var attributeMetadataName = MetadataTypeName.FromFullName(description.FullName);
-            var userDefinedAttribute = _sourceAssembly.SourceModule.LookupTopLevelMetadataType(ref attributeMetadataName);
-            Debug.Assert(userDefinedAttribute is null || (object)userDefinedAttribute.ContainingModule == _sourceAssembly.SourceModule);
-            Debug.Assert(userDefinedAttribute?.IsErrorType() != true);
-
-            if (userDefinedAttribute is not null)
+            NamedTypeSymbol? getExistingType(AttributeDescription description)
             {
-                diagnostics.Add(ErrorCode.ERR_TypeReserved, userDefinedAttribute.GetFirstLocation(), description.FullName);
+                var attributeMetadataName = MetadataTypeName.FromFullName(description.FullName);
+                var userDefinedAttribute = _sourceAssembly.SourceModule.LookupTopLevelMetadataType(ref attributeMetadataName);
+                Debug.Assert(userDefinedAttribute is null || (object)userDefinedAttribute.ContainingModule == _sourceAssembly.SourceModule);
+                Debug.Assert(userDefinedAttribute?.IsErrorType() != true);
+
+                return userDefinedAttribute;
             }
         }
-
 #nullable disable
 
         protected NamespaceSymbol GetOrSynthesizeNamespace(string namespaceFullName)

diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs b/src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
@@ -2356,6 +2356,8 @@ internal enum ErrorCode
         WRN_UnscopedRefAttributeOldRules = 9269,
         WRN_InterceptsLocationAttributeUnsupportedSignature = 9270,
 
+        ERR_EmbeddedAttributeMustFollowPattern = 9271,
+
         // Note: you will need to do the following after adding errors:
         //  1) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)
 

diff --git a/src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs b/src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs
@@ -1844,6 +1844,7 @@ or ErrorCode.ERR_RefReturnReadonlyNotField
                 or ErrorCode.ERR_RefReturnReadonlyNotField2
                 or ErrorCode.ERR_ExplicitReservedAttr
                 or ErrorCode.ERR_TypeReserved
+                or ErrorCode.ERR_EmbeddedAttributeMustFollowPattern
                 or ErrorCode.ERR_RefExtensionMustBeValueTypeOrConstrainedToOne
                 or ErrorCode.ERR_InExtensionMustBeValueType
                 or ErrorCode.ERR_FieldsInRoStruct

diff --git a/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol.cs b/src/Compilers/CSharp/Portable/Symbols/Source/SourceNamedTypeSymbol.cs
@@ -4,6 +4,7 @@
 
 #nullable disable
 
+using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
@@ -1418,7 +1419,9 @@ internal override bool HasCodeAnalysisEmbeddedAttribute
             get
             {
                 var data = GetEarlyDecodedWellKnownAttributeData();
-                return data != null && data.HasCodeAnalysisEmbeddedAttribute;
+                return (data != null && data.HasCodeAnalysisEmbeddedAttribute)
+                    // If this is Microsoft.CodeAnalysis.EmbeddedAttribute, we'll synthesize EmbeddedAttribute even if it's not applied.
+                    || this.IsMicrosoftCodeAnalysisEmbeddedAttribute();
             }
         }
 
@@ -1761,6 +1764,21 @@ internal override void AddSynthesizedAttributes(PEModuleBuilder moduleBuilder, r
                         ImmutableArray.Create(new TypedConstant(compilation.GetWellKnownType(WellKnownType.System_Type), TypedConstantKind.Type, originalType)),
                         isOptionalUse: true));
             }
+
+            if (this.IsMicrosoftCodeAnalysisEmbeddedAttribute() && GetEarlyDecodedWellKnownAttributeData() is null or { HasCodeAnalysisEmbeddedAttribute: false })
+            {
+                // This is Microsoft.CodeAnalysis.EmbeddedAttribute, and the user didn't manually apply this attribute to itself. Grab the parameterless constructor
+                // and apply it; if there isn't a parameterless constructor, there would have been a declaration diagnostic
+
+                var parameterlessConstructor = InstanceConstructors.FirstOrDefault(c => c.ParameterCount == 0);
+
+                if (parameterlessConstructor is not null)
+                {
+                    AddSynthesizedAttribute(
+                        ref attributes,
+                        SynthesizedAttributeData.Create(DeclaringCompilation, parameterlessConstructor, arguments: [], namedArguments: []));
+                }
+            }
         }
 
         #endregion
@@ -1918,6 +1936,34 @@ protected override void AfterMembersCompletedChecks(BindingDiagnosticBag diagnos
                     diagnostics.Add(ErrorCode.ERR_RuntimeDoesNotSupportInlineArrayTypes, GetFirstLocation());
                 }
             }
+
+            if (this.IsMicrosoftCodeAnalysisEmbeddedAttribute())
+            {
+                // This is a user-defined implementation of the special attribute Microsoft.CodeAnalysis.EmbeddedAttribute. It needs to follow specific rules:
+                // 1. It must be internal
+                // 2. It must be a class
+                // 3. It must be sealed
+                // 4. It must be non-static
+                // 5. It must have an internal or public parameterless constructor
+                // 6. It must inherit from System.Attribute
+                // 7. It must be allowed on any type declaration (class, struct, interface, enum, or delegate)
+                // 8. It must be non-generic (checked as part of IsMicrosoftCodeAnalysisEmbeddedAttribute, we don't error on this because both types can exist)
+
+                const AttributeTargets expectedTargets = AttributeTargets.Class | AttributeTargets.Struct | AttributeTargets.Interface | AttributeTargets.Enum | AttributeTargets.Delegate;
+
+                if (DeclaredAccessibility != Accessibility.Internal
+                    || TypeKind != TypeKind.Class
+                    || !IsSealed
+                    || IsStatic
+                    || !InstanceConstructors.Any(c => c is { ParameterCount: 0, DeclaredAccessibility: Accessibility.Internal or Accessibility.Public })
+                    || !this.DeclaringCompilation.IsAttributeType(this)
+                    || (GetAttributeUsageInfo().ValidTargets & expectedTargets) != expectedTargets)
+                {
+                    // The type 'Microsoft.CodeAnalysis.EmbeddedAttribute' must be non-generic, internal, sealed, non-static, have a parameterless constructor, inherit from System.Attribute, and be able to be applied to any type.
+                    diagnostics.Add(ErrorCode.ERR_EmbeddedAttributeMustFollowPattern, GetFirstLocation());
+                }
+
+            }
         }
     }
 }
diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs
@@ -2137,6 +2137,17 @@ internal static bool IsWellKnownTypeLock(this TypeSymbol typeSymbol)
                 typeSymbol.IsContainedInNamespace(nameof(System), nameof(System.Threading));
         }
 
+        internal static bool IsMicrosoftCodeAnalysisEmbeddedAttribute(this TypeSymbol typeSymbol)
+        {
+            return typeSymbol is NamedTypeSymbol
+            {
+                Name: "EmbeddedAttribute",
+                Arity: 0,
+                ContainingType: null,
+            }
+            && typeSymbol.IsContainedInNamespace("Microsoft", "CodeAnalysis");
+        }
+
         private static bool IsWellKnownInteropServicesTopLevelType(this TypeSymbol typeSymbol, string name)
         {
             if (typeSymbol.Name != name || typeSymbol.ContainingType is object)

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