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

Analyzer: Replace occurrences of TypeInfo with Type and GetTypeInfo() with GetType() #61122

Open
buyaa-n opened this issue Nov 2, 2021 · 3 comments
Labels
api-approved API was approved in API review, it can be implemented area-System.Reflection code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer help wanted [up-for-grabs] Good issue for external contributors
Milestone

Comments

@buyaa-n
Copy link
Contributor

buyaa-n commented Nov 2, 2021

Related to #53217 (comment)
For obsoleting TypeInfo/GetTypeInfo we need an analyzer/code fixer that replaces GetTypeInfo() with calls to GetType(), and occurrences of TypeInfo with Type. A heavy reflection code base that migrated to Windows 8/was written for the Windows 8 era, would have had widely used both. The key would be making it easier for folks to replace them when the type is obsolete.

class DerivedClass : TypeInfo // Do not flag base types
{
   //  ...
}
class MyClass
{
    // I guess we don't want to warn/fix return type of a public/protected methods (maybe make it configurable)
    // flag/fix return type only for private methods by default
    // flag fix all local variables including the parameters
    TypeInfo GetTypeInfo(TypeInfo ti) => null;
    // flag/fix only private properties by default
    TypeInfo TypeInfo => null;
    // flag/fix only private fields by default
    private TypeInfo typeInfo = null;

    void ExampleBeforeFix()
    {
        TypeInfo ti = typeof(MyClass).GetTypeInfo();
        TypeInfo anotherInfo = GetTypeInfo(null); // TypeInfo comes from other API

        // possible references needs updated:
        IEnumerable<ConstructorInfo> constructors = ti.DeclaredConstructors;
        IEnumerable<EventInfo> events = ti.DeclaredEvents;
        IEnumerable<FieldInfo> fields = ti.DeclaredFields;
        IEnumerable<MemberInfo> memebrs = ti.DeclaredMembers;
        IEnumerable<MethodInfo> methods = ti.DeclaredMethods;
        IEnumerable<TypeInfo> nTypes = ti.DeclaredNestedTypes;
        IEnumerable<PropertyInfo> properties = ti.DeclaredProperties;
        Type[] genTypeParams = ti.GenericTypeParameters;
        IEnumerable<Type> interfaces = ti.ImplementedInterfaces;
        Type t = ti.AsType();
        EventInfo ei = ti.GetDeclaredEvent(name);
        FieldInfo fi = ti.GetDeclaredField(name);
        MethodInfo mi = ti.GetDeclaredMethod(name);
        IEnumerable<MethodInfo> mInfos = ti.GetDeclaredMethods(name);
        TypeInfo tInfo = ti.GetDeclaredNestedType(name);
        PropertyInfo pi = ti.GetDeclaredProperty(name);
        if (ti.IsAssignableFrom(anotherInfo));
        TypeInfo ati = ti.GetTypeInfo();
    }

