Fix shared code references in analyzers and address feedback

captainsafia · captainsafia · commit 001e108bf09d · 2023-05-07T22:48:09.000-07:00
diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/AtMostOneFromBodyAttribute.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/AtMostOneFromBodyAttribute.cs
@@ -27,7 +27,7 @@ private static void AtMostOneFromBodyAttribute(
         foreach (var asParameterDecoratedParameter in asParametersDecoratedParameters)
         {
             var fromBodyMetadataInterfaceMembers = asParameterDecoratedParameter.Type.GetMembers().Where(
-                m => m.TryGetAttributeImplementingInterface(fromBodyMetadataInterfaceType)
+                m => m.HasAttributeImplementingInterface(fromBodyMetadataInterfaceType)
                 );
 
             if (fromBodyMetadataInterfaceMembers.Count() >= 2)
@@ -36,7 +36,7 @@ private static void AtMostOneFromBodyAttribute(
             }
         }
 
-        var fromBodyMetadataInterfaceParameters = methodSymbol.Parameters.Where(p => p.TryGetAttributeImplementingInterface(fromBodyMetadataInterfaceType));
+        var fromBodyMetadataInterfaceParameters = methodSymbol.Parameters.Where(p => p.HasAttributeImplementingInterface(fromBodyMetadataInterfaceType));
 
         if (fromBodyMetadataInterfaceParameters.Count() >= 2)
         {
diff --git a/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs b/src/Framework/AspNetCoreAnalyzers/src/Analyzers/RouteHandlers/DisallowNonParsableComplexTypesOnParameters.cs
@@ -98,7 +98,7 @@ static bool ReportFromAttributeDiagnostic(OperationAnalysisContext context, Well
         {
             var fromMetadataInterfaceTypeSymbol = wellKnownTypes.Get(fromMetadataInterfaceType);
             var parsability = ParsabilityHelper.GetParsability(parameterTypeSymbol, wellKnownTypes);
-            if (parameter.TryGetAttributeImplementingInterface(fromMetadataInterfaceTypeSymbol) && parsability != Parsability.Parsable)
+            if (parameter.HasAttributeImplementingInterface(fromMetadataInterfaceTypeSymbol) && parsability != Parsability.Parsable)
             {
                 context.ReportDiagnostic(Diagnostic.Create(
                     DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable,
diff --git a/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameter.cs b/src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameter.cs
@@ -142,16 +142,16 @@ private void ProcessEndpointParameterSource(Endpoint endpoint, ISymbol symbol, I
             }
             if (symbol is IPropertySymbol ||
                 Type is not INamedTypeSymbol namedTypeSymbol ||
-                !TryGetAsParametersConstructor(endpoint, namedTypeSymbol, out var isParameterlessConstructor, out var matchedParameters))
+                !TryGetAsParametersConstructor(endpoint, namedTypeSymbol, out var isDefaultConstructor, out var matchedProperties))
             {
                 if (symbol is IPropertySymbol)
                 {
                     endpoint.Diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.InvalidAsParametersNested, location));
                 }
                 return;
             }
-            EndpointParameters = matchedParameters.Select(matchedParameter => new EndpointParameter(endpoint, matchedParameter.Property, matchedParameter.Parameter, wellKnownTypes));
-            if (isParameterlessConstructor == true)
+            EndpointParameters = matchedProperties.Select(matchedParameter => new EndpointParameter(endpoint, matchedParameter.Property, matchedParameter.Parameter, wellKnownTypes));
+            if (isDefaultConstructor == true)
             {
                 var parameterList = string.Join(", ", EndpointParameters.Select(p => $"{p.LookupName} = {p.EmitHandlerArgument()}"));
                 AssigningCode = $"new {namedTypeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {{ {parameterList} }}";
@@ -471,10 +471,10 @@ private static string GetEscapedParameterName(AttributeData attribute, string pa
         }
     }
 
-    private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeSymbol type, out bool? isParameterlessConstructor, [NotNullWhen(true)] out IEnumerable<ConstructorParameter>? matchedParameters)
+    private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeSymbol type, out bool? isDefaultConstructor, [NotNullWhen(true)] out IEnumerable<ConstructorParameter>? matchedProperties)
     {
-        isParameterlessConstructor = null;
-        matchedParameters = null;
+        isDefaultConstructor = null;
+        matchedProperties = null;
         var parameterTypeString = type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat);
         var location = endpoint.Operation.Syntax.GetLocation();
         if (type.IsAbstract)
@@ -492,7 +492,7 @@ private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeS
 
         if (numOfConstructors == 1)
         {
-            var targetConstructor = constructors.SingleOrDefault();
+            var targetConstructor = constructors.Single();
             var lookupTable = new Dictionary<ParameterLookupKey, IPropertySymbol>();
             foreach (var property in properties)
             {
@@ -504,8 +504,8 @@ private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeS
 
             if (parameters.Length == 0)
             {
-                isParameterlessConstructor = true;
-                matchedParameters = writableProperties.Select(property => new ConstructorParameter(property, null));
+                isDefaultConstructor = true;
+                matchedProperties = writableProperties.Select(property => new ConstructorParameter(property, null));
                 return true;
             }
 
@@ -523,15 +523,15 @@ private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeS
                 }
             }
 
-            isParameterlessConstructor = false;
-            matchedParameters = propertiesWithParameterInfo;
+            isDefaultConstructor = false;
+            matchedProperties = propertiesWithParameterInfo;
             return true;
         }
 
         if (type.IsValueType)
         {
-            isParameterlessConstructor = true;
-            matchedParameters = writableProperties.Select(property => new ConstructorParameter(property, null));
+            isDefaultConstructor = true;
+            matchedProperties = writableProperties.Select(property => new ConstructorParameter(property, null));
             return true;
         }
 
diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs
@@ -1495,7 +1495,7 @@ private static Expression BindParameterFromProperties(ParameterInfo parameter, R
             for (var i = 0; i < properties.Length; i++)
             {
                 // For parameterless ctor we will init only writable properties.
-                if (properties[i].CanWrite)
+                if (properties[i].CanWrite && properties[i].GetSetMethod(nonPublic: false) != null)
                 {
                     var parameterInfo = new PropertyAsParameterInfo(properties[i], factoryContext.NullabilityContext);
                     bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext)));
diff --git a/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.AsParameters.cs b/src/Http/Http.Extensions/test/RequestDelegateGenerator/RequestDelegateCreationTests.AsParameters.cs
@@ -290,13 +290,15 @@ void TestAction(HttpContext context, [AsParameters] ParameterListWithReadOnlyPro
         httpContext.Request.RouteValues[nameof(ParameterListWithReadOnlyProperties.Value)] = routeParamValue.ToString(NumberFormatInfo.InvariantInfo);
         httpContext.Request.RouteValues[nameof(ParameterListWithReadOnlyProperties.ConstantValue)] = routeParamValue.ToString(NumberFormatInfo.InvariantInfo);
         httpContext.Request.RouteValues[nameof(ParameterListWithReadOnlyProperties.ReadOnlyValue)] = routeParamValue.ToString(NumberFormatInfo.InvariantInfo);
+        httpContext.Request.RouteValues[nameof(ParameterListWithReadOnlyProperties.PrivateSetValue)] = routeParamValue.ToString(NumberFormatInfo.InvariantInfo);
 
         await endpoint.RequestDelegate(httpContext);
 
         var input = Assert.IsType<ParameterListWithReadOnlyProperties>(httpContext.Items["input"]);
         Assert.Equal(expectedInput.Value, input.Value);
         Assert.Equal(expectedInput.ConstantValue, input.ConstantValue);
         Assert.Equal(expectedInput.ReadOnlyValue, input.ReadOnlyValue);
+        Assert.Equal(expectedInput.PrivateSetValue, input.PrivateSetValue);
     }
 
     [Fact]
diff --git a/src/Http/Http.Extensions/test/RequestDelegateGenerator/SharedTypes.cs b/src/Http/Http.Extensions/test/RequestDelegateGenerator/SharedTypes.cs
@@ -799,13 +799,16 @@ public class ParameterListWithReadOnlyProperties
     public ParameterListWithReadOnlyProperties()
     {
         ReadOnlyValue = 1;
+        PrivateSetValue = 2;
     }
 
     public int Value { get; set; }
 
     public int ConstantValue => 1;
 
     public int ReadOnlyValue { get; }
+
+    public int PrivateSetValue { get; private set;  }
 }
 
 public record struct SampleParameterList(int Foo);

Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ private static void AtMostOneFromBodyAttribute(`
`27`	`27`	`foreach (var asParameterDecoratedParameter in asParametersDecoratedParameters)`
`28`	`28`	`{`
`29`	`29`	`var fromBodyMetadataInterfaceMembers = asParameterDecoratedParameter.Type.GetMembers().Where(`
`30`		`- m => m.TryGetAttributeImplementingInterface(fromBodyMetadataInterfaceType)`
	`30`	`+ m => m.HasAttributeImplementingInterface(fromBodyMetadataInterfaceType)`
`31`	`31`	`);`
`32`	`32`
`33`	`33`	`if (fromBodyMetadataInterfaceMembers.Count() >= 2)`
`@@ -36,7 +36,7 @@ private static void AtMostOneFromBodyAttribute(`
`36`	`36`	`}`
`37`	`37`	`}`
`38`	`38`
`39`		`- var fromBodyMetadataInterfaceParameters = methodSymbol.Parameters.Where(p => p.TryGetAttributeImplementingInterface(fromBodyMetadataInterfaceType));`
	`39`	`+ var fromBodyMetadataInterfaceParameters = methodSymbol.Parameters.Where(p => p.HasAttributeImplementingInterface(fromBodyMetadataInterfaceType));`
`40`	`40`
`41`	`41`	`if (fromBodyMetadataInterfaceParameters.Count() >= 2)`
`42`	`42`	`{`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ static bool ReportFromAttributeDiagnostic(OperationAnalysisContext context, Well`
`98`	`98`	`{`
`99`	`99`	`var fromMetadataInterfaceTypeSymbol = wellKnownTypes.Get(fromMetadataInterfaceType);`
`100`	`100`	`var parsability = ParsabilityHelper.GetParsability(parameterTypeSymbol, wellKnownTypes);`
`101`		`- if (parameter.TryGetAttributeImplementingInterface(fromMetadataInterfaceTypeSymbol) && parsability != Parsability.Parsable)`
	`101`	`+ if (parameter.HasAttributeImplementingInterface(fromMetadataInterfaceTypeSymbol) && parsability != Parsability.Parsable)`
`102`	`102`	`{`
`103`	`103`	`context.ReportDiagnostic(Diagnostic.Create(`
`104`	`104`	`DiagnosticDescriptors.RouteParameterComplexTypeIsNotParsable,`
Original file line number	Diff line number	Diff line change
`@@ -142,16 +142,16 @@ private void ProcessEndpointParameterSource(Endpoint endpoint, ISymbol symbol, I`
`142`	`142`	`}`
`143`	`143`	`if (symbol is IPropertySymbol \|\|`
`144`	`144`	`Type is not INamedTypeSymbol namedTypeSymbol \|\|`
`145`		`- !TryGetAsParametersConstructor(endpoint, namedTypeSymbol, out var isParameterlessConstructor, out var matchedParameters))`
	`145`	`+ !TryGetAsParametersConstructor(endpoint, namedTypeSymbol, out var isDefaultConstructor, out var matchedProperties))`
`146`	`146`	`{`
`147`	`147`	`if (symbol is IPropertySymbol)`
`148`	`148`	`{`
`149`	`149`	`endpoint.Diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.InvalidAsParametersNested, location));`
`150`	`150`	`}`
`151`	`151`	`return;`
`152`	`152`	`}`
`153`		`- EndpointParameters = matchedParameters.Select(matchedParameter => new EndpointParameter(endpoint, matchedParameter.Property, matchedParameter.Parameter, wellKnownTypes));`
`154`		`- if (isParameterlessConstructor == true)`
	`153`	`+ EndpointParameters = matchedProperties.Select(matchedParameter => new EndpointParameter(endpoint, matchedParameter.Property, matchedParameter.Parameter, wellKnownTypes));`
	`154`	`+ if (isDefaultConstructor == true)`
`155`	`155`	`{`
`156`	`156`	`var parameterList = string.Join(", ", EndpointParameters.Select(p => $"{p.LookupName} = {p.EmitHandlerArgument()}"));`
`157`	`157`	`AssigningCode = $"new {namedTypeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat)} {{ {parameterList} }}";`
`@@ -471,10 +471,10 @@ private static string GetEscapedParameterName(AttributeData attribute, string pa`
`471`	`471`	`}`
`472`	`472`	`}`
`473`	`473`
`474`		`- private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeSymbol type, out bool? isParameterlessConstructor, [NotNullWhen(true)] out IEnumerable<ConstructorParameter>? matchedParameters)`
	`474`	`+ private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeSymbol type, out bool? isDefaultConstructor, [NotNullWhen(true)] out IEnumerable<ConstructorParameter>? matchedProperties)`
`475`	`475`	`{`
`476`		`- isParameterlessConstructor = null;`
`477`		`- matchedParameters = null;`
	`476`	`+ isDefaultConstructor = null;`
	`477`	`+ matchedProperties = null;`
`478`	`478`	`var parameterTypeString = type.ToDisplayString(SymbolDisplayFormat.CSharpShortErrorMessageFormat);`
`479`	`479`	`var location = endpoint.Operation.Syntax.GetLocation();`
`480`	`480`	`if (type.IsAbstract)`
`@@ -492,7 +492,7 @@ private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeS`
`492`	`492`
`493`	`493`	`if (numOfConstructors == 1)`
`494`	`494`	`{`
`495`		`- var targetConstructor = constructors.SingleOrDefault();`
	`495`	`+ var targetConstructor = constructors.Single();`
`496`	`496`	`var lookupTable = new Dictionary<ParameterLookupKey, IPropertySymbol>();`
`497`	`497`	`foreach (var property in properties)`
`498`	`498`	`{`
`@@ -504,8 +504,8 @@ private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeS`
`504`	`504`
`505`	`505`	`if (parameters.Length == 0)`
`506`	`506`	`{`
`507`		`- isParameterlessConstructor = true;`
`508`		`- matchedParameters = writableProperties.Select(property => new ConstructorParameter(property, null));`
	`507`	`+ isDefaultConstructor = true;`
	`508`	`+ matchedProperties = writableProperties.Select(property => new ConstructorParameter(property, null));`
`509`	`509`	`return true;`
`510`	`510`	`}`
`511`	`511`
`@@ -523,15 +523,15 @@ private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeS`
`523`	`523`	`}`
`524`	`524`	`}`
`525`	`525`
`526`		`- isParameterlessConstructor = false;`
`527`		`- matchedParameters = propertiesWithParameterInfo;`
	`526`	`+ isDefaultConstructor = false;`
	`527`	`+ matchedProperties = propertiesWithParameterInfo;`
`528`	`528`	`return true;`
`529`	`529`	`}`
`530`	`530`
`531`	`531`	`if (type.IsValueType)`
`532`	`532`	`{`
`533`		`- isParameterlessConstructor = true;`
`534`		`- matchedParameters = writableProperties.Select(property => new ConstructorParameter(property, null));`
	`533`	`+ isDefaultConstructor = true;`
	`534`	`+ matchedProperties = writableProperties.Select(property => new ConstructorParameter(property, null));`
`535`	`535`	`return true;`
`536`	`536`	`}`
`537`	`537`
Original file line number	Diff line number	Diff line change
`@@ -1495,7 +1495,7 @@ private static Expression BindParameterFromProperties(ParameterInfo parameter, R`
`1495`	`1495`	`for (var i = 0; i < properties.Length; i++)`
`1496`	`1496`	`{`
`1497`	`1497`	`// For parameterless ctor we will init only writable properties.`
`1498`		`- if (properties[i].CanWrite)`
	`1498`	`+ if (properties[i].CanWrite && properties[i].GetSetMethod(nonPublic: false) != null)`
`1499`	`1499`	`{`
`1500`	`1500`	`var parameterInfo = new PropertyAsParameterInfo(properties[i], factoryContext.NullabilityContext);`
`1501`	`1501`	`bindings.Add(Expression.Bind(properties[i], CreateArgument(parameterInfo, factoryContext)));`
Original file line number	Diff line number	Diff line change
`@@ -799,13 +799,16 @@ public class ParameterListWithReadOnlyProperties`
`799`	`799`	`public ParameterListWithReadOnlyProperties()`
`800`	`800`	`{`
`801`	`801`	`ReadOnlyValue = 1;`
	`802`	`+ PrivateSetValue = 2;`
`802`	`803`	`}`
`803`	`804`
`804`	`805`	`public int Value { get; set; }`
`805`	`806`
`806`	`807`	`public int ConstantValue => 1;`
`807`	`808`
`808`	`809`	`public int ReadOnlyValue { get; }`
	`810`	`+`
	`811`	`+ public int PrivateSetValue { get; private set; }`
`809`	`812`	`}`
`810`	`813`
`811`	`814`	`public record struct SampleParameterList(int Foo);`