Inner Member Shadows Outer

ID

csharp.inner_member_shadows_outer

Severity

high

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Code Smell

Language

CSharp

Tags

code_smell, naming, nested-type, shadowing

Description

Reports a member of a nested type — field, constant, property, method or event — whose name is also the name of a static member of an enclosing type. The whole enclosing chain is checked, so a type nested two levels deep is compared against both of its enclosing types.

Only static members of the enclosing types are compared, because instance members are not in scope unqualified inside a nested type and so cannot be shadowed. Constants count as static.

A static method of a generic enclosing type (Outer<T>) is not collected, unlike its fields, constants, properties and events. A generic outer type is routinely used as a namespace for a nested, per-T-specialised dispatch helper that declares its own method of the same name, always called through the qualified form Outer<T>.Nested<U>.Method(…​) — the CA1000-constrained "static generic facade delegates to a non-generic instance implementation" idiom. There the nested method is not shadowing anything: it is the specialised implementation the generic facade’s own static method forwards to.

Rationale

The static members of an enclosing type are visible without qualification throughout the body of a nested type — that visibility is much of the reason to nest a type in the first place. Declaring a nested member with one of those names takes the name over for the nested body:

public class Configuration
{
    public static string DefaultPath = "/etc";
    public const int MaxSize = 1024;
    public string InstanceName = "cfg";

    public class Section
    {
        public string DefaultPath;                 // FLAW - hides Configuration.DefaultPath here
        public int MaxSize { get; set; }           // FLAW - hides the enclosing constant
        public string InstanceName;                // OK - the enclosing member is not static
        public string Label;                       // OK - no enclosing member of that name

        public string Describe() => DefaultPath;   // which DefaultPath?
    }
}

The last line is the cost: nothing in it says which member is read, and the answer depends on a declaration that may be a hundred lines away. The failure mode is also asymmetric in time — code in the nested type written before the nested member existed used to read the enclosing value and now reads the new one, with no diff on those lines. Reaching the shadowed member again requires writing the enclosing type’s name, which is the very thing the nesting was meant to avoid.

Remediation

Rename one of the two members so each name means one thing. The nested one is usually the better candidate: it is newer and has fewer readers, and a name qualified by its role in the nested type often reads better anyway. If the two really hold the same value, drop the nested member and read the enclosing static one directly.

References