Ticket #1555: check-miscaptures-v2.darcs.patch

1 patch for repository http://tahoe-lafs.org/source/tahoe/trunk:

Fri Oct  7 08:41:21 BST 2011  david-sarah@jacaranda.org
  * Add misc/coding_tools/check-miscaptures.py to detect incorrect captures of variables declared in a for loop, and a 'make check-miscaptures' Makefile target to run it. (It is also run by 'make code-checks'.) This is a rewritten version that reports much fewer false positives, by determining captured variables more accurately. fixes #1555

New patches:

[Add misc/coding_tools/check-miscaptures.py to detect incorrect captures of variables declared in a for loop, and a 'make check-miscaptures' Makefile target to run it. (It is also run by 'make code-checks'.) This is a rewritten version that reports much fewer false positives, by determining captured variables more accurately. fixes #1555
david-sarah@jacaranda.org**20111007074121
 Ignore-this: 51318e9678d132c374ea557ab955e79e
] {
hunk ./Makefile 124
        false
 endif
 
-code-checks: build version-and-path check-interfaces -find-trailing-spaces -check-umids pyflakes
+code-checks: build version-and-path check-interfaces check-miscaptures -find-trailing-spaces -check-umids pyflakes
 
 version-and-path:
        $(TAHOE) --version-and-path
hunk ./Makefile 133
        $(TAHOE) @misc/coding_tools/check-interfaces.py 2>&1 |tee violations.txt
        @echo
 
+check-miscaptures:
+       $(PYTHON) misc/coding_tools/check-miscaptures.py $(SOURCES) 2>&1 |tee miscaptures.txt
+       @echo
+
 pyflakes:
        $(PYTHON) -OOu `which pyflakes` $(SOURCES) |sort |uniq
        @echo
addfile ./misc/coding_tools/check-miscaptures.py
hunk ./misc/coding_tools/check-miscaptures.py 1
+#! /usr/bin/python
+
+import os, sys, compiler, traceback
+from compiler.ast import Node, For, AssName, Name, Lambda, Function
+
+
+def check_source(source):
+    return check_thing(compiler.parse, source)
+
+def check_file(path):
+    return check_thing(compiler.parseFile, path)
+
+def check_thing(parser, thing):
+    try:
+        ast = parser(thing)
+    except SyntaxError, e:
+        return [e]
+    else:
+        results = []
+        check_ast(ast, results)
+        return results
+
+def check_ast(ast, results):
+    """Check a node outside a 'for' loop."""
+    if isinstance(ast, For):
+        check_for(ast, results)
+    else:
+        for child in ast.getChildNodes():
+            if isinstance(ast, Node):
+                check_ast(child, results)
+
+def check_for(ast, results):
+    """Check a particular outer 'for' loop."""
+
+    declared = {}  # maps name to lineno of declaration
+    nested = set()
+    collect_declared_and_nested(ast, declared, nested)
+
+    # For each nested function...
+    for funcnode in nested:
+        # Check for captured variables in this function.
+        captured = set()
+        collect_captured(funcnode, declared, captured)
+        for name in captured:
+            # We want to report the outermost capturing function
+            # (since that is where the workaround will need to be
+            # added), and the variable declaration. Just one report
+            # per capturing function per variable will do.
+            results.append(make_result(funcnode, name, declared[name]))
+
+        # Check each node in the function body in case it
+        # contains another 'for' loop.
+        childnodes = funcnode.getChildNodes()[len(funcnode.defaults):]
+        for child in childnodes:
+            check_ast(funcnode, results)
+
+def collect_declared_and_nested(ast, declared, nested):
+    """
+    Collect the names declared in this 'for' loop, not including
+    names declared in nested functions. Also collect the nodes of
+    functions that are nested one level deep.
+    """
+    if isinstance(ast, AssName):
+        declared[ast.name] = ast.lineno
+    else:
+        childnodes = ast.getChildNodes()
+        if isinstance(ast, (Lambda, Function)):
+            nested.add(ast)
+
+            # The default argument expressions are "outside" the
+            # function, even though they are children of the
+            # Lambda or Function node.
+            childnodes = childnodes[:len(ast.defaults)]
+
+        for child in childnodes:
+            if isinstance(ast, Node):
+                collect_declared_and_nested(child, declared, nested)
+
+def collect_captured(ast, declared, captured):
+    """Collect any captured variables that are also in declared."""
+    if isinstance(ast, Name):
+        if ast.name in declared:
+            captured.add(ast.name)
+    else:
+        childnodes = ast.getChildNodes()
+
+        if isinstance(ast, (Lambda, Function)):
+            # Formal parameters of the function are excluded from
+            # captures we care about in subnodes of the function body.
+            declared = declared.copy()
+            for argname in ast.argnames:
+                if argname in declared:
+                    del declared[argname]
+
+            for child in childnodes[len(ast.defaults):]:
+                collect_captured(child, declared, captured)
+
+            # The default argument expressions are "outside" the
+            # function, even though they are children of the
+            # Lambda or Function node.
+            childnodes = childnodes[:len(ast.defaults)]
+
+        for child in childnodes:
+            if isinstance(ast, Node):
+                collect_captured(child, declared, captured)
+
+
+def make_result(funcnode, var_name, var_lineno):
+    if hasattr(funcnode, 'name'):
+        func_name = 'function %r' % (funcnode.name,)
+    else:
+        func_name = '<lambda>'
+    return (funcnode.lineno, func_name, var_name, var_lineno)
+
+def report(out, path, results):
+    for r in results:
+        if isinstance(r, SyntaxError):
+            print >>out, path + (" NOT ANALYSED due to syntax error: %s" % r)
+        else:
+            print >>out, path + (":%r %s captures %r declared at line %d" % r)
+
+def check(sources, out):
+    class Counts:
+        n = 0
+        processed_files = 0
+        suspect_files = 0
+    counts = Counts()
+
+    def _process(path):
+        results = check_file(path)
+        report(out, path, results)
+        counts.n += len(results)
+        counts.processed_files += 1
+        if len(results) > 0:
+            counts.suspect_files += 1
+
+    for source in sources:
+        print >>out, "Checking %s..." % (source,)
+        if os.path.isfile(source):
+            _process(source)
+        else:
+            for (dirpath, dirnames, filenames) in os.walk(source):
+                for fn in filenames:
+                    (basename, ext) = os.path.splitext(fn)
+                    if ext == '.py':
+                        _process(os.path.join(dirpath, fn))
+
+    print >>out, ("%d suspiciously captured variables in %d out of %d files"
+                  % (counts.n, counts.suspect_files, counts.processed_files))
+    return counts.n
+
+
+sources = ['src']
+if len(sys.argv) > 1:
+    sources = sys.argv[1:]
+if check(sources, sys.stderr) > 0:
+    sys.exit(1)
+
+
+# TODO: self-tests
}

