Some FxCop rules for VB

We’ve got a rather large VB code base at FinRad, at least some of which we would like to migrate to C# in the nearish future. Because of this, we have created several custom rules that are intended to detect issues in VB code that would cause problems when migrating the code to C#. However, while attempting to assign categories for those custom rules, we quickly realized that each of them had merits besides portability of the code base, and that we wanted our assemblies to abide by these rules regardless of whether or not they might eventually be ported to C#.

I hadn’t been planning on writing a post about these rules for some time to come, but then I saw the announcement last week that Microsoft is using VB to build the Silverlight javascript compiler and VBx, which seems to have boosted the topic’s prority on my internal TODO list enough that I can’t quite seem to ignore it…

Rule #1: Interfaces should not contain nested types (Design)

By definition, interfaces should not contain any implementation, and a nested type would certainly qualify as implementation. Unfortunately, the VB compiler generates no warnings or errors when any type whatsoever is nested within an interface. For example, the following interface will compile just fine (and FxCop won’t flag it as an “empty” interface either):

Public Interface ISomething

Enum Animal
End Enum

Class SomeClass
Public Sub DoSomething()
End Sub
End Class

End Interface

To make matters worse, the VB compiler will also generate a type within an interface under certain conditions. Consider, for example, the following interface:

Public Interface ISomethingElse

Event SomethingDone(ByVal sender As Object, ByVal e As EventArgs)

End Interface

When an event is declared using the above syntax, the VB compiler will generate a delegate for the event signature within the same parent type. The resulting compiled assembly actually ends up containing the following interface definition, which includes a nested type:

Public Interface ISomethingElse

Delegate Sub SomethingDoneEventHandler(ByVal sender As Object, ByVal e As EventArgs)

Event SomethingDone As SomethingDoneEventHandler

End Interface

Rule #2: Properties should not have arguments (Design)

In order to be easily invokable from C#, a property should never have more than one argument, and only indexers (i.e.: properties named “Item” in VB) should have even one argument. If either of these conventions are ignored, C# code is going to have to use syntax like get_SomeProperty(a, b, c) to use your properties, which is considerably less readable than some of the alternatives.  Do you really want to be forcing your API’s callers to be writing ugly code?

Rule #3: Member names should not match type names (Naming)

C# will generate compiler error CS0542 if you try to create a member with the same name as its parent type, but VB won’t complain about this at all. The use of a member with the same name as the type to signify a constructor in C# presumably explains this difference in compiler behaviour. However, there’s an API useability problem here as well since this significance of a member (usually a property) with the same name as its parent class is rarely immediately obvious. For example, imagine a class named “Label” with a property named “Label”. What exactly does “Label.Label” represent? In such a case, either the class or the member probably ought to be renamed to convey more information about its role.

Rule #4: Review call scope forcing (Usage)

VB allows for forcing the scope of invocation of a virtual member to the current class via the MyClass keyword. Both VB and C# allow for forcing the scope of invocation of a virtual member to the base class (via MyBase and base, respectively). Since call scope forcing isn’t really a VB-specific capability, this isn’t really a VB-specific rule. However, eliminating MyClass use was what provoked its development at FinRad, so I’m including it here anyway.

In case you’re wondering why call scope forcing should be reviewed, consider why a member would be declared as virtual in the first place: it’s meant to be overrideable. If a subclass has overridden a virtual method, why should a base class be deliberately avoiding invoking that override? Obviously, there are cases where this behaviour is desirable, which is why this is a “review” rule rather than a “do not” rule, but there are also plenty of cases where scope forcing is likely to be used inappropriately, particularly when developers aren’t aware of the consequences.

If you want to implement this rule, there are a few things to watch out for:

  • Within the implementation of an overriding, overloading, or hiding member, it is quite common to invoke the base class implementation explicitly. In order to avoid undesirable noise in the rule analysis results, these cases should not be flagged as problems by the rule.

  • Because of VB interface member implementation renaming, the name of an overload will not necessarily be the same as the name of the member it is overloading.

  • The C# compiler will force a non-virtual call when invoking a non-virtual base class member even if your code uses this rather than base to scope the call. The VB compiler will retain the virtual call.

  • FxCop 1.35 and Visual Studio 2005 Static Analysis both have problems detecting whether a property is overriding and/or hiding, although the exact behaviour differs between the two engines, as well as between .NET 1.1. and 2.0 assemblies for FxCop 1.35. For more details on this issue (which appears to have been resolved in Orcas), see this bug report.

Leave a Reply

Your email address will not be published. Required fields are marked *