    void ExampleAfterFix()
    {
        Type ti = typeof(MyClass);
        Type anotherInfo = GetTypeInfo(null); // TypeInfo comes from custom method

        // possible fixes for references
        IEnumerable<ConstructorInfo> constructors = ti.GetConstructors(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        IEnumerable<EventInfo> events = ti.GetEvents(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        IEnumerable<FieldInfo> fields = ti.GetFields(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        IEnumerable<MemberInfo> memebrs = ti.GetMembers(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        IEnumerable<MethodInfo> methods = ti.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        // Should we cast the result to IEnumerable or change the type of the variable into array of Type? (IEnumerable<TypeInfo> nTypes = ti.DeclaredNestedTypes;)
        IEnumerable<TypeInfo> nTypes = (IEnumerable<TypeInfo>)ti.GetNestedTypes(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        IEnumerable<PropertyInfo> properties = ti.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly); ;
        Type[] genTypeParams = ti.GetGenericArguments(); // or use: IsGenericTypeDefinition ? GetGenericArguments() : Type.EmptyTypes;
        IEnumerable<Type> interfaces = ti.GetInterfaces(); // before: ti.ImplementedInterfaces;
        Type t = ti; // before:  ti.AsType();
        EventInfo ei = ti.GetEvent(name, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        FieldInfo fi = ti.GetField(name, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        MethodInfo mi = ti.GetMethod(name, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        // IEnumerable<MethodInfo> mInfos = ti.GetDeclaredMethods(name); No direct replacement, we might want to add correspodning method to the Type
        // Below type of tInfo changed from TypeInfo to Type 
        Type tInfo = ti.GetNestedType(name, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        PropertyInfo pi = ti.GetProperty(name, BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly);
        if (ti.IsAssignableFrom(anotherInfo)); // no change needed
        Type ati = ti.GetType(); // or just Type ati = ti; in case analyzer update type of ti
    }

    // we might want to add this const to Type and use for the arguments
    public const BindingFlags DeclaredOnlyLookup = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static | BindingFlags.DeclaredOnly;

    private string name = "name";
 }

Category: Maintainability or Design
Severity: Info

cc @terrajobst @carlossanlop

@buyaa-n buyaa-n added area-System.Reflection code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer labels Nov 2, 2021
@ghost
Copy link

ghost commented Nov 2, 2021

Tagging subscribers to this area: @buyaa-n
See info in area-owners.md if you want to be subscribed.

Issue Details

Related to #53217 (comment)
For obsoleting TypeInfo/GetTypeInfo we need an analyzer/code fixer that replaces GetTypeInfo() with calls to GetType(), and occurrences of TypeInfo with Type. A heavy reflection code base that migrated to Windows 8/was written for the Windows 8 era, would have had widely used both. The key would be making it easier for folks to replace them when the type is obsolete.

Category: Reliability
Severity: Info

Author: buyaa-n
Assignees: -
Labels:

area-System.Reflection, code-analyzer, code-fixer

Milestone: -

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Nov 2, 2021
@buyaa-n buyaa-n added api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner labels Nov 2, 2021
@buyaa-n buyaa-n added this to the 7.0.0 milestone Nov 2, 2021
@carlossanlop carlossanlop changed the title Analyzer: Replace occurrences of TypeInfo with Type and TypeInfo.GetTypeInfo() with Type.GetType() Analyzer: Replace occurrences of TypeInfo with Type and GetTypeInfo() with GetType() Nov 23, 2021
@carlossanlop
Copy link
Member

carlossanlop commented Nov 23, 2021

Suggested severity: Suggestion
Suggested category: Design
Suggested milestone: 7.0 (same as issue suggesting obsoleting TypeInfo and GetTypeInfo())

Flag
MemoryStream ms = new MemoryStream();

Type type = ms.GetType();
MemberInfo[] members1 = type.GetMembers();


// Before
TypeInfo typeInfo1 = type.GetTypeInfo();
MemberInfo[] members2 = typeInfo1.GetMembers();


// After
Type typeInfo1 = type;
MemberInfo[] members2 = typeInfo1.GetMembers();

Edit: The main description contains all the cases to address.

@buyaa-n buyaa-n added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-needs-work API needs work before it is approved, it is NOT ready for implementation labels Nov 30, 2021
@buyaa-n buyaa-n modified the milestones: 7.0.0, Future Nov 30, 2021
@bartonjs
Copy link
Member

bartonjs commented Nov 30, 2021

Video

  • Generally looks good as proposed.
  • While a fixer may produce errors in a few edge cases, it seems generally valueable, so we should make one.
  • The analyzer should check the Type/TypeInfo hierarchy to not run when it's not appropriate (e.g. netstandard1.x).
  • It should not flag uses where any TypeInfo-specific members are used.

Category: Maintainability
Severity: Info

@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 30, 2021
@buyaa-n buyaa-n added the help wanted [up-for-grabs] Good issue for external contributors label Nov 30, 2021
@buyaa-n buyaa-n modified the milestones: Future, 7.0.0 Nov 30, 2021
@buyaa-n buyaa-n removed this from the 7.0.0 milestone Jul 6, 2022
@buyaa-n buyaa-n added this to the Future milestone Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Reflection code-analyzer Marks an issue that suggests a Roslyn analyzer code-fixer Marks an issue that suggests a Roslyn code fixer help wanted [up-for-grabs] Good issue for external contributors
Projects
None yet
Development

No branches or pull requests

3 participants