Context:

[no_network.py: Clean up whitespace around code changed by previous patch.
david-sarah@jacaranda.org**20111004010407
 Ignore-this: 647ec8a9346dca1a41212ab250619b72
] 
[no_network.py: Fix potential bugs in some tests due to capture of slots in for loops.
david-sarah@jacaranda.org**20111004010231
 Ignore-this: 9c496877613a3befd54979e5de6e63d2
] 
[docs: fix the rst formatting of COPYING.TGPPL.rst
zooko@zooko.com**20111003043333
 Ignore-this: c5fbc83f4a3db81a0c95b27053c463c5
 Now it renders correctly both on trac and with rst2html --verbose from docutils v0.8.1.
] 
[MDMF: remove extension fields from caps, tolerate arbitrary ones. Fixes #1526
Brian Warner <warner@lothar.com>**20111001233553
 Ignore-this: 335e1690aef1146a2c0b8d8c18c1cb21
 
 The filecaps used to be produced with hints for 'k' and segsize, but they
 weren't actually used, and doing so had the potential to limit how we change
 those filecaps in the future. Also the parsing code had some problems dealing
 with other numbers of extensions. Removing the existing fields and making the
 parser tolerate (and ignore) extra ones makes MDMF more future-proof.
] 
[test/test_runner.py: BinTahoe.test_path has rare nondeterministic failures; this patch probably fixes a problem where the actual cause of failure is masked by a string conversion error.
david-sarah@jacaranda.org**20110927225336
 Ignore-this: 6f1ad68004194cc9cea55ace3745e4af
] 
[docs/configuration.rst: add section about the types of node, and clarify when setting web.port enables web-API service. fixes #1444
zooko@zooko.com**20110926203801
 Ignore-this: ab94d470c68e720101a7ff3c207a719e
] 
[TAG allmydata-tahoe-1.9.0a2
warner@lothar.com**20110925234811
 Ignore-this: e9649c58f9c9017a7d55008938dba64f
] 
Patch bundle hash:
98a38d22095e3489ff7773a45cc873e21893eeb5