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

Exception thrown for non-generic method inside generic class #93

Closed
eestein opened this issue Oct 29, 2021 · 8 comments
Closed

Exception thrown for non-generic method inside generic class #93

eestein opened this issue Oct 29, 2021 · 8 comments
Assignees
Labels
Bug issues that suggest a flaw with existing code

Comments

@eestein
Copy link

eestein commented Oct 29, 2021

The exception:

System.InvalidOperationException: Sequence contains no matching element
at System.Linq.ThrowHelper.ThrowNoMatchException()
at System.Linq.Enumerable.First[TSource](IEnumerable`1 source, Func`2 predicate)
at Towel.Meta.GetXmlNameMethodBase(MethodInfo methodInfo, ConstructorInfo constructorInfo)
at Towel.Meta.GetXmlName(MethodInfo methodInfo)
at Towel.Meta.GetDocumentation(MethodInfo methodInfo)

image

This exception happens on Meta.cs, line 846:

methodInfo = methodInfo.DeclaringType.GetGenericTypeDefinition().GetMethods().First(x => x.MetadataToken == methodInfo.MetadataToken);

My method is not generic and the class is, is it still required to update the method info in that case?

image

Structure:

public class GenericClass<TType> 
{
    public void Dispose()
    {
    }
}

The problem is, the MetadataToken for the methods are different, please take a look:

This is the value for the original method info (the one that called GetDocumentation:

image

And this is the value for the one in the call response:

image

@ZacharyPatten please let me know if you need more information, I'm also trying to figure it out on my end.

@eestein eestein added the Bug issues that suggest a flaw with existing code label Oct 29, 2021
@ZacharyPatten ZacharyPatten self-assigned this Oct 29, 2021
@ZacharyPatten
Copy link
Owner

Can you share the code of how you get the MethodInfo that you are calling GetDocumentation on?

Using the MetadataToken was a way to fix #52, but it looks like that introduced a bug when I added that. I just need to be able to replicate the issue in a unit test.

@eestein
Copy link
Author

eestein commented Oct 29, 2021

Sure, there you go.

Type being evaluated:

/// <summary>
/// Comment.
/// </summary>
public class GenericClass<TType> : IInterfaceA, IInterfaceB<TType>
    where TType : class
{
    // ...

    /// <summary>
    /// Comment.
    /// </summary>
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}

This is the code that uses your library:

var members = type.GetMembers(
    BindingFlags.Static | BindingFlags.Public | BindingFlags.Instance | BindingFlags.NonPublic
).Where(m => !m.Name.StartsWith("get_") && !m.Name.StartsWith("set_"));

foreach (var memberInfo in members)
{
    switch (memberInfo.MemberType)
    {
        // ...

        case MemberTypes.Method:
            var method = (MethodInfo)memberInfo;

            if (method.IsPrivate)
            {
                continue;
            }

            var methodDoc = method.GetDocumentation();
    }
}

Is this helpful?

@eestein
Copy link
Author

eestein commented Oct 29, 2021

@ZacharyPatten I don't know if it could break some other part of the code, but by changing it to:

if (methodInfo.DeclaringType.IsGenericType && methodInfo.IsGenericMethod)
{
    methodInfo = methodInfo.DeclaringType.GetGenericTypeDefinition().GetMethods(
        BindingFlags.Static | BindingFlags.Public | BindingFlags.Instance | BindingFlags.NonPublic
    ).First(x => x.MetadataToken == methodInfo.MetadataToken);
}

Because protected methods were not being returned.

it works.

I don't know if it could break something else, I'm still testing it, but at least for the first scenario it works.

@eestein
Copy link
Author

eestein commented Oct 29, 2021

I just tested that solution with my full project and it seems to be working.
I'm going to use that local version, and in the meantime, I'm still going to test it further.
I'll keep you posted, but I'm confident this version is working, I'll just ship it as is, for now.

Thanks for your help!

@ZacharyPatten
Copy link
Owner

ZacharyPatten commented Oct 29, 2021

Ah... you are 100% correct. I never added the BindingFlags to the GetMethods call inside the GetXmlNameMethodBase method; the default BondingFlags only gets public members. That was just an oversight on my part.

If you would like to open a pull request I would gladly pull in this code fix. That way you can get credit for the fix. :) Otherwise I can fix it myself. Let me know.

I will also add a unit test for this to ensure it will be covered in future regression testing.

@eestein
Copy link
Author

eestein commented Oct 29, 2021

Awesome, I don't mind the credit 😅
I think it's easier on you and for you to just copy/paste and push if everything's OK.

Again, thank you for your work!
I'll keep you posted if I find anything

@ZacharyPatten
Copy link
Owner

ZacharyPatten commented Oct 29, 2021

This fix was deployed in https://github.com/ZacharyPatten/Towel/releases/tag/1.0.40

Sorry you ran into another bug, but thank you for addressing it so it could be fixed. :)

As always, if the fix did not fully resolve the issue feel free to re-open.

@eestein
Copy link
Author

eestein commented Oct 29, 2021

@ZacharyPatten thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug issues that suggest a flaw with existing code
Projects
None yet
Development

No branches or pull requests

2 participants