From d051a846e333c3c350db328ba3c6206a99ef2097 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 19 Jul 2023 08:33:39 -0700 Subject: [PATCH] Avoid generating internal static classes with constant names in Options Source gen (#89148) --- .../gen/Emitter.cs | 42 ++- .../gen/Generator.cs | 2 +- .../tests/SourceGeneration.Unit.Tests/Main.cs | 260 ++++++++++++------ .../Baselines/NetCoreApp/Validators.g.cs | 6 +- .../Baselines/NetFX/Validators.g.cs | 6 +- 5 files changed, 221 insertions(+), 95 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Options/gen/Emitter.cs b/src/libraries/Microsoft.Extensions.Options/gen/Emitter.cs index 91ad41a630bf6..adbaa874ad7f3 100644 --- a/src/libraries/Microsoft.Extensions.Options/gen/Emitter.cs +++ b/src/libraries/Microsoft.Extensions.Options/gen/Emitter.cs @@ -5,6 +5,8 @@ using System.Collections.Generic; using System.Linq; using System.Threading; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.CSharp; namespace Microsoft.Extensions.Options.Generators { @@ -13,17 +15,37 @@ namespace Microsoft.Extensions.Options.Generators /// internal sealed class Emitter : EmitterBase { - private const string StaticValidationAttributeHolderClassName = "__Attributes"; - private const string StaticValidatorHolderClassName = "__Validators"; private const string StaticFieldHolderClassesNamespace = "__OptionValidationStaticInstances"; - private const string StaticValidationAttributeHolderClassFQN = $"global::{StaticFieldHolderClassesNamespace}.{StaticValidationAttributeHolderClassName}"; - private const string StaticValidatorHolderClassFQN = $"global::{StaticFieldHolderClassesNamespace}.{StaticValidatorHolderClassName}"; private const string StaticListType = "global::System.Collections.Generic.List"; private const string StaticValidationResultType = "global::System.ComponentModel.DataAnnotations.ValidationResult"; private const string StaticValidationAttributeType = "global::System.ComponentModel.DataAnnotations.ValidationAttribute"; + private string _staticValidationAttributeHolderClassName = "__Attributes"; + private string _staticValidatorHolderClassName = "__Validators"; + private string _staticValidationAttributeHolderClassFQN; + private string _staticValidatorHolderClassFQN; + private string _modifier; + private sealed record StaticFieldInfo(string FieldTypeFQN, int FieldOrder, string FieldName, IList InstantiationLines); + public Emitter(Compilation compilation, bool emitPreamble = true) : base(emitPreamble) + { + if (((CSharpCompilation)compilation).LanguageVersion >= Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp11) + { + _modifier = "file"; + } + else + { + _modifier = "internal"; + string suffix = $"_{new Random().Next():X8}"; + _staticValidationAttributeHolderClassName += suffix; + _staticValidatorHolderClassName += suffix; + } + + _staticValidationAttributeHolderClassFQN = $"global::{StaticFieldHolderClassesNamespace}.{_staticValidationAttributeHolderClassName}"; + _staticValidatorHolderClassFQN = $"global::{StaticFieldHolderClassesNamespace}.{_staticValidatorHolderClassName}"; + } + public string Emit( IEnumerable validatorTypes, CancellationToken cancellationToken) @@ -37,8 +59,8 @@ public string Emit( GenValidatorType(vt, ref staticValidationAttributesDict, ref staticValidatorsDict); } - GenStaticClassWithStaticReadonlyFields(staticValidationAttributesDict.Values, StaticFieldHolderClassesNamespace, StaticValidationAttributeHolderClassName); - GenStaticClassWithStaticReadonlyFields(staticValidatorsDict.Values, StaticFieldHolderClassesNamespace, StaticValidatorHolderClassName); + GenStaticClassWithStaticReadonlyFields(staticValidationAttributesDict.Values, StaticFieldHolderClassesNamespace, _staticValidationAttributeHolderClassName); + GenStaticClassWithStaticReadonlyFields(staticValidatorsDict.Values, StaticFieldHolderClassesNamespace, _staticValidatorHolderClassName); return Capture(); } @@ -95,7 +117,7 @@ private void GenStaticClassWithStaticReadonlyFields(IEnumerable OutOpenBrace(); OutGeneratedCodeAttribute(); - OutLn($"internal static class {className}"); + OutLn($"{_modifier} static class {className}"); OutOpenBrace(); var staticValidationAttributes = staticFields @@ -214,7 +236,7 @@ private void GenMemberValidation(ValidatedMember vm, ref Dictionary 0) { - var emitter = new Emitter(); + var emitter = new Emitter(compilation); var result = emitter.Emit(validatorTypes, context.CancellationToken); context.AddSource("Validators.g.cs", SourceText.From(result, Encoding.UTF8)); diff --git a/src/libraries/Microsoft.Extensions.Options/tests/SourceGeneration.Unit.Tests/Main.cs b/src/libraries/Microsoft.Extensions.Options/tests/SourceGeneration.Unit.Tests/Main.cs index 478b37cde349b..756b1698bec28 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/SourceGeneration.Unit.Tests/Main.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/SourceGeneration.Unit.Tests/Main.cs @@ -101,7 +101,7 @@ partial struct MyOptionsValidator namespace __OptionValidationStaticInstances { [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")] - internal static class __Attributes + file static class __Attributes { internal static readonly global::System.ComponentModel.DataAnnotations.RequiredAttribute A1 = new global::System.ComponentModel.DataAnnotations.RequiredAttribute(); @@ -113,24 +113,14 @@ internal static class __Attributes namespace __OptionValidationStaticInstances { [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")] - internal static class __Validators + file static class __Validators { } } """; - var (diagnostics, generatedSources) = await RoslynTestUtils.RunGenerator( - new Generator(), - new[] - { - Assembly.GetAssembly(typeof(RequiredAttribute))!, - Assembly.GetAssembly(typeof(OptionsValidatorAttribute))!, - Assembly.GetAssembly(typeof(IValidateOptions))!, - }, - new List { source }) - .ConfigureAwait(false); - + var (diagnostics, generatedSources) = await RunGeneratorOnOptionsSource(source); Assert.Empty(diagnostics); _ = Assert.Single(generatedSources); @@ -1211,15 +1201,7 @@ public class SecondClassInAnotherAssembly string assemblyName = Path.GetRandomFileName(); string assemblyPath = Path.Combine(Path.GetTempPath(), assemblyName + ".dll"); - var compilation = CSharpCompilation - .Create(assemblyName, options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) - .AddReferences(MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(a => a.GetName().Name == "System.Runtime").Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(string).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(RequiredAttribute).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(OptionsValidatorAttribute).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(IValidateOptions).Assembly.Location)) - .AddSyntaxTrees(CSharpSyntaxTree.ParseText(source)); - + CSharpCompilation compilation = CreateCompilationForOptionsSource(assemblyName, source); EmitResult emitResult = compilation.Emit(assemblyPath); Assert.True(emitResult.Success); @@ -1243,18 +1225,7 @@ public class MyOptions Assembly assembly = Assembly.LoadFrom(assemblyFullPath); - var (diagnostics, generatedSources) = await RoslynTestUtils.RunGenerator( - new Generator(), - new[] - { - assembly, - Assembly.GetAssembly(typeof(RequiredAttribute)), - Assembly.GetAssembly(typeof(OptionsValidatorAttribute)), - Assembly.GetAssembly(typeof(IValidateOptions)), - }, - new List { source1 }) - .ConfigureAwait(false); - + var (diagnostics, generatedSources) = await RunGeneratorOnOptionsSource(source1, assembly); _ = Assert.Single(generatedSources); var diag = Assert.Single(diagnostics); Assert.Equal(DiagDescriptors.PotentiallyMissingTransitiveValidation.Id, diag.Id); @@ -1266,6 +1237,138 @@ public class MyOptions File.Delete(assemblyPath); // cleanup } + [ConditionalTheory(nameof(SupportRemoteExecutionAndNotInBrowser))] + [InlineData(LanguageVersion.CSharp10)] + [InlineData(LanguageVersion.CSharp11)] + public async Task InternalsVisibleToAssembliesTest(LanguageVersion languageVersion) + { + string assemblyName = Path.GetRandomFileName(); + string assemblyPath = Path.Combine(Path.GetTempPath(), assemblyName + ".dll"); + + string source = $$""" + using Microsoft.Extensions.Options; + using System.ComponentModel.DataAnnotations; + + // Make this assembly visible to the other assembly + [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("{{assemblyName + "0"}}")] + + #nullable enable + + namespace ValidationTest + { + public class FirstOptions + { + [Required] + public string? Prop { get; set; } + } + + [OptionsValidator] + internal sealed partial class FirstOptionsValidator : IValidateOptions + { + } + } + """; + + var (diagnostics, generatedSources) = await RunGeneratorOnOptionsSource(source, null, languageVersion); + Assert.Empty(diagnostics); + _ = Assert.Single(generatedSources); + + CSharpCompilation compilation = CreateCompilationForOptionsSource(assemblyName, source + Environment.NewLine + generatedSources[0].SourceText.ToString()); + EmitResult emitResult = compilation.Emit(assemblyPath); + Assert.True(emitResult.Success); + + RemoteExecutor.Invoke(async (asmName, assemblyFullPath, langVersion) => { + + Assembly assembly = Assembly.LoadFrom(assemblyFullPath); + + string source1 = """ + using Microsoft.Extensions.Options; + using System.ComponentModel.DataAnnotations; + + #nullable enable + + namespace ValidationTest + { + public class SecondOptions + { + [Required] + public string? Prop { get; set; } + } + + [OptionsValidator] + internal sealed partial class SecondOptionsValidator : IValidateOptions + { + } + } + """; + + var (diagnostics, generatedSources) = await RunGeneratorOnOptionsSource(source1, null, (LanguageVersion)Enum.Parse(typeof(LanguageVersion), langVersion)); + Assert.Empty(diagnostics); + _ = Assert.Single(generatedSources); + + CSharpCompilation compilation1 = CreateCompilationForOptionsSource(asmName + "0", source1 + Environment.NewLine + generatedSources[0].SourceText.ToString(), assemblyFullPath); + MemoryStream ms = new(); + EmitResult emitResult1 = compilation1.Emit(ms); + Assert.True(emitResult1.Success); + }, assemblyName, assemblyPath, languageVersion.ToString(), new RemoteInvokeOptions { TimeOut = 300 * 1000}).Dispose(); + + File.Delete(assemblyPath); // cleanup + } + + [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))] + [InlineData(LanguageVersion.Preview)] + [InlineData(LanguageVersion.CSharp11)] + [InlineData(LanguageVersion.CSharp10)] + [InlineData(LanguageVersion.CSharp9)] + public async Task GenerateSourceUsingVariousLanguageVersions(LanguageVersion languageVersion) + { + string source = $$""" + using Microsoft.Extensions.Options; + using System.ComponentModel.DataAnnotations; + + #nullable enable + + namespace LanguageVersionTest + { + public class MyOptions + { + [Required] public string? Prop { get; set; } + [Range(1, 3)] public int Val { get; set; } + } + + [OptionsValidator] + internal sealed partial class MyOptionsValidator : IValidateOptions + { + } + } + """; + + var (diagnostics, generatedSources) = await RunGeneratorOnOptionsSource(source, null, languageVersion); + Assert.Empty(diagnostics); + _ = Assert.Single(generatedSources); + + // Console.WriteLine(generatedSources[0].SourceText.ToString()); + string generatedSource = generatedSources[0].SourceText.ToString(); + + if (languageVersion >= LanguageVersion.CSharp11) + { + Assert.Contains("file static class __Attributes", generatedSource); + Assert.Contains("file static class __Validators", generatedSource); + } + else + { + const string attributesClassDefinition = "internal static class __Attributes_"; + const string validatorsClassDefinition = "internal static class __Validators_"; + int index = generatedSource.IndexOf(attributesClassDefinition, StringComparison.Ordinal); + Assert.True(index > 0, $"{attributesClassDefinition} not found in the generated source"); + string suffix = generatedSource.Substring(index + attributesClassDefinition.Length, 8); + index = generatedSource.IndexOf(validatorsClassDefinition, StringComparison.Ordinal); + Assert.True(index > 0, $"{validatorsClassDefinition} not found in the generated source"); + Assert.True(index + validatorsClassDefinition.Length + 8 <= generatedSource.Length, $"{validatorsClassDefinition} suffix not found in the generated source"); + Assert.Equal(suffix, generatedSource.Substring(index + validatorsClassDefinition.Length, 8)); + } + } + [ConditionalFact(nameof(SupportRemoteExecutionAndNotInBrowser))] public async Task InaccessibleValidationAttributesTest() { @@ -1299,15 +1402,7 @@ protected override ValidationResult IsValid(object? value, ValidationContext? va string assemblyName = Path.GetRandomFileName(); string assemblyPath = Path.Combine(Path.GetTempPath(), assemblyName + ".dll"); - var compilation = CSharpCompilation - .Create(assemblyName, options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) - .AddReferences(MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(a => a.GetName().Name == "System.Runtime").Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(string).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(RequiredAttribute).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(OptionsValidatorAttribute).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(IValidateOptions).Assembly.Location)) - .AddSyntaxTrees(CSharpSyntaxTree.ParseText(source)); - + CSharpCompilation compilation = CreateCompilationForOptionsSource(assemblyName, source); EmitResult emitResult = compilation.Emit(assemblyPath); Assert.True(emitResult.Success); @@ -1344,18 +1439,7 @@ internal sealed partial class ExtOptionsValidator : IValidateOptions Assembly assembly = Assembly.LoadFrom(assemblyFullPath); - var (diagnostics, generatedSources) = await RoslynTestUtils.RunGenerator( - new Generator(), - new[] - { - assembly, - Assembly.GetAssembly(typeof(RequiredAttribute)), - Assembly.GetAssembly(typeof(OptionsValidatorAttribute)), - Assembly.GetAssembly(typeof(IValidateOptions)), - }, - new List { source0 + source1 + source2 }) - .ConfigureAwait(false); - + var (diagnostics, generatedSources) = await RunGeneratorOnOptionsSource(source0 + source1 + source2, assembly); _ = Assert.Single(generatedSources); Assert.Single(diagnostics); Assert.Equal(DiagDescriptors.InaccessibleValidationAttribute.Id, diagnostics[0].Id); @@ -1364,18 +1448,7 @@ internal sealed partial class ExtOptionsValidator : IValidateOptions Assert.Contains("global::System.ComponentModel.DataAnnotations.RequiredAttribute", generatedSource); Assert.DoesNotContain("Timeout", generatedSource); - // Ensure the generated source compiles - var compilation = CSharpCompilation - .Create(Path.GetRandomFileName()+".dll", options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) - .AddReferences(MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(a => a.GetName().Name == "System.Runtime").Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(string).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(RequiredAttribute).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(OptionsValidatorAttribute).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(IValidateOptions).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(typeof(System.CodeDom.Compiler.GeneratedCodeAttribute).Assembly.Location)) - .AddReferences(MetadataReference.CreateFromFile(assemblyFullPath)) - .AddSyntaxTrees(CSharpSyntaxTree.ParseText(source1 + Environment.NewLine + generatedSource)); - + CSharpCompilation compilation = CreateCompilationForOptionsSource(Path.GetRandomFileName()+".dll", source1 + Environment.NewLine + generatedSource, assemblyFullPath); MemoryStream ms = new(); EmitResult emitResult = compilation.Emit(ms); Assert.True(emitResult.Success); @@ -1418,17 +1491,7 @@ public sealed partial class MyOptionsValidator : IValidateOptions } """; - var (diagnostics, generatedSources) = await RoslynTestUtils.RunGenerator( - new Generator(), - new[] - { - Assembly.GetAssembly(typeof(RequiredAttribute)), - Assembly.GetAssembly(typeof(OptionsValidatorAttribute)), - Assembly.GetAssembly(typeof(IValidateOptions)), - }, - new List { source3 }) - .ConfigureAwait(false); - + var (diagnostics, generatedSources) = await RunGeneratorOnOptionsSource(source3); _ = Assert.Single(generatedSources); Assert.Single(diagnostics); Assert.Equal(DiagDescriptors.InaccessibleValidationAttribute.Id, diagnostics[0].Id); @@ -1519,6 +1582,47 @@ public partial class FirstValidator : IValidateOptions Assert.Equal(DiagDescriptors.NotEnumerableType.Id, diagnostics[0].Id); } + private static CSharpCompilation CreateCompilationForOptionsSource(string assemblyName, string source, string? refAssemblyPath = null) + { + // Ensure the generated source compiles + var compilation = CSharpCompilation + .Create(Path.GetRandomFileName()+".dll", options: new CSharpCompilationOptions(OutputKind.DynamicallyLinkedLibrary)) + .AddReferences(MetadataReference.CreateFromFile(AppDomain.CurrentDomain.GetAssemblies().FirstOrDefault(a => a.GetName().Name == "System.Runtime").Location)) + .AddReferences(MetadataReference.CreateFromFile(typeof(string).Assembly.Location)) + .AddReferences(MetadataReference.CreateFromFile(typeof(RequiredAttribute).Assembly.Location)) + .AddReferences(MetadataReference.CreateFromFile(typeof(OptionsValidatorAttribute).Assembly.Location)) + .AddReferences(MetadataReference.CreateFromFile(typeof(IValidateOptions).Assembly.Location)) + .AddReferences(MetadataReference.CreateFromFile(typeof(System.CodeDom.Compiler.GeneratedCodeAttribute).Assembly.Location)) + .AddSyntaxTrees(CSharpSyntaxTree.ParseText(source)); + + if (refAssemblyPath is not null) + { + compilation = compilation.AddReferences(MetadataReference.CreateFromFile(refAssemblyPath)); + } + + return compilation; + } + + private static async Task<(IReadOnlyList, ImmutableArray)> RunGeneratorOnOptionsSource( + string source, + Assembly? refAssembly = null, + LanguageVersion languageVersion = LanguageVersion.Preview) + { + List refAssemblies = new() + { + Assembly.GetAssembly(typeof(RequiredAttribute)), + Assembly.GetAssembly(typeof(OptionsValidatorAttribute)), + Assembly.GetAssembly(typeof(IValidateOptions)), + }; + + if (refAssembly is not null) + { + refAssemblies.Add(refAssembly); + } + + return await RoslynTestUtils.RunGenerator(new Generator(), refAssemblies.ToArray(), new List { source }, includeBaseReferences: true, languageVersion).ConfigureAwait(false); + } + private static async Task<(IReadOnlyList diagnostics, ImmutableArray generatedSources)> RunGenerator( string code, bool wrap = true, diff --git a/src/libraries/Microsoft.Extensions.Options/tests/SourceGenerationTests/Baselines/NetCoreApp/Validators.g.cs b/src/libraries/Microsoft.Extensions.Options/tests/SourceGenerationTests/Baselines/NetCoreApp/Validators.g.cs index a78ca02c4b6ca..f84844343a47d 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/SourceGenerationTests/Baselines/NetCoreApp/Validators.g.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/SourceGenerationTests/Baselines/NetCoreApp/Validators.g.cs @@ -1,4 +1,4 @@ - + // #nullable enable #pragma warning disable CS1591 // Compensate for https://github.com/dotnet/roslyn/issues/54103 @@ -1968,7 +1968,7 @@ partial struct FirstValidator namespace __OptionValidationStaticInstances { [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")] - internal static class __Attributes + file static class __Attributes { internal static readonly global::System.ComponentModel.DataAnnotations.RequiredAttribute A1 = new global::System.ComponentModel.DataAnnotations.RequiredAttribute(); @@ -2059,7 +2059,7 @@ internal static class __Attributes namespace __OptionValidationStaticInstances { [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")] - internal static class __Validators + file static class __Validators { internal static readonly global::SecondValidatorNoNamespace V1 = new global::SecondValidatorNoNamespace(); diff --git a/src/libraries/Microsoft.Extensions.Options/tests/SourceGenerationTests/Baselines/NetFX/Validators.g.cs b/src/libraries/Microsoft.Extensions.Options/tests/SourceGenerationTests/Baselines/NetFX/Validators.g.cs index daa65114c056a..9055e117422ba 100644 --- a/src/libraries/Microsoft.Extensions.Options/tests/SourceGenerationTests/Baselines/NetFX/Validators.g.cs +++ b/src/libraries/Microsoft.Extensions.Options/tests/SourceGenerationTests/Baselines/NetFX/Validators.g.cs @@ -1,4 +1,4 @@ - + // #nullable enable #pragma warning disable CS1591 // Compensate for https://github.com/dotnet/roslyn/issues/54103 @@ -1968,7 +1968,7 @@ partial struct FirstValidator namespace __OptionValidationStaticInstances { [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")] - internal static class __Attributes + file static class __Attributes { internal static readonly global::System.ComponentModel.DataAnnotations.RequiredAttribute A1 = new global::System.ComponentModel.DataAnnotations.RequiredAttribute(); @@ -2051,7 +2051,7 @@ internal static class __Attributes namespace __OptionValidationStaticInstances { [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Extensions.Options.SourceGeneration", "42.42.42.42")] - internal static class __Validators + file static class __Validators { internal static readonly global::SecondValidatorNoNamespace V1 = new global::SecondValidatorNoNamespace();