Utility or not of cobra compiler generating code metrics.

[hopscc]

I'm wondering what folks opinions are of the utility or not of augmenting cobra (compiler) to also generate some code metrics info
either as warnings when a particular metric value is exceeded or as a mode/compile switch that generates tables for each method (class, ...) showing the value of a metric.

A while ago I put some temp code in the compiler to generate a warning if a methods LOC exceeded 100 lines - this picked up a number of excessively large methods in the compiler that were corrected.
The code for this remains in the compiler but not enabled with a note to eventually turn it into a proper option.

I was recently reminded of the McCabe Cyclomatic complexity calculation and followed that to a utility that generates an mcc for python code.
If you are not familiar with Cyclomatic complexity metric see

• wikipedia
• Debunking Cyclomatic complecity
• measuring cyclomatic complexity of python code
• pymetrics
• Continuous Integration with BuildBot

I hacked a quick and dirty pseudo calc into the compiler, compiled it and ran the result on the compiler code

Warning Results with cutoff set to > 25

Code: Select all

580 xx:...cobra/wkspace/Source> mkcobcsh
Compiler.cobra(900): warning: Method Compiler.__loadReference numLines=53 complexity=28  McCabe Cyclomatic Complexity > 25.
CobraParser.cobra(1475): warning: Method CobraParser.declareMethod numLines=128 complexity=34  McCabe Cyclomatic Complexity > 25.
CobraParser.cobra(2932): warning: Method CobraParser.expression2 numLines=5 complexity=46  McCabe Cyclomatic Complexity > 25.
TypeProxies.cobra(287): warning: Method ClrTypeProxy._realTypeWithoutCache numLines=119 complexity=31  McCabe Cyclomatic Complexity > 25.
Compilation succeeded - 4 warnings
...


Results with Table cutoff level >10
First number is lines of code in method - its incorrect for methods with deeply indented trailing statements (like .expression2)
Second is complexity calc ( the exact values may differ some cos I was also fiddling with the calculation accumulation)

Code: Select all

