Skip to content

Descriptive Error for Improper Use of [AsParameters] #58218

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 10, 2025
Merged

Descriptive Error for Improper Use of [AsParameters] #58218

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
11bdfe1
Instead of throwing there, we emit diagnostics when the type is invalid.
smnsht Nov 2, 2024
9ece4d0
Merge branch 'main' into 56114
smnsht Feb 27, 2025
358cedb
symbol-based comparison
smnsht Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Http/Http.Extensions/gen/DiagnosticDescriptors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,4 +125,13 @@ internal static class DiagnosticDescriptors
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
helpLinkUri: GetHelpLinkUrl("RDG013"));

public static DiagnosticDescriptor InvalidAsParametersEnumerableType { get; } = new(
"RDG014",
new LocalizableResourceString(nameof(Resources.InvalidAsParametersEnumerableType_Title), Resources.ResourceManager, typeof(Resources)),
new LocalizableResourceString(nameof(Resources.InvalidAsParametersEnumerableType_Message), Resources.ResourceManager, typeof(Resources)),
"Usage",
DiagnosticSeverity.Warning,
isEnabledByDefault: true,
helpLinkUri: GetHelpLinkUrl("RDG014"));
}
6 changes: 6 additions & 0 deletions src/Http/Http.Extensions/gen/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,10 @@
<data name="KeyedAndNotKeyedServiceAttributesNotSupported_Message" xml:space="preserve">
<value>The [FromKeyedServices] attribute is not supported on parameters that are also annotated with IFromServiceMetadata. For more information, please see https://aka.ms/aspnet/rdg-known-issues</value>
</data>
<data name="InvalidAsParametersEnumerableType_Title" xml:space="preserve">
<value>Invalid enumerable type</value>
</data>
<data name="InvalidAsParametersEnumerableType_Message" xml:space="preserve">
<value>The enumerable type '{0}' is not supported. For more information, please see https://aka.ms/aspnet/rdg-known-issues</value>
</data>
</root>
7 changes: 7 additions & 0 deletions src/Http/Http.Extensions/gen/StaticRouteHandlerModel/EndpointParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics.CodeAnalysis;
Expand Down Expand Up @@ -466,6 +467,12 @@ private static bool TryGetAsParametersConstructor(Endpoint endpoint, INamedTypeS
return false;
}

if (type.AllInterfaces.Any(x => x.ToString() == typeof(IEnumerable).FullName))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of doing a string-based comparison check, we should do a symbol-based comparison using the extension method that is defined here.

You'll need to add the IEnumerable type to the well-known types cache in this file then resolve it from the WellKnownTypes cache based to this call path.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will do this in February.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for letting me know! I removed the pr: pending author input label so that the bot doesn't out-close your PR in the meantime.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

symbol-based comparison - done.
Appreciate the tip!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update looks good! I've merged this in. Should come out in .NET 10 Preview 3.

{
endpoint.Diagnostics.Add(Diagnostic.Create(DiagnosticDescriptors.InvalidAsParametersEnumerableType, location, parameterTypeString));
return false;
}

var constructors = type.Constructors.Where(constructor => constructor.DeclaredAccessibility == Accessibility.Public && !constructor.IsStatic);
var numOfConstructors = constructors.Count();
// When leveraging parameterless constructors, we want to ensure we only emit for writable
Expand Down
6 changes: 6 additions & 0 deletions ...tp/Http.Extensions/test/RequestDelegateGenerator/CompileTimeCreationTests.AsParameters.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,19 @@ static string GetNoContructorsError(Type type)
static string GetInvalidConstructorError(Type type)
=> $"The public parameterized constructor must contain only parameters that match the declared public properties for type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}'. For more information, please see https://aka.ms/aspnet/rdg-known-issues";

static string GetEnumerableTypeError(Type type)
=> $"The enumerable type '{TypeNameHelper.GetTypeDisplayName(type, fullName: false)}' is not supported. For more information, please see https://aka.ms/aspnet/rdg-known-issues";

return new []
{
new object[] { "BadArgumentListRecord", DiagnosticDescriptors.InvalidAsParametersSingleConstructorOnly.Id, GetMultipleContructorsError(typeof(BadArgumentListRecord)) },
new object[] { "BadArgumentListClass", DiagnosticDescriptors.InvalidAsParametersSignature.Id, GetInvalidConstructorError(typeof(BadArgumentListClass)) },
new object[] { "BadArgumentListClassMultipleCtors", DiagnosticDescriptors.InvalidAsParametersSingleConstructorOnly.Id, GetMultipleContructorsError(typeof(BadArgumentListClassMultipleCtors)) },
new object[] { "BadAbstractArgumentListClass", DiagnosticDescriptors.InvalidAsParametersAbstractType.Id, GetAbstractTypeError(typeof(BadAbstractArgumentListClass)) },
new object[] { "BadNoPublicConstructorArgumentListClass", DiagnosticDescriptors.InvalidAsParametersNoConstructorFound.Id, GetNoContructorsError(typeof(BadNoPublicConstructorArgumentListClass)) },
new object[] { "List<string>", DiagnosticDescriptors.InvalidAsParametersEnumerableType.Id, GetEnumerableTypeError(typeof(List<string>)) },
new object[] { "List<IFormFile>", DiagnosticDescriptors.InvalidAsParametersEnumerableType.Id, GetEnumerableTypeError(typeof(List<IFormFile>)) },
new object[] { "Dictionary<string, string>", DiagnosticDescriptors.InvalidAsParametersEnumerableType.Id, GetEnumerableTypeError(typeof(Dictionary<string, string>)) }
};
}
}
Expand Down
Loading