Skip to content
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.

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

[API Proposal]: Add overload of Type.GetMethod that takes a name, generic parameter count, binding flags and parameter types #90995

Closed
Neme12 opened this issue Aug 23, 2023 · 6 comments · Fixed by #94889
Labels
api-approved API was approved in API review, it can be implemented area-System.Reflection help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@Neme12
Copy link

Neme12 commented Aug 23, 2023

Background and motivation

In #42753, an overload of Type.GetMethod was added that takes a name, binding flags, and paramater types, to simplify usage when people don't care about the binder and paramater modifiers (which as I understand is there only for COM), which basically nobody passes in ever. However no equivalent overload was added that also takes genericParameterCount at the beginning. This is the overload I always use, even when the method isn't generic and I pass in 0, to be as specific as possible and make sure that my code still works even in case there is a generic overload added in the future. It seems that all the other overloads have an equivalent that takes genericParameterCount at the beginning, so this would just square off the feature in my opinion.

API Proposal

 namespace System;

 public abstract partial class Type
 {
     public MethodInfo? GetMethod(string name, int genericParameterCount, Type[] types);
     public MethodInfo? GetMethod(string name, int genericParameterCount, Type[] types, ParameterModifier[]? modifiers);
+    public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Type[] types);
     public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder? binder, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder? binder, CallingConventions callConvention, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name);
     public MethodInfo? GetMethod(string name, Type[] types);
     public MethodInfo? GetMethod(string name, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Type[] types);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Binder? binder, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Binder? binder, CallingConventions callConvention, Type[] types, ParameterModifier[]? modifiers);
 }

API Usage

using System.Reflection;

var method = typeof(C).GetMethod("M", 0, BindingFlags.Public | BindingFlags.Instance, Array.Empty<Type>());
// or "Type.EmptyTypes" instead of "Array.Empty<Type>()"

class C
{
    public void M();
}

Alternative Designs

Original issue alternative (not proposed): optionally, to square off the set of overloads completely, with complete parity between the ones that take genericParameterCount and the ones that don't (7 overloads for each):

 namespace System;

 public abstract partial class Type
 {
+    public MethodInfo? GetMethod(string name, int genericParameterCount);
     public MethodInfo? GetMethod(string name, int genericParameterCount, Type[] types);
     public MethodInfo? GetMethod(string name, int genericParameterCount, Type[] types, ParameterModifier[]? modifiers);
+    public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr);
+    public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Type[] types);
     public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder? binder, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder? binder, CallingConventions callConvention, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name);
     public MethodInfo? GetMethod(string name, Type[] types);
     public MethodInfo? GetMethod(string name, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Type[] types);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Binder? binder, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Binder? binder, CallingConventions callConvention, Type[] types, ParameterModifier[]? modifiers);
 }

Risks

No response

@Neme12 Neme12 added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Aug 23, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 23, 2023
@ghost
Copy link

ghost commented Aug 23, 2023

Tagging subscribers to this area: @dotnet/area-system-reflection
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

In #42753, an overload of Type.GetMethod was added that takes a name, binding flags, and paramater types, to simplify usage when people don't care about the binder and paramater modifiers (which as I understand is there only for COM), which basically nobody passes in ever. However no equivalent overload was added that also takes genericParameterCount at the beginning. This is the overload I always use, even when the method isn't generic and I pass in 0, to be as specific as possible and make sure that my code still works even in case there is a generic overload added in the future. It seems that all the other overloads have an equivalent that takes genericParameterCount at the beginning, so this would just square off the feature in my opinion.

API Proposal

 namespace System;

 public abstract partial class Type
 {
     public System.Reflection.MethodInfo? GetMethod(string name) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, int genericParameterCount, System.Type[] types) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, int genericParameterCount, System.Type[] types, System.Reflection.ParameterModifier[]? modifiers) { throw null; }
