Allow generate method to handle delegates #11402
Conversation
| { | ||
| eventParameterType = argument.ToString(); | ||
| if (attribute.IsGenericTypedProperty()) |
There was a problem hiding this comment.
Do we need to have an extra out parameter here to prevent "Generate Async Event Handler" showing up, or does the runtime happily call Action<T> delegates in an async context?
| /// <param name="tagHelper"></param> | ||
| /// <param name="binding"></param> | ||
| /// <param name="typeName"></param> | ||
| /// <returns></returns> |
There was a problem hiding this comment.
Consider removing these empty docstring tags.
| /// <returns></returns> | ||
| public static bool TryGetGenericTypeName(this TagHelperDescriptor tagHelper, TagHelperBinding binding, [NotNullWhen(true)] out string typeName) | ||
| { | ||
| if (!tagHelper.IsComponentTagHelper) |
There was a problem hiding this comment.
It looks like the method only works for components, not normal "tag helpers", consider renaming it and updating its doc string to make that clearer.
| /// <param name="binding"></param> | ||
| /// <param name="typeName"></param> | ||
| /// <returns></returns> | ||
| public static bool TryGetGenericTypeName(this TagHelperDescriptor tagHelper, TagHelperBinding binding, [NotNullWhen(true)] out string typeName) |
There was a problem hiding this comment.
I don't think [NotNullWhen(true)] has any effect since this whole file is #nullable disabled. Consider nullable-enabling the new code.
| // it has to be looked up in the bindingAttributes to find the value for that type. This assumes that the type is valid because the user | ||
| // provided it, and if it's not the calling context probably doesn't care. | ||
| if (boundAttribute.Metadata.TryGetValue(ComponentMetadata.Component.TypeParameterKey, out var value) && | ||
| string.Equals(bool.TrueString, value) && |
There was a problem hiding this comment.
Consider using TagHelperBoundAttributeDescriptorExtensions.IsTypeParameterProperty extension method.
| string.Equals(bool.TrueString, value) && | ||
| boundAttribute.Metadata.TryGetValue(TagHelperMetadata.Common.PropertyName, out var propertyName) && | ||
| !string.IsNullOrEmpty(propertyName) && | ||
| binding.Attributes.FirstOrDefault(kvp => kvp.Key == propertyName) is { Value: var bindingTypeName}) |
There was a problem hiding this comment.
| binding.Attributes.FirstOrDefault(kvp => kvp.Key == propertyName) is { Value: var bindingTypeName}) | |
| binding.Attributes.FirstOrDefault(propertyName, static (kvp, propertyName) => kvp.Key == propertyName) is { Value: var bindingTypeName }) |
| // provided it, and if it's not the calling context probably doesn't care. | ||
| if (boundAttribute.Metadata.TryGetValue(ComponentMetadata.Component.TypeParameterKey, out var value) && | ||
| string.Equals(bool.TrueString, value) && | ||
| boundAttribute.Metadata.TryGetValue(TagHelperMetadata.Common.PropertyName, out var propertyName) && |
There was a problem hiding this comment.
Consider using BoundAttributeDescriptorExtensions.GetPropertyName extension method.
| // Strip 'global::' from the candidate. | ||
| if (candidate.Span.StartsWith("global::".AsSpan())) | ||
| { | ||
| candidate = candidate["global::".Length..]; | ||
| } | ||
|
|
||
| if (!candidate.Span.StartsWith("System.Action".AsSpan())) | ||
| { | ||
| argument = default; | ||
| return false; | ||
| } | ||
|
|
||
|
|
||
| candidate = candidate["System.Action".Length..]; | ||
| if (candidate.Span is not ['<', .., '>']) | ||
| { | ||
| argument = default; | ||
| return false; | ||
| } | ||
|
|
||
| candidate = candidate[1..^1]; |
There was a problem hiding this comment.
Can't you reuse the method above for all this code?
| return false; | ||
| } | ||
|
|
||
| internal static bool TryGetGenericActionArgument(ReadOnlyMemory<char> candidate, string genericType, [NotNullWhen(true)] out string argument) |
There was a problem hiding this comment.
Consider adding a doc comment with an example to this method, because it's not obvious what it's doing.
| /// | ||
| /// <code> | ||
| /// <MyTagHelper | ||
| /// TItem="string" |
There was a problem hiding this comment.
If there are multiple generic arguments, this method will return just the first one, right? Is that okay (and should it be mentioned in the doc comment) or would it be better if the method returned false in that case?
...crosoft.CodeAnalysis.Razor.Compiler/src/Language/Components/TagHelperDescriptorExtensions.cs
Outdated
Show resolved
Hide resolved
...crosoft.CodeAnalysis.Razor.Compiler/src/Language/Components/TagHelperDescriptorExtensions.cs
Outdated
Show resolved
Hide resolved
...crosoft.CodeAnalysis.Razor.Compiler/src/Language/Components/TagHelperDescriptorExtensions.cs
Show resolved
Hide resolved
| @@ -94,6 +91,7 @@ public static bool TryGetGenericTypeName(this TagHelperDescriptor tagHelper, Tag | |||
| typeName = null; | |||
| return false; | |||
| } | |||
| #nullable restore | |||
There was a problem hiding this comment.
| #nullable restore | |
| #nullable disable |
Fixes #11380