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

Generic attributes: Attribute.GetCustomAttributes returns null when passed an unbound generic type #64335

Closed
Tracked by #44655
canton7 opened this issue Jan 26, 2022 · 10 comments · Fixed by #65237 or #66549
Closed
Tracked by #44655

Comments

@canton7
Copy link

canton7 commented Jan 26, 2022

Description

Attribute.GetCustomAttributes normally returns an empty array when no attributes are found (or throws an exception if the parameters don't make sense).

However, Attribute.GetCustomAttributes(someType, typeof(SomeGenericAttribute<>)) returns null. This is unexpected, particularly as the nullable annotations say that it doesn't return null.

It's worth noting that this wasn't previously possible: it was simply not possible to create a generic type which derived from Attribute, and if you passed a type which wasn't derived from Attribute to Attribute.GetCustomAttributes, you got an exception telling you that. So I suspect this is an unhandled case which just fell through the cracks.

Reproduction Steps

Test.M();

public class Test
{
    public static void M()
    {
        var attrs = Attribute.GetCustomAttributes(typeof(Test), typeof(TestAttribute<>));
        Console.WriteLine(attrs == null); // true
    }
}

public class TestAttribute<T> : Attribute { }

SharpLab

Expected behavior

Attribute.GetCustomAttributes either returns an array, or throws an exception. It should not return null.

Actual behavior

Attribute.GetCustomAttributes returns null in this case.

Regression?

No. It was not previously possible to pass an unbound generic attribute type to Attribute.GetCustomAttributes.

Known Workarounds

No response

Configuration

No response

Other information

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Reflection untriaged New issue has not been triaged by the area owner labels Jan 26, 2022
@ghost
Copy link

ghost commented Jan 26, 2022

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

Issue Details

Description

Attribute.GetCustomAttributes normally returns an empty array when no attributes are found (or throws an exception if the parameters don't make sense).

However, Attribute.GetCustomAttributes(someType, typeof(SomeGenericAttribute<>)) returns null. This is unexpected, particularly as the nullable annotations say that it doesn't return null.

It's worth noting that this wasn't previously possible: it was simply not possible to create a generic type which derived from Attribute, and if you passed a type which wasn't derived from Attribute to Attribute.GetCustomAttributes, you got an exception telling you that. So I suspect this is an unhandled case which just fell through the cracks.

Reproduction Steps

new Test().M();

public class Test
{
    public void M()
    {
        var attrs = Attribute.GetCustomAttributes(this.GetType(), typeof(TestAttribute<>));
        Console.WriteLine(attrs == null); // true
    }
}

public class TestAttribute<T> : Attribute { }

SharpLab

Expected behavior

Attribute.GetCustomAttributes either returns an empty array, or throws an exception.

Actual behavior

Attribute.GetCustomAttributes returns null in this case.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

No response

Author: canton7
Assignees: -
Labels:

area-System.Reflection, untriaged

Milestone: -

@canton7 canton7 changed the title Attribute.GetCustomAttributes returns null when passed an unbound generic type Generic attributes: Attribute.GetCustomAttributes returns null when passed an unbound generic type Jan 26, 2022
@VBAndCs
Copy link

VBAndCs commented Jan 26, 2022

I have asked to accept the open generic form and return all generic attributes of this type #64169

@CyrusNajmabadi
Copy link
Member

@VBAndCs you didn't address teh question i asked you there. :)

@jkotas
Copy link
Member

jkotas commented Jan 26, 2022

The bug is caused by ! at this line

_ => (element.GetCustomAttributes(typeof(Attribute), inherit) as Attribute[])!,
:

element.GetCustomAttributes returns an empty object[] since it is not possible to create array with open generic element type. Casting object[] to Attribute[] using as is producing null.

@VBAndCs
Copy link

VBAndCs commented Jan 26, 2022

@jkotas

since it is not possible to create array with open generic element type

You can return an Attrubute array containing all the generic versions of this open generic attribute.

@buyaa-n buyaa-n added bug and removed untriaged New issue has not been triaged by the area owner labels Jan 28, 2022
@buyaa-n buyaa-n added this to the 7.0.0 milestone Jan 28, 2022
@buyaa-n buyaa-n added the help wanted [up-for-grabs] Good issue for external contributors label Jan 28, 2022
@Joe4evr
Copy link
Contributor

Joe4evr commented Jan 31, 2022

So I took a look at this, and I think the best solution would be down around here:

bool useObjectArray = (caType.IsValueType || caType.ContainsGenericParameters);
RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : caType;

While my gut reaction is to possibly rethink some parts of this implementation, there's probably more involved that I can't see and may end up breaking assumptions elsewhere. In which case, the most effective fix here might be to add a check whether or not caType.IsAssignableTo(typeof(Attribute)) and if so, set arrayType = (RuntimeType)typeof(Attribute) for the array creation.

Effectively:

             bool useObjectArray = (caType.IsValueType || caType.ContainsGenericParameters);
+            bool useAttributeArray = (caType.ContainsGenericParameters && caType.IsAssignableTo(typeof(Attribute)));
-            RuntimeType arrayType = useObjectArray ? (RuntimeType)typeof(object) : caType;
+            RuntimeType arrayType = useAttributeArray ? (RuntimeType)typeof(Attribute) : useObjectArray ? (RuntimeType)typeof(object) : caType;

(and there's a total of 3 places in the file for this.)

@madelson
Copy link
Contributor

@Joe4evr / @buyaa-n I'd be interested in fixing this if it is still up for grabs.

@buyaa-n
Copy link
Contributor

buyaa-n commented Feb 11, 2022

@Joe4evr are working on the fix? if not i will assign to @madelson, thanks!

@Joe4evr
Copy link
Contributor

Joe4evr commented Feb 11, 2022

@buyaa-n I was trying to, but I just ran out of time over the past week, so it's cool.

@buyaa-n buyaa-n assigned buyaa-n and madelson and unassigned buyaa-n Feb 11, 2022
@buyaa-n buyaa-n removed the help wanted [up-for-grabs] Good issue for external contributors label Feb 11, 2022
madelson added a commit to madelson/runtime that referenced this issue Feb 12, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 12, 2022
madelson added a commit to madelson/runtime that referenced this issue Feb 12, 2022
Previously, this test asserted that GetCustomAttributes on an open generic
type would return null. Now we return empty instead.
buyaa-n pushed a commit that referenced this issue Mar 4, 2022
… type (#65237)

* Avoid Attribute.GetCustomAttributes() returning null for open generic type.

Fix #64335
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Mar 4, 2022
@stephentoub
Copy link
Member

Reopening as #65237 was reverted in #66508 due to #66496.

@stephentoub stephentoub reopened this Mar 11, 2022
@buyaa-n buyaa-n mentioned this issue Mar 14, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.