DotNetAnalyzers / ReflectionAnalyzers

Analyzers checking System.Reflection
MIT License
80 stars 8 forks source link

Incorrect 'Use nameof' within nested types #229

Open jnm2 opened 4 years ago

jnm2 commented 4 years ago

This affects REFL016 and the fixes for others like REFL014.

There should be no warning here:

class Base
{
    protected void M() { }
}

class Derived : Base
{
    class Nested
    {
        void M()
        {
            // REFL016 Use nameof. ↓↓↓
            typeof(Base).GetMethod("M");
        }
    }
}

Result:

class Base
{
    protected void M() { }
}

class Derived : Base
{
    class Nested
    {
        void M()
        {
            // CS1540 Cannot access protected member 'Base.M()' via a qualifier of type 'Base';
            // the qualifier must be of type 'Derived.Nested' (or derived from it)
            //                                 ↓
            typeof(Base).GetMethod(nameof(Base.M));
        }
    }
}
jnm2 commented 4 years ago

Here is the fix for REFL014 making the same mistake.

Before:

using System.Reflection;

class Base
{
    protected bool M { get; }
}

class Derived : Base
{
    class Nested
    {
        void M()
        {
            // REFL014 Prefer typeof(Base).GetProperty(nameof(Base.M), BindingFlags.NonPublic |
            // BindingFlags.Instance | BindingFlags.DeclaredOnly).GetMethod.
            //           ↓↓↓↓↓↓↓↓↓
            typeof(Base).GetMethod("get_M", BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly);
        }
    }
}

After:

using System.Reflection;

class Base
{
    protected bool M { get; }
}

class Derived : Base
{
    class Nested
    {
        void M()
        {
            // CS1540 Cannot access protected member 'Base.M' via a qualifier of type 'Base';
            // the qualifier must be of type 'Derived.Nested' (or derived from it)
            //                                   ↓
            typeof(Base).GetProperty(nameof(Base.M), BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.DeclaredOnly).GetMethod;
        }
    }
}
JohanLarsson commented 4 years ago

Hmm, it is somewhat annoying that we already have a check SemanticModel.IsAccessible(context.Node.SpanStart, member.Symbol) that I feel should avoid this.

JohanLarsson commented 4 years ago

The fix in the first post should be typeof(Base).GetMethod(nameof(C.M));

jnm2 commented 3 years ago

I still repro this in 0.1.22-dev, and the code snippet I originally reported repros in 0.1.22-dev too. The commit that closed this issue was included in the 0.1.22 tag, so I think this means that the issue is not fixed in the main branch.