Emit interceptor info correctly when invocation expr is on separate l…

…ine (#91107) * Emit interceptor info correctly when invocation expr is on separate line * Add more tests and add helper to udpate baselines
dotnet · Aug 25, 2023 · a8a8a13 · a8a8a13
1 parent 17bae6c
commit a8a8a13
Show file tree

Hide file tree

Showing 9 changed files with 144 additions and 44 deletions.
diff --git a/...ibraries/Microsoft.Extensions.Configuration.Binder/gen/Helpers/InterceptorLocationInfo.cs b/...ibraries/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/...icrosoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Helpers.cs b/...icrosoft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.Helpers.cs
@@ -160,5 +160,17 @@ public class CustomICollectionWithoutAnAddMethod : ICollection<string>
         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/...soft.Extensions.Configuration.Binder/tests/Common/ConfigurationBinderTests.TestClasses.cs b/...soft.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/...ns.Configuration.Binder/tests/SourceGenerationTests/ConfigurationBinderTests.Generator.cs b/...ns.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<int>("Latitude");
+            Assert.Equal(2, lat);
+
+            record = configuration.Get
+                <GeolocationRecord>();
+            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/...indingGeneratorTests.Baselines.Options.cs → ...Tests/GeneratorTests.Baselines.Options.cs b/...indingGeneratorTests.Baselines.Options.cs → ...Tests/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/...urationBindingGeneratorTests.Baselines.cs → ...nerationTests/GeneratorTests.Baselines.cs b/...urationBindingGeneratorTests.Baselines.cs → ...nerationTests/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<T> : ISet<T>
 
             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/...sts/ConfigurationBindingGeneratorTests.cs → ...s/SourceGenerationTests/GeneratorTests.cs b/...sts/ConfigurationBindingGeneratorTests.cs → ...s/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<string> 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<Diagnostic>, ImmutableArray<GeneratedSourceResult>)> RunGenerator(

diff --git a/...ceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj b/...ceGenerationTests/Microsoft.Extensions.Configuration.Binder.SourceGeneration.Tests.csproj
@@ -11,7 +11,7 @@
   <PropertyGroup>
     <DefineConstants>$(DefineConstants);BUILDING_SOURCE_GENERATOR_TESTS;ROSLYN4_0_OR_GREATER;ROSLYN4_4_OR_GREATER</DefineConstants>
     <DefineConstants Condition="'$(LaunchTestDebugger)' == 'true'">$(DefineConstants);LAUNCH_DEBUGGER</DefineConstants>
-    <DefineConstants Condition="'$(SkipBaselines)' == 'true'">$(DefineConstants);SKIP_BASELINES</DefineConstants>
+    <DefineConstants Condition="'$(UpdateBaselines)' == 'true'">$(DefineConstants);UPDATE_BASELINES</DefineConstants>
   </PropertyGroup>
 
   <ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
@@ -28,6 +28,7 @@
     <Compile Include="..\Common\ConfigurationBinderTests.Helpers.cs" Link="Common\ConfigurationBinderTests.Helpers.cs" />
     <Compile Include="..\Common\ConfigurationBinderTests.TestClasses.cs" Link="Common\ConfigurationBinderTests.TestClasses.cs" />
     <Compile Include="..\Common\ConfigurationBinderTests.TestClasses.Collections.cs" Link="Common\ConfigurationBinderTests.TestClasses.Collections.cs" />
+    <Compile Include="ConfigurationBinderTests.Generator.cs" />
   </ItemGroup>
 
   <ItemGroup>
@@ -50,9 +51,9 @@
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
 
-    <Compile Include="ConfigurationBindingGeneratorTests.cs" />
-    <Compile Include="ConfigurationBindingGeneratorTests.Baselines.cs" />
-    <Compile Include="ConfigurationBindingGeneratorTests.Baselines.Options.cs" />
+    <Compile Include="GeneratorTests.cs" />
+    <Compile Include="GeneratorTests.Baselines.cs" />
+    <Compile Include="GeneratorTests.Baselines.Options.cs" />
   </ItemGroup>
 
   <Target Name="FixIncrementalCoreCompileWithAnalyzers" BeforeTargets="CoreCompile">

diff --git a/...ptions.ConfigurationExtensions/tests/Common/OptionsBuilderConfigurationExtensionsTests.cs b/...ptions.ConfigurationExtensions/tests/Common/OptionsBuilderConfigurationExtensionsTests.cs
@@ -31,7 +31,8 @@ public static void BindConfiguration_ThrowsForNullConfigurationSectionPath()
 
             Assert.Throws<ArgumentNullException>("configSectionPath", () =>
             {
-                optionsBuilder.BindConfiguration(configSectionPath);
+                optionsBuilder
+                    .BindConfiguration(configSectionPath);
             });
         }
 
@@ -170,8 +171,8 @@ public static void BindConfiguration_UpdatesOptionOnConfigurationUpdate()
             services.AddSingleton<IConfiguration>(new ConfigurationBuilder()
                     .Add(configSource)
                     .Build());
-            OptionsBuilder<FakeOptions> optionsBuilder = services.AddOptions<FakeOptions>();
-            _ = optionsBuilder.BindConfiguration(configSectionName);
+            _ = services.AddOptions<FakeOptions>()
+                .BindConfiguration(configSectionName);
             using ServiceProvider serviceProvider = services.BuildServiceProvider();
             var optionsMonitor = serviceProvider.GetRequiredService<IOptionsMonitor<FakeOptions>>();
             bool updateHasRun = false;