yellow team mailing list archive

[Nicola Larosa <nicola.larosa@xxxxxxxxxxxxx>]

Nicola Larosa has proposed merging lp:~teknico/juju-gui/extract-doc-stats into lp:juju-gui.

Requested reviews:
  Juju GUI Hackers (juju-gui)
Related bugs:
  Bug #1082073 in juju-gui: "Compute documentation statistics"
  https://bugs.launchpad.net/juju-gui/+bug/1082073

For more details, see:
https://code.launchpad.net/~teknico/juju-gui/extract-doc-stats/+merge/135958

Compute documentation statistics.

Count the total number of YUIDoc documentation lines by
enhancing the output of the "bin/lint-yuidoc" command.

https://codereview.appspot.com/6845085/

-- 
https://code.launchpad.net/~teknico/juju-gui/extract-doc-stats/+merge/135958
Your team Juju GUI Hackers is requested to review the proposed merge of lp:~teknico/juju-gui/extract-doc-stats into lp:juju-gui.

=== modified file 'app/views/charm-panel.js'
--- app/views/charm-panel.js	2012-11-19 16:31:18 +0000
+++ app/views/charm-panel.js	2012-11-23 18:04:32 +0000
@@ -379,7 +379,8 @@
           if (query.requires || query.provides) {
             store.find(
                 query,
-                { /**
+                {
+                  /**
                    * If the charm we searched for is still the same as the
                    * view's charm, ask renderRelatedCharms to render the
                    * results.  If they differ, discard the results, because they

=== modified file 'bin/lint-yuidoc'
--- bin/lint-yuidoc	2012-11-02 08:58:16 +0000
+++ bin/lint-yuidoc	2012-11-23 18:04:32 +0000
@@ -1,10 +1,21 @@
 #!/usr/bin/env python
 
+"""
+Check inline documentation of Javascript functions.
+
+Parse the Javascript codebase, noting which functions are and which are not
+documented. Also, compute the total number of documentation lines.
+
+The file named "undocumented" lists the currently undocumented functions.
+Once a function is documented, it should be manually removed from that file;
+this script will complain if it is not.
+"""
+
 import os
-import os.path
 import re
 import sys
 
+
 # We don't want to build or integrate with a full JS parser; fortunately we
 # comply with our coding style well enough that we can pick out the non-trivial
 # function definitions because they are on a line by themselves and are either
@@ -14,6 +25,9 @@
 
 
 def find_functions(source):
+    """
+    Iterable returning (line_number, line) tuples for each function in source.
+    """
     for line_number, line in enumerate(source):
         match = function_regex.match(line)
         if match:
@@ -22,95 +36,119 @@
             yield name, line_number
 
 
-def find_docs(function_name, line_number, boundry, source):
-    # Walk backwards from the function declaration to find some yuidoc.  If we
-    # hit the previous function declaration ("boundry") before finding one,
-    # then there is none.
+def find_docs(line_number, boundary, source):
+    """
+    In source, search for the documentation fragment starting from line_number
+    and going backwards until boundary. Return its number of lines if found,
+    otherwise zero.
+    """
     in_comment = False
-    for current_line_number in range(line_number-1, boundry, -1):
-        source_line = source[current_line_number]
-        if source_line.strip().startswith('*/'):
+    for current_line_number in range(line_number-1, boundary, -1):
+        source_line = source[current_line_number].strip()
+        if source_line.endswith('*/'):
+            ending_line_number = current_line_number
             in_comment = True
-        elif source_line.strip().startswith('/*'):
+        if source_line.startswith('/**'):
+            # We found a documentation block, return its number of lines.
+            return ending_line_number - current_line_number + 1
+        elif source_line.startswith('/*'):
+            # False alarm, keep looking.
             in_comment = False
         # If we enter or exit a block while scanning backwards without finding
         # documentation, then there is none to be found.
         if not in_comment and ('{' in source_line or '}' in source_line):
-            return False
-        # If we find a documentation block, tell the caller that we did.
-        if '/**' in source[current_line_number]:
-            return True
-    return False
+            return 0
+    return 0
 
 
 def check_file(file_name, undocumented):
+    """
+    Check the documentation included in file_name, contrasting it with the
+    undocumented list. Return all functions names found, the ones without
+    documentation, the ones with one but listed as without, and the overall
+    number of documentation lines.
+    """
     # Every function.
-    functions = []
-    # Every function that has (unexpectedly) missing documentation.
-    missing_documentation = []
+    file_functions = set()
+    # Every function that has unexpectedly missing documentation.
+    missing_doc_functions = set()
     # Every function that has documentation but wasn't expected to.
-    falsely_undocumented = []
+    falsely_undoc_functions = set()
+    # Accumulate the total number of doc lines.
+    file_num_doc_lines = 0
     with open(file_name) as f:
         source = f.readlines()
-        boundry = 0
+        boundary = 0
         for function_name, line_number in find_functions(source):
-            functions.append((file_name, function_name))
-            found_docs = find_docs(function_name, line_number, boundry, source)
-            boundry = line_number
+            file_functions.add((file_name, function_name))
+            num_doc_lines = find_docs(line_number, boundary, source)
+            boundary = line_number
 
             # Report our findings.
             code_location = '%s:%d "%s"' % (
                 file_name, line_number+1, function_name)
-            is_undocumented = (file_name, function_name) in undocumented
+            in_undocumented = (file_name, function_name) in undocumented
             # If we found documentation for the function...
-            if found_docs:
+            if num_doc_lines:
+                file_num_doc_lines += num_doc_lines
                 # If it is listed as an undocumented function...
-                if is_undocumented:
+                if in_undocumented:
                     #...report the incongruence.
-                    falsely_undocumented.append(code_location)
+                    falsely_undoc_functions.add(code_location)
             # Otherwise if we found an undocumented function that is not
-            # supposed to be undocumented, report it.
-            elif not found_docs and not is_undocumented:
-                missing_documentation.append(code_location)
+            # listed as undocumented, report it.
+            elif not num_doc_lines and not in_undocumented:
+                missing_doc_functions.add(code_location)
 
-    return functions, missing_documentation, falsely_undocumented
+    return (
+        file_functions, missing_doc_functions, falsely_undoc_functions,
+        file_num_doc_lines)
 
 
 def main():
+    """
+    Traverse the "app" subtree excluding all "assets" directories and
+    parse all Javascript file not named "template.js". Report the total
+    number of documentation lines, the total number of undocumented functions,
+    and any discrepancies between the current status and the undocumented list.
+    """
     # Did we find any lint?
     found_errors = False
     # All of the functions found.
-    all_functions = []
+    all_functions = set()
+    total_num_doc_lines = 0
+    with open('undocumented') as f:
+        undocumented = [tuple(line.split()) for line in f.readlines()]
     for root, dirs, files in os.walk('app'):
         # Ignore any asset directories.
         try:
             dirs.remove('assets')
-        except:
+        except ValueError:
             pass
         # Ignore the template.js file.
         files = [f for f in files if f.endswith('.js') and f != 'templates.js']
-
-        with open('undocumented') as f:
-            undocumented = [tuple(line.split()) for line in f.readlines()]
         for file_name in [os.path.join(root, name) for name in files]:
-            functions, missing_documentation, falsely_undocumented = (
-                check_file(file_name, undocumented))
-            all_functions.extend(functions)
-            for code_location in missing_documentation:
-                print code_location, 'missing yuidoc'
+            (file_functions, missing_doc_functions, falsely_undoc_functions,
+                file_num_doc_lines) = check_file(file_name, undocumented)
+            all_functions.update(file_functions)
+            for code_location in missing_doc_functions:
+                print code_location, 'undocumented but not listed as such'
                 found_errors = True
-            for code_location in falsely_undocumented:
+            for code_location in falsely_undoc_functions:
                 print code_location, 'erroneously listed as undocumented'
                 found_errors = True
+            total_num_doc_lines += file_num_doc_lines
+
+    print
+    print 'Total documentation lines: %d' % total_num_doc_lines
 
     # Find functions that are listed as undocumented but don't actually exist.
-    missing = set(undocumented) - set(all_functions)
+    missing = set(undocumented) - all_functions
     for code_location in ['%s "%s"' % x for x in missing]:
         print code_location, 'listed as undocumented but does not exist'
         found_errors = True
 
     # Urge the user to drive down the number of known undocumented functions.
-    print
     print 'Backlog of undocumented functions:', len(undocumented)
     print 'Please do your part to drive the above to zero by documenting '
     print 'functions listed in the "undocumented" file and removing their '