No Decreased Inherited Visibility

ID

csharp.no_decreased_inherited_visibility

Severity

high

Remediation Complexity

medium

Remediation Risk

low

Remediation Effort

medium

Resource

Code Smell

Language

CSharp

Tags

code_smell, inheritance, shadowing, visibility

Description

Reports a private method of a derived class whose name and parameter count match a public, protected or protected internal method of its base class that cannot be overridden — one that is neither virtual nor abstract nor itself an override.

Not reported: a declaration carrying the new modifier, which states the hiding intent explicitly and is covered by csharp.do_not_hide_base_class_methods; an explicitly implemented interface member, whose name is qualified and shadows nothing; and the case where the base method is overridable, which is a dispatch relationship rather than shadowing. Base classes from other files are not compared.

Rationale

Reducing the visibility of an inherited member is not something C# lets you do. The base method is not virtual, so there is nothing to dispatch and nothing is replaced — the private declaration only shadows the name inside the derived class body. Every other reader of the object keeps calling the base implementation:

public class Repository
{
    public void Save(string key) { }
    public virtual void Refresh() { }
}

public class CachedRepository : Repository
{
    private void Save(string key) { }     // FLAW - external callers still reach Repository.Save
    private void Refresh() { }            // OK - the base method is virtual
    private void Evict(string key) { }    // OK - no inherited method of that name
}

// new CachedRepository().Save("k");                  // Repository.Save runs
// ((Repository)new CachedRepository()).Save("k");    // Repository.Save runs

The declaration reads as if it locked the operation down or replaced it, and it did neither. If the intent was to restrict access, the type hierarchy is the wrong tool; if it was to change behaviour, the base member has to be made overridable first. Either way the code as written leaves two methods of the same name with different bodies, and which one runs depends on where the call is written.

Remediation

If the derived logic is meant to replace the inherited one, make the base method virtual and declare the derived one as an override with the same access level. If it is a separate internal helper, rename it so it no longer collides with the inherited name. If the goal was to keep the inherited operation out of the derived type’s contract, prefer composition — hold the base type in a field and expose only the members that belong to the new abstraction.

References