586 xx:...cobra/wkspace/Source> mkcobcsh > mccabe
587 xx:...cobra/wkspace/Source> sort -k 3 mccabe
                                  DictLit._bindImp              16      11
                                IndexExpr._bindImp              21      11
                              CompareExpr._bindImp              38      11
                              Initializer._bindImp              28      11
                            CobraParser.branchStmt              48      11
                          CobraType.isAssignableTo              7       11
                         AssignExpr._writeSharpDef              46      11
                         RequirePart.writeSharpDef              61      11
                        PostCallExpr.writeSharpDef              1       11
                      AbstractMethod.writeSharpImp              2       11
                      Compiler.isCompilationNeeded              15      11
                      Extension.extensionMemberFor              30      11
                      StringTokenDef._matchBetween              14      11
                     Compiler.printConsoleMessages              13      11
                     NameSpace._extensionMemberFor              14      11
                   ChainedCompareExpr._willCompare              35      11
                 Box.canHaveDetailedStackTrace.get              13      11
             AbstractTypeIdentifier._symbolForName              25      11
             (unnamed extension 63591).isEnumerable              10      11
                                  CobraParser.stmt              114     12
                                RaiseStmt._bindImp              9       12
                               CobraParser.tryStmt              43      12
                               MemberExpr._bindImp              51      12
                             InheritsExpr._bindImp              42      12
                            Compiler._loadAssembly              30      12
                            DotExpr._writeSharpDef              1       12
                            IntType.isAssignableTo              12      12
                           CobraCore._findCobraExe              1       12
                          TypeExpr._requiresTypeOf              2       12
                         CobraParser.statementsFor              4       12
                         TestifyRunner._testifyRun              33      12
                        CompareExpr._writeSharpDef              52      12
                      CobraParser.declareMethodSig              53      12
                      Container<of>.isAssignableTo              13      12
                      MemberExpr._bindImpBoxMember              25      12
                    CobraParser.nonqualifiedTypeId              66      12
                 Utils.cobraNameForSharpMemberName              1       12
           PrimitiveType._installNativeMethodsFrom              7       12
                                  Box._scanMethods              1       13
                                  DotExpr._bindImp              12      13
                              ArgParser._parseArgs              66      13
                             CommandLine.doCompile              64      13
                           Class._makeInitializers              5       13
                           IdentifierExpr._bindImp              23      13
              TestifyRunner._testifyInlineMessages              56      13
       Container<of>.greatestCommonDenominatorWith              12      13
                                 CobraParser.block              39      14
                        TestifyRunner._testifyFile              67      14
                   NumericTypeInfo.promoteNumerics              32      14
        AbstractMethod._computeMatchingBaseMembers              6       14
                                 Box.symbolForName              32      15
                             Compiler.compileSharp              102     15
                             PostCallExpr._bindImp              87      15
                            CobraParser.expression              71      15
                          ArrayType.isAssignableTo              11      15
                          Tokenizer._nextToken.get              2       15
           CobraType.greatestCommonDenominatorWith              14      15
                                CommandLine.doHelp              29      16
                               AssignExpr._bindImp              56      16
                     CobraParser._nameSpaceAddDecl              46      16
                            CobraCore.findCobraExe              41      17
                         NumericPromoExpr._bindImp              72      17
                     CobraParser.compilerDirective              10      17
       (unnamed extension 63591).isDictionaryLike              21      17
                                    Set<of>.equals              1       18
                        IdentifyMainPhase.innerRun              28      18
                      CobraParser.indexOrSliceExpr              52      18
                                 BinaryOpExpr.make              1       19
                             UseDirective._bindUse              58      19
                MemberOverload.computeBestOverload              90      19
          CobraParser._declareGetOrSetOnlyProperty              68      19
                                 Box.innerType.get              46      20
                       GenerateHtmlDocVisitor.rank              11      20
                                 CallExpr._bindImp              64      21
         HtmlExceptionReportWriter.willDumpHtmlFor              13      21
          (unnamed extension 63591).isSequenceLike              27      21
         TestifyRunner._processFirstlineDirectives              86      22
                       CobraParser.declareProperty              82      23
                           StringMaker._makeString              37      24
                         CobraParser.typeSpecDecls              78      24
                                   IfStmt._bindImp              84      25
                          Compiler.__loadReference              53      28
                ClrTypeProxy._realTypeWithoutCache              119     31
                         CobraParser.declareMethod              128     34
                           CobraParser.expression2              5       46
... std compiler status elided...


I looked at the methods with the largest complexity count and as well as being (excessively) long (LOC) they are also uninhibitedly complex.
(I'll provide corrections to those anyway eventually)

but What are folks thoughts as to the usefulness of this and
if useful how should it be presented
- warnings with a fixed (wired cutoff)
- warnings with an optionally set cutoff
- table mode ( cobra -metric:ccyclomatic *.cobra)
- table mode and cutoff (args)
- other ??

At the moment I'm only considering LOC/method and complexity/method but it could be engineered so as to provide settings, accumulation and presentation scaffolding that made it easier
to add additional code metrics calculations in the future.

[Charles]

Just quickly, whatever we do, it could be its own phase that visits the AST nodes rather than putting it "here and there" in the existing classes. See the -doc implementation for an example.

Also, for a long time now, I have wanted to not just measure the complexity of members, but to also score the quality controls (types, unit tests, contracts, asserts) and then net the two numbers, sort by the net score and output that list. A long method with no unit tests or contracts would be at the top of the list. As you broke it up and added quality controls it would drop down accordingly.

I'll look at your message in more detail later this week. Time for sleep.

[hopscc]

Yes, If done and applicable it would be done by visiting AST nodes.
Currently its just exploring the concept which is why I called it a 'quick and dirty' hack.

[nevdelap]

That sounds really, really cool.

[torial]

Awesome, love cyclomatic complexity metric!

[hopscc]

Any ideas about presentation, display, interface ?

Theres a bit of an outstanding query about how to allow args specific to commandline options which I suppose feeds into this.
(so far we've punted and made two options one for the option enablement and one for its args ( cf tesitfyirunner multithreading )

[Charles]

I like HTML because then you can do formatting, color, links, etc. On Mac at least, I can even arrange a link to bring up my editor on the local file.

Also, a parseable format would be nice for display in an IDE. But that's not a high priority as I don't think it would get picked up any time soon.

Yes we still have the issue of command specific arguments. I can't say that I have an answer at this point... suggestions?

[hopscc]

OTTOMH
We have -option and -option:value

how about augmenting to allow something like
-majorOption:minorOptName[=value][,minorOptName[=value]...]

i.e Possibly multiple sub opts within a major option, comma separated name=value pairs as a single string, optional value specified
The value can default to an internal value (if not specified) or just be used as a bool (on/off) - like verbosity/verbose

e.g

Code: Select all

cobra -metrics:loc=50,mcc=15,noTests  -files:filesList.txt
# enable metrics,
#   calc/report only linesOfCode/method with threshold at 50 lines,
#                           Mccabe Complexity calc with threshold at 15 and
#                           enable reporting of methods with noTests (say)  ( all,any others off)
cobra -metrics -files:filesList.txt
# enable metrics, all calculations, default values/thresholds

cobra -testifyControl:resultsFile=testResults,numthreads=8 -testify *.cobra
# replacing -testify-results and -testifyThreads
# major option testfyControl, subOptions(overriding internal defaults) resultsFile (name) and numThreads to use 


Any thoughts/prefs re display as a warning/threshold vs a Table ??

A simple Text table form ; padded text fields with Tab separators is human readable and readily parseable by pretty much anything
The down side of HTML ( complex .*ML)is that you need a reader/browser to clear the obscuring formatting for human readability..

[hopscc]

I've implemented a new OptionSpecs type 'multival' that allows you to spec an option that takes sub options.
The subOptions are described in a new clause 'subOption' and are themselves a List of OptionSpecs so any of the existing arg types (int, bool, set,...)
and capabilities (defaults) can be specified (rather than just type string (or bool).

modified the three testify-* options to use a new multival option 'testifyArgs and tested, seems to work Ok.
e.g

Code: Select all

cobra -testifyArgs:resultsFile=testResults,numthreads=8 -testify *.cobra
# replacing -testify-results and -testifyThreads and testify-runner
# major option testfyArgs, subOptions(overriding internal defaults) resultsFile (name) and numThreads and runner to use.

cobra -testifyArgs:numthreads=2 -testify *.cobra   # use rest of defaults as is


The OptionsSpec clause for testifyArgs looks like this

Code: Select all

      {
         'name': 'testifyArgs',
         'type': 'multival',
         'description': 'Set names and values for controlling testify as a list of comma separated name=value pairs.',
         'choices': ['resultsFile', 'runner', 'numThreads'],  #allowable subOption keys
         'example': ['numThreads=4,resultsFile=testResult', 'runner=Core.myTestRunner', 'numThreads=2'],
         'developer-only': true,
         'default': 'resultsFile=r-testify,runner=Cobra.Lang.CobraCore.runAllTests,numThreads=1',
         'subOptions' : [
            {
               'name': 'resultsFile',
               'type': 'string',
               'description': 'Filename to write the testify results to. Progress is still written to console.',
               'default': 'r-testify',          # value to default to if not specify name at all
               #'defaultIfKeyOnly': 'r-testify', # this value if only specify subOption name on commandline
            },   
            {
               'name': 'runner',
               'type': 'string',
               'description': 'method to invoke to run the unit tests. The method must be "shared". Typically the method will make use of classes in Cobra.Lang.Test to setup and initiate the test run.',
               'default': 'Cobra.Lang.CobraCore.runAllTests',
            },
            {
               'name': 'numThreads',
               'type': 'int',
               'description': 'Number of threads to run the testify testsuite in.',
               'default': '1',
            },
         ],
      },


See ticket:225

[Charles]

Sorry I haven't had a chance to keep up with this thread. I'll try to take a look later in the week.