+    public System.Reflection.MethodInfo? GetMethod(string name, int genericParameterCount, System.Reflection.BindingFlags bindingAttr, System.Type[] types) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, int genericParameterCount, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder? binder, System.Type[] types, System.Reflection.ParameterModifier[]? modifiers) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, int genericParameterCount, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder? binder, System.Reflection.CallingConventions callConvention, System.Type[] types, System.Reflection.ParameterModifier[]? modifiers) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, System.Type[] types) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, System.Type[] types, System.Reflection.ParameterModifier[]? modifiers) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, System.Reflection.BindingFlags bindingAttr) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, System.Reflection.BindingFlags bindingAttr, System.Type[] types) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder? binder, System.Type[] types, System.Reflection.ParameterModifier[]? modifiers) { throw null; }
     public System.Reflection.MethodInfo? GetMethod(string name, System.Reflection.BindingFlags bindingAttr, System.Reflection.Binder? binder, System.Reflection.CallingConventions callConvention, System.Type[] types, System.Reflection.ParameterModifier[]? modifiers) { throw null; }
 }

API Usage

using System.Reflection;

var method = typeof(C).GetMethod("M", 0, BindingFlags.Public | BindingFlags.Instance, Array.Empty<Type>());

class C
{
    public void M();
}

Alternative Designs

No response

Risks

No response

Author: Neme12
Assignees: -
Labels:

api-suggestion, area-System.Reflection

Milestone: -

@Neme12 Neme12 changed the title [API Proposal]: [API Proposal]: Add oveerload of Type.GetMethod that takes a name, generic parameter count, binding flags and parameter types Aug 23, 2023
@Neme12 Neme12 changed the title [API Proposal]: Add oveerload of Type.GetMethod that takes a name, generic parameter count, binding flags and parameter types [API Proposal]: Add overload of Type.GetMethod that takes a name, generic parameter count, binding flags and parameter types Aug 23, 2023
@steveharter
Copy link
Member

However no equivalent overload was added that also takes genericParameterCount at the beginning.

The original request would help the common cases:

public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Type[] types);

However, I'm not so sure about the other 2 since they remove the Type[] parameter which is a good practice for open-ended method lookup to disambiguate similar methods.

@steveharter steveharter removed the untriaged New issue has not been triaged by the area owner label Aug 24, 2023
@steveharter steveharter added this to the Future milestone Aug 24, 2023
@Neme12
Copy link
Author

Neme12 commented Aug 24, 2023

Right, I'm only interested in that one, I just added the proposal for the rest of them only as an idea if parity and consistency is desired.

@steveharter
Copy link
Member

I think it makes sense to add this to v9:

public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Type[] types);

@steveharter steveharter added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Nov 2, 2023
@steveharter steveharter modified the milestones: Future, 9.0.0 Nov 2, 2023
@steveharter steveharter added the help wanted [up-for-grabs] Good issue for external contributors label Nov 2, 2023
@TheMaximum
Copy link
Contributor

TheMaximum commented Nov 10, 2023

I'd be interested in taking this up if it gets through the API review :)

@bartonjs
Copy link
Member

bartonjs commented Nov 14, 2023

Video

Looks good as proposed.

 namespace System;

 public abstract partial class Type
 {
     public MethodInfo? GetMethod(string name, int genericParameterCount, Type[] types);
     public MethodInfo? GetMethod(string name, int genericParameterCount, Type[] types, ParameterModifier[]? modifiers);
+    public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Type[] types);
     public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder? binder, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, int genericParameterCount, BindingFlags bindingAttr, Binder? binder, CallingConventions callConvention, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name);
     public MethodInfo? GetMethod(string name, Type[] types);
     public MethodInfo? GetMethod(string name, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Type[] types);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Binder? binder, Type[] types, ParameterModifier[]? modifiers);
     public MethodInfo? GetMethod(string name, BindingFlags bindingAttr, Binder? binder, CallingConventions callConvention, Type[] types, ParameterModifier[]? modifiers);
 }

@bartonjs bartonjs added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Nov 14, 2023
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 17, 2023
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 21, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-approved API was approved in API review, it can be implemented area-System.Reflection help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants