Saturday, November 28, 2009

Working on F# with NDepend

Following up from the earlier post here, looking more in depth at some of the results out of NDepend for my own little code quality project, and in addition to the results noted there

First out of the gate, for sanity's sake, when working with F#, it would make sense to add !HasAttribute OPTIONAL:System.Runtime.CompilerServices.CompilerGeneratedAttribute onto every rule. That way, you are not left to contemplate how and why the generated CompareTo methods on a simple record type

can have an IL Cyclomatic complexity of 43.

A similar blanket exclusion should be applied to types whose names contain the sigil @ marking them as generated function objects. Unfortunately, compiler inserted write-once local variables do not have any such distinctive sigils by which they can be filtered, just unexciting normal names like str, str2.

It would be nice if the HasAttribute condition also allowed you to filter on properties of the attributes like !HasAttribute OPTIONAL:Microsoft.FSharp.Core.CompilationMappingAttribute WITH (SourceConstructFlags & 31 == 1) OR (SourceConstructFlags & 31 == 2) -- to filter record or sum types, which are rich in generated structure, where appropriate. For example, though the sum type

may be implemented into the CLR by making Either<'a, 'b> an abstract type with concrete inner types Either<'a, 'b>+Left and Either<'a, 'b>+Right, the headline Either<'a, 'b> type is not an abstract base class in the usual sense, that user written code will be expected to extend the type.

Similarly, while it is arguably an oversight of the F# code generation that none of the inner types within a sum type are marked as sealed -- there is no good case for extending them -- it would also be nice to be able to exclude all (implicitly, generated) types that derived from (or are an inner type of) a sum or record type from analysis; as well as all the content of types attributed with [CompilationMapping(SourceConstructFlags.NonpublicRepresentation | ...)].

Allowing names of the form |ActivePattern|_| is a trivial extension of the "names should be upper case" rule.

There is what looks like a bug in NDepend's analysis as it manages to make all the types I have derived from Microsoft.FxCop.Sdk.BaseIntrospectionRule pass the filter

DepthOfInheritance == 1 // Must derive directly from System.Object

The containing rule for that test Classes that are candidate to be turned into Structures could probably also benefit from having AND !IsStatic added, so as to exempt functional modules.

No comments :