From a8a8a134d80f3ab119dfac00047d604caff44ea4 Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Fri, 25 Aug 2023 15:18:14 -0700 Subject: [PATCH] Emit interceptor info correctly when invocation expr is on separate line (#91107) * Emit interceptor info correctly when invocation expr is on separate line * Add more tests and add helper to udpate baselines --- .../gen/Helpers/InterceptorLocationInfo.cs | 35 ++++---- .../ConfigurationBinderTests.Helpers.cs | 12 +++ .../ConfigurationBinderTests.TestClasses.cs | 12 --- .../ConfigurationBinderTests.Generator.cs | 86 +++++++++++++++++++ ...cs => GeneratorTests.Baselines.Options.cs} | 2 +- ...selines.cs => GeneratorTests.Baselines.cs} | 3 +- ...ingGeneratorTests.cs => GeneratorTests.cs} | 22 ++++- ...ation.Binder.SourceGeneration.Tests.csproj | 9 +- ...ionsBuilderConfigurationExtensionsTests.cs | 7 +- 9 files changed, 144 insertions(+), 44 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBinderTests.Generator.cs rename src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/{ConfigurationBindingGeneratorTests.Baselines.Options.cs => GeneratorTests.Baselines.Options.cs} (98%) rename src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/{ConfigurationBindingGeneratorTests.Baselines.cs => GeneratorTests.Baselines.cs} (99%) rename src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/{ConfigurationBindingGeneratorTests.cs => GeneratorTests.cs} (94%) diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/InterceptorLocationInfo.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/InterceptorLocationInfo.cs index d1dc4f4afa7e7..441acbe6a7444 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/InterceptorLocationInfo.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/InterceptorLocationInfo.cs @@ -15,28 +15,27 @@ internal sealed record InterceptorLocationInfo { public InterceptorLocationInfo(IInvocationOperation operation) { - SyntaxNode operationSyntax = operation.Syntax; - TextSpan operationSpan = operationSyntax.Span; - SyntaxTree operationSyntaxTree = operationSyntax.SyntaxTree; - - FilePath = GetInterceptorFilePath(operationSyntaxTree, operation.SemanticModel?.Compilation.Options.SourceReferenceResolver); - - FileLinePositionSpan span = operationSyntaxTree.GetLineSpan(operationSpan); - LineNumber = span.StartLinePosition.Line + 1; - - // Calculate the character offset to the end of the binding invocation detected. - int invocationLength = ((MemberAccessExpressionSyntax)((InvocationExpressionSyntax)operationSyntax).Expression).Expression.Span.Length; - CharacterNumber = span.StartLinePosition.Character + invocationLength + 2; + MemberAccessExpressionSyntax memberAccessExprSyntax = ((MemberAccessExpressionSyntax)((InvocationExpressionSyntax)operation.Syntax).Expression); + SyntaxTree operationSyntaxTree = operation.Syntax.SyntaxTree; + TextSpan memberNameSpan = memberAccessExprSyntax.Name.Span; + FileLinePositionSpan linePosSpan = operationSyntaxTree.GetLineSpan(memberNameSpan); + + LineNumber = linePosSpan.StartLinePosition.Line + 1; + CharacterNumber = linePosSpan.StartLinePosition.Character + 1; + FilePath = GetInterceptorFilePath(); + + // Use the same logic used by the interceptors API for resolving the source mapped value of a path. + // https://github.com/dotnet/roslyn/blob/f290437fcc75dad50a38c09e0977cce13a64f5ba/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs#L1063-L1064 + string GetInterceptorFilePath() + { + SourceReferenceResolver? sourceReferenceResolver = operation.SemanticModel?.Compilation.Options.SourceReferenceResolver; + return sourceReferenceResolver?.NormalizePath(operationSyntaxTree.FilePath, baseFilePath: null) ?? operationSyntaxTree.FilePath; + } } public string FilePath { get; } public int LineNumber { get; } public int CharacterNumber { get; } - - // Utilize the same logic used by the interceptors API for resolving the source mapped value of a path. - // https://github.com/dotnet/roslyn/blob/f290437fcc75dad50a38c09e0977cce13a64f5ba/src/Compilers/CSharp/Portable/Compilation/CSharpCompilation.cs#L1063-L1064 - private static string GetInterceptorFilePath(SyntaxTree tree, SourceReferenceResolver? resolver) => - resolver?.NormalizePath(tree.FilePath, baseFilePath: null) ?? tree.FilePath; } internal sealed record ConfigurationBinderInterceptorInfo @@ -52,7 +51,7 @@ public void RegisterOverloadInfo(MethodsToGen_ConfigurationBinder overload, Type } public OverloadInterceptorInfo GetOverloadInfo(MethodsToGen_ConfigurationBinder overload) => - DetermineOverload(overload, initIfNull: false) ?? throw new ArgumentNullException(nameof(overload)); + DetermineOverload(overload, initIfNull: false) ?? throw new ArgumentOutOfRangeException(nameof(overload)); private OverloadInterceptorInfo? DetermineOverload(MethodsToGen_ConfigurationBinder overload, bool initIfNull) { diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Helpers.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Helpers.cs index a1d1a72ffab20..d6521ed86dfde 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Helpers.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Helpers.cs @@ -160,5 +160,17 @@ public class CustomICollectionWithoutAnAddMethod : ICollection public int Count => _items.Count; public bool IsReadOnly => false; } + + public interface IGeolocation + { + public double Latitude { get; set; } + public double Longitude { get; set; } + } + + public sealed record GeolocationRecord : IGeolocation + { + public double Latitude { get; set; } + public double Longitude { get; set; } + } #endregion } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs index 1e537b407963d..76ed9d959e622 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs @@ -667,12 +667,6 @@ public struct StructWithParameterlessAndParameterizedCtor public int MyInt { get; } } - public interface IGeolocation - { - public double Latitude { get; set; } - public double Longitude { get; set; } - } - [TypeConverter(typeof(GeolocationTypeConverter))] public struct Geolocation : IGeolocation { @@ -704,12 +698,6 @@ public sealed class GeolocationClass : IGeolocation public double Longitude { get; set; } } - public sealed record GeolocationRecord : IGeolocation - { - public double Latitude { get; set; } - public double Longitude { get; set; } - } - public class GeolocationWrapper { public Geolocation Location { get; set; } diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBinderTests.Generator.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBinderTests.Generator.cs new file mode 100644 index 0000000000000..16390e054b87c --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBinderTests.Generator.cs @@ -0,0 +1,86 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Configuration; +using Xunit; + +namespace Microsoft.Extensions.SourceGeneration.Configuration.Binder.Tests +{ + public partial class ConfigurationBinderTests : ConfigurationBinderTestsBase + { + [Fact] + public void GeneratorHandlesInvocationsOnNewline() + { + IConfiguration configuration = TestHelpers.GetConfigurationFromJsonString(@"{""Longitude"":1,""Latitude"":2}"); + + GeolocationRecord record = configuration.Get< + GeolocationRecord + >(); + Verify(); + + record = (GeolocationRecord)configuration + .Get(typeof(GeolocationRecord), _ => { }); + Verify(); + + TestHelpers + .GetConfigurationFromJsonString(@"{""Longitude"":3,""Latitude"":4}") + .Bind(record); + Verify(3, 4); + + int lat = configuration + .GetValue("Latitude"); + Assert.Equal(2, lat); + + record = configuration.Get + (); + Verify(); + + record = (GeolocationRecord)configuration + .Get( + typeof(GeolocationRecord), _ => + { }); + Verify(); + + TestHelpers + .GetConfigurationFromJsonString(@"{""Longitude"":3, +""Latitude"":4} +") + .Bind( + record + ); + Verify(3, 4); + + long latLong = configuration + .GetValue< +#if DEBUG + int +#else + long +#endif + > + ("Latitude") + ; + Assert.Equal(2, lat); + + record = (GeolocationRecord)configuration. + Get(typeof(GeolocationRecord), _ => { }); + Verify(); + + record = (GeolocationRecord) + configuration. + Get(typeof(GeolocationRecord), _ => { }); + Verify(); + + record = (GeolocationRecord) + configuration + .Get(typeof(GeolocationRecord), _ => { }); + Verify(); + + void Verify(int longitude = 1, int latitude = 2) + { + Assert.Equal(longitude, record.Longitude); + Assert.Equal(latitude, record.Latitude); + } + } + } +} diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.Options.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.Options.cs similarity index 98% rename from src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.Options.cs rename to src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.Options.cs index c9eb9c70927f9..0126744eacbfa 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.Options.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.Options.cs @@ -4,7 +4,7 @@ using System.Threading.Tasks; using Xunit; -namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests +namespace Microsoft.Extensions.SourceGeneration.Configuration.Binder.Tests { public partial class ConfigurationBindingGeneratorTests { diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.cs similarity index 99% rename from src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.cs rename to src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.cs index b5c3fb49c5e7e..65e812d30b420 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.Baselines.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.Baselines.cs @@ -8,7 +8,7 @@ using Microsoft.CodeAnalysis.CSharp; using Xunit; -namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests +namespace Microsoft.Extensions.SourceGeneration.Configuration.Binder.Tests { public partial class ConfigurationBindingGeneratorTests { @@ -649,7 +649,6 @@ public interface ICustomSet : ISet await VerifyAgainstBaselineUsingFile("Collections.generated.txt", source, assessDiagnostics: (d) => { - Console.WriteLine((d.Where(diag => diag.Id == Diagnostics.TypeNotSupported.Id).Count(), d.Where(diag => diag.Id == Diagnostics.PropertyNotSupported.Id).Count())); Assert.Equal(3, d.Where(diag => diag.Id == Diagnostics.TypeNotSupported.Id).Count()); Assert.Equal(6, d.Where(diag => diag.Id == Diagnostics.PropertyNotSupported.Id).Count()); }); diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.cs b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs similarity index 94% rename from src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.cs rename to src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs index a512c5efe495b..68f8a66612e67 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBindingGeneratorTests.cs +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/GeneratorTests.cs @@ -14,13 +14,14 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Configuration.Binder.SourceGeneration; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Options; -using Microsoft.Extensions.SourceGeneration.Configuration.Binder.Tests; using SourceGenerators.Tests; using Xunit; -namespace Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests +namespace Microsoft.Extensions.SourceGeneration.Configuration.Binder.Tests { [ActiveIssue("https://github.com/dotnet/runtime/issues/52062", TestPlatforms.Browser)] public partial class ConfigurationBindingGeneratorTests : ConfigurationBinderTestsBase @@ -388,11 +389,24 @@ private static async Task VerifyAgainstBaselineUsingFile( var (d, r) = await RunGenerator(testSourceCode, languageVersion); bool success = RoslynTestUtils.CompareLines(expectedLines, r[0].SourceText, out string errorMessage); -#if !SKIP_BASELINES +#if UPDATE_BASELINES + if (!success) + { + string? repoRootDir = Environment.GetEnvironmentVariable("RepoRootDir"); + Assert.True(repoRootDir is not null, "To update baselines, specifiy the root runtime repo dir"); + + IEnumerable lines = r[0].SourceText.Lines.Select(l => l.ToString()); + string source = string.Join(Environment.NewLine, lines).TrimEnd(Environment.NewLine.ToCharArray()) + Environment.NewLine; + path = Path.Combine($"{repoRootDir}\\src\\libraries\\Microsoft.Extensions.Configuration.Binder\\tests\\SourceGenerationTests\\", path); + + await File.WriteAllTextAsync(path, source).ConfigureAwait(false); + success = true; + } +#endif + Assert.Single(r); (assessDiagnostics ?? ((d) => Assert.Empty(d))).Invoke(d); Assert.True(success, errorMessage); -#endif } private static async Task<(ImmutableArray, ImmutableArray)> RunGenerator( diff --git a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj index cfd45c365d42a..94d95564a4735 100644 --- a/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Configuration.Binder/tests/SourceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj @@ -11,7 +11,7 @@ $(DefineConstants);BUILDING_SOURCE_GENERATOR_TESTS;ROSLYN4_0_OR_GREATER;ROSLYN4_4_OR_GREATER $(DefineConstants);LAUNCH_DEBUGGER - $(DefineConstants);SKIP_BASELINES + $(DefineConstants);UPDATE_BASELINES @@ -28,6 +28,7 @@ + @@ -50,9 +51,9 @@ PreserveNewest - - - + + + diff --git a/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/Common/OptionsBuilderConfigurationExtensionsTests.cs b/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/Common/OptionsBuilderConfigurationExtensionsTests.cs index 6d1d4a018f097..d90122ef74d8b 100644 --- a/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/Common/OptionsBuilderConfigurationExtensionsTests.cs +++ b/src/libraries/Microsoft.Extensions.Options.ConfigurationExtensions/tests/Common/OptionsBuilderConfigurationExtensionsTests.cs @@ -31,7 +31,8 @@ public static void BindConfiguration_ThrowsForNullConfigurationSectionPath() Assert.Throws("configSectionPath", () => { - optionsBuilder.BindConfiguration(configSectionPath); + optionsBuilder + .BindConfiguration(configSectionPath); }); } @@ -170,8 +171,8 @@ public static void BindConfiguration_UpdatesOptionOnConfigurationUpdate() services.AddSingleton(new ConfigurationBuilder() .Add(configSource) .Build()); - OptionsBuilder optionsBuilder = services.AddOptions(); - _ = optionsBuilder.BindConfiguration(configSectionName); + _ = services.AddOptions() + .BindConfiguration(configSectionName); using ServiceProvider serviceProvider = services.BuildServiceProvider(); var optionsMonitor = serviceProvider.GetRequiredService>(); bool updateHasRun = false;