Explanation of RSPEC-1694

[jae...@wayoftheleaf.net]

So,

https://rules.sonarsource.com/csharp/RSPEC-1694

I get the examples shown make total sense for their suggestions, but it's the primary definition that I am curious about. 

The purpose of an abstract class is to provide some heritable behaviors while also defining methods which must be implemented by sub-classes.

Where I agree with the first part, I do not necessarily agree with the second.  The idea of an abstract class is, to use the wording of VB.Net (i know please don't cringe to bad) MustInherit.  That concept to me says that I can have a class that is a "base type" but does not necessarily need to have any "abstract" implementations. 

The RSPEC description is correct that marking the class abstract is absolutely to prevent creating an instance of that exact class type but does not mean that it should be automatically considered a concrete class with a private constructor that not even descendants can access.  This kind of behavior is especially helpful when working with Generics, being able to create some base functionality, but setting up the class so that people aren't creating direct generic references to the type, but creating a declared descendant type.

Example:

    public abstract class MyTypedDataRow<T> : DataRow
        where T : DataTable
    {
        protected MyTypedDataRow(DataRowBuilder rb) : base(rb) { }

        public T OwnerTable
        {
            get { return this.Table as T; }
        }
    }

    public abstract class MyTypedDataTable<R, T> : TypedTableBase<R>
        where R : MyTypedDataRow<T>
        where T : DataTable
    {
       //this has several methods and properties for the class purpose, removed for brevity.
       //as well this class does have abstract methods
    }

I purposefully do not want people doing:

MyTypedDataRow<MyDataTable> row = myDataTable[0];

nor do I want:

public class OrdersTable<MyTypedDataRow<OrdersTable>, OrdersTable> {
}

I specifically want to demand they create classes for both so you get:

public class OrdersTable<OrdersRow, OrdersTable> {
}

public class OrdersRow<OrdersTable> {
}

The problem is RSPEC-1694 does not recognize this configuration, and complains that MyTypedDataRow should be either not abstract or concrete with private constructor.  If I do the former, descendants could be rather liberal with the type usage and not be explicit in their inheritance, which I don't want.  On the other hand, if I do the latter, I can't create descendants of MyTypedDataRow at all.

Is this an oversight of the RSPEC implementation?  I can find no other references on SonarSource to corroborate or cite references for this ruling.

Thanks
Jaeden "Sifo Dyas" al'Raec Ruiner

[valeri....@sonarsource.com]

Hi Jaeden,

Thanks for the feedback. The implementation of the rule does not check the accessibility of the constructors. If there are no concrete methods it suggests converting the class to interface, if there are no abstract methods, it suggests concrete class...

After a little discussion we agreed that the message should suggest protected because otherwise it would be difficult to inherit from this class (possible only with inner classes). I created a ticket, it will be implemented in the next release of SonarC#:

https://github.com/SonarSource/sonar-csharp/issues/1224

Kind regards,

Valeri

[jae...@wayoftheleaf.net]

Cool.

Thanks
J"SD"a'RR