From e3b33db1045b44412c6735aef381d1ad7c723faa Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 14 Feb 2012 20:55:48 +0100 Subject: [PATCH 01/12] fixed the path to pyflakes --- flake8/pep8.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flake8/pep8.py b/flake8/pep8.py index a3e93cd..bdde37b 100644 --- a/flake8/pep8.py +++ b/flake8/pep8.py @@ -93,7 +93,7 @@ for space. """ from flake8 import __version__ as flake8_version -from pyflakes import __version__ as pep8_version +from flake8.pyflakes import __version__ as pep8_version __version__ = '0.6.1' From aa4a855c384c4846db6b176496fca4d385a7622d Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 14 Feb 2012 21:15:54 +0100 Subject: [PATCH 02/12] starting a py2/py3 compatible version --- flake8/mccabe.py | 34 ++++++++++++++++++++-------------- flake8/pyflakes.py | 39 ++++++++++++++++++++++++++------------- 2 files changed, 46 insertions(+), 27 deletions(-) diff --git a/flake8/mccabe.py b/flake8/mccabe.py index b693ca7..ec5118b 100644 --- a/flake8/mccabe.py +++ b/flake8/mccabe.py @@ -3,7 +3,13 @@ http://nedbatchelder.com/blog/200803/python_code_complexity_microtool.html MIT License. """ -import compiler +try: + from compiler.visitor import ASTVisitor as Visitor + from compiler import parse +except ImportError: + from ast import NodeVisitor as Visitor + from ast import parse + import optparse import sys @@ -14,8 +20,8 @@ class PathNode: self.look = look def to_dot(self): - print 'node [shape=%s,label="%s"] %d;' % \ - (self.look, self.name, self.dot_id()) + print('node [shape=%s,label="%s"] %d;' % \ + (self.look, self.name, self.dot_id())) def dot_id(self): return id(self) @@ -36,13 +42,13 @@ class PathGraph: self.nodes.setdefault(n1, []).append(n2) def to_dot(self): - print 'subgraph {' + print('subgraph {') for node in self.nodes: node.to_dot() for node, nexts in self.nodes.items(): for next in nexts: - print '%s -- %s;' % (node.dot_id(), next.dot_id()) - print '}' + print('%s -- %s;' % (node.dot_id(), next.dot_id())) + print('}') def complexity(self): """ Return the McCabe complexity for the graph. @@ -53,13 +59,13 @@ class PathGraph: return num_edges - num_nodes + 2 -class PathGraphingAstVisitor(compiler.visitor.ASTVisitor): +class PathGraphingAstVisitor(Visitor): """ A visitor for a parsed Abstract Syntax Tree which finds executable statements. """ def __init__(self): - compiler.visitor.ASTVisitor.__init__(self) + Visitor.__init__(self) self.classname = "" self.graphs = {} self.reset() @@ -175,9 +181,9 @@ class PathGraphingAstVisitor(compiler.visitor.ASTVisitor): def get_code_complexity(code, min=7, filename='stdin'): complex = [] try: - ast = compiler.parse(code) + ast = parse(code) except AttributeError as e: - print >> sys.stderr, "Unable to parse %s: %s" % (filename, e) + sys.stderr.write("Unable to parse %s: %s\n" % (filename, e)) return 0 visitor = PathGraphingAstVisitor() @@ -215,20 +221,20 @@ def main(argv): options, args = opar.parse_args(argv) text = open(args[0], "rU").read() + '\n\n' - ast = compiler.parse(text) + ast = parse(text) visitor = PathGraphingAstVisitor() visitor.preorder(ast, visitor) if options.dot: - print 'graph {' + print('graph {') for graph in visitor.graphs.values(): if graph.complexity() >= options.min: graph.to_dot() - print '}' + print('}') else: for graph in visitor.graphs.values(): if graph.complexity() >= options.min: - print graph.name, graph.complexity() + print(graph.name, graph.complexity()) if __name__ == '__main__': diff --git a/flake8/pyflakes.py b/flake8/pyflakes.py index 036b25c..00aa8fd 100644 --- a/flake8/pyflakes.py +++ b/flake8/pyflakes.py @@ -2,7 +2,11 @@ # (c) 2005-2010 Divmod, Inc. # See LICENSE file for details -import __builtin__ +try: + import __builtin__ +except ImportError: + import builtins as __builtin__ + import os.path import _ast import sys @@ -251,7 +255,7 @@ class Checker(object): all = [] # Look for imported names that aren't used. - for importation in scope.itervalues(): + for importation in scope.values(): if isinstance(importation, Importation): if not importation.used and importation.name not in all: self.report( @@ -284,7 +288,7 @@ class Checker(object): def handleNode(self, node, parent): node.parent = parent if self.traceTree: - print ' ' * self.nodeDepth + node.__class__.__name__ + print(' ' * self.nodeDepth + node.__class__.__name__) self.nodeDepth += 1 if self.futuresAllowed and not \ (isinstance(node, _ast.ImportFrom) or self.isDocstring(node)): @@ -296,7 +300,7 @@ class Checker(object): finally: self.nodeDepth -= 1 if self.traceTree: - print ' ' * self.nodeDepth + 'end ' + node.__class__.__name__ + print (' ' * self.nodeDepth + 'end ' + node.__class__.__name__) def ignore(self, node): pass @@ -527,10 +531,15 @@ class Checker(object): if isinstance(arg, _ast.Tuple): addArgs(arg.elts) else: - if arg.id in args: + try: + id_ = arg.id + except AttributeError: + id_ = arg.arg + + if id_ in args: self.report(messages.DuplicateArgument, - node.lineno, arg.id) - args.append(arg.id) + node.lineno, id_) + args.append(id_) self.pushFunctionScope() addArgs(node.args.args) @@ -554,7 +563,7 @@ class Checker(object): """ Check to see if any assignments have not been used. """ - for name, binding in self.scope.iteritems(): + for name, binding in self.scope.items(): if (not binding.used and not name in self.scope.globals and isinstance(binding, Assignment)): self.report(messages.UnusedVariable, @@ -629,8 +638,9 @@ def checkPath(filename): @return: the number of warnings printed """ try: - return check(file(filename, 'U').read() + '\n', filename) - except IOError, msg: + return check(open(filename, 'U').read() + '\n', filename) + except IOError: + msg = sys.exc_info()[1] print >> sys.stderr, "%s: %s" % (filename, msg.args[1]) return 1 @@ -652,7 +662,8 @@ def check(codeString, filename): # First, compile into an AST and handle syntax errors. try: tree = compile(codeString, filename, "exec", _ast.PyCF_ONLY_AST) - except SyntaxError, value: + except SyntaxError: + value = sys.exc_info()[1] msg = value.args[0] (lineno, offset, text) = value.lineno, value.offset, value.text @@ -679,13 +690,15 @@ def check(codeString, filename): else: # Okay, it's syntactically valid. Now check it. w = Checker(tree, filename) - w.messages.sort(lambda a, b: cmp(a.lineno, b.lineno)) + sorting = [(msg.lineno, msg) for msg in w.messages] + sorting.sort() + w.messages = [msg for index, msg in sorting] valid_warnings = 0 for warning in w.messages: if skip_warning(warning): continue - print warning + print(warning) valid_warnings += 1 return valid_warnings From 232fc33060b666dd8ee36de6e9475b51a0d4dcaf Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 14 Feb 2012 21:19:03 +0100 Subject: [PATCH 03/12] removing setuptools dep for py3 and install --- setup.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/setup.py b/setup.py index 49d6ef7..c420708 100755 --- a/setup.py +++ b/setup.py @@ -1,7 +1,14 @@ -try: - from setuptools import setup -except ImportError: - from distutils.core import setup # NOQA +import sys + +ispy3 = sys.version_info[0] == 3 + +if ispy3: + from distutils.core import setup +else: + try: + from setuptools import setup + except ImportError: + from distutils.core import setup # NOQA from flake8 import __version__ From 0ac0cec20ffe3c0e479db6e04f7f0dde653996d7 Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 14 Feb 2012 21:27:41 +0100 Subject: [PATCH 04/12] more py3 fixes --- flake8/mccabe.py | 3 ++- flake8/pyflakes.py | 2 +- flake8/run.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/flake8/mccabe.py b/flake8/mccabe.py index ec5118b..cb54cfe 100644 --- a/flake8/mccabe.py +++ b/flake8/mccabe.py @@ -182,7 +182,8 @@ def get_code_complexity(code, min=7, filename='stdin'): complex = [] try: ast = parse(code) - except AttributeError as e: + except AttributeError: + e = sys.exc_info()[1] sys.stderr.write("Unable to parse %s: %s\n" % (filename, e)) return 0 diff --git a/flake8/pyflakes.py b/flake8/pyflakes.py index 00aa8fd..ec1b1d6 100644 --- a/flake8/pyflakes.py +++ b/flake8/pyflakes.py @@ -300,7 +300,7 @@ class Checker(object): finally: self.nodeDepth -= 1 if self.traceTree: - print (' ' * self.nodeDepth + 'end ' + node.__class__.__name__) + print(' ' * self.nodeDepth + 'end ' + node.__class__.__name__) def ignore(self, node): pass diff --git a/flake8/run.py b/flake8/run.py index a24807b..7fd5d48 100644 --- a/flake8/run.py +++ b/flake8/run.py @@ -60,7 +60,7 @@ def main(): def _get_files(repo, **kwargs): seen = set() - for rev in xrange(repo[kwargs['node']], len(repo)): + for rev in range(repo[kwargs['node']], len(repo)): for file_ in repo[rev].files(): file_ = os.path.join(repo.root, file_) if file_ in seen or not os.path.exists(file_): From 3027204914499a65f8010a74426d950d890f35e7 Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 21 Feb 2012 09:47:05 +0100 Subject: [PATCH 05/12] ignore warning on the import - fixes #9 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index c420708..4c036bf 100755 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ if ispy3: from distutils.core import setup else: try: - from setuptools import setup + from setuptools import setup # NOQA except ImportError: from distutils.core import setup # NOQA From 0d0d5d84b5d2953dd0eeb412585599b65e977522 Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 21 Feb 2012 10:36:55 +0100 Subject: [PATCH 06/12] make sure skip_warning works with no files --- flake8/util.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/flake8/util.py b/flake8/util.py index 89640bb..2bcfa66 100644 --- a/flake8/util.py +++ b/flake8/util.py @@ -1,8 +1,11 @@ import re +import os def skip_warning(warning): # XXX quick dirty hack, just need to keep the line in the warning + if not os.path.isfile(warning.filename): + return False line = open(warning.filename).readlines()[warning.lineno - 1] return skip_line(line) From b97f9f542d00cd1e1c40407f897094315ad874e9 Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 21 Feb 2012 10:37:12 +0100 Subject: [PATCH 07/12] io under py3 --- flake8/tests/test_mccabe.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/flake8/tests/test_mccabe.py b/flake8/tests/test_mccabe.py index f1ae82b..5d3f4c1 100644 --- a/flake8/tests/test_mccabe.py +++ b/flake8/tests/test_mccabe.py @@ -1,6 +1,9 @@ import unittest import sys -from StringIO import StringIO +try: + from StringIO import StringIO +except ImportError: + from io import StringIO from flake8.mccabe import get_code_complexity From 7beb6d3845b79201d03928c928b1e8f4b1e34365 Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 21 Feb 2012 10:37:40 +0100 Subject: [PATCH 08/12] make sure skip_warning works with no files --- flake8/run.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/flake8/run.py b/flake8/run.py index 7fd5d48..4a7ddfa 100644 --- a/flake8/run.py +++ b/flake8/run.py @@ -118,6 +118,8 @@ def git_hook(complexity=-1, strict=False): ext = os.path.splitext(filename)[-1] if ext != '.py': continue + if not os.path.exists(filename): + continue warnings += check_file(filename, complexity) if strict: From c3dc6b8c010395312df8c5510ae4907975fec271 Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 21 Feb 2012 10:40:43 +0100 Subject: [PATCH 09/12] more ignores --- .hgignore | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.hgignore b/.hgignore index 6294639..2fd7a5b 100644 --- a/.hgignore +++ b/.hgignore @@ -1,3 +1,7 @@ lib include .*\.pyc$ +dist +bin +flake8.egg-info +man From b1690f818da71d9d532964f5b5ae0f4ef1f817a4 Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 21 Feb 2012 10:40:58 +0100 Subject: [PATCH 10/12] more ignores --- .hgignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.hgignore b/.hgignore index 2fd7a5b..239c799 100644 --- a/.hgignore +++ b/.hgignore @@ -5,3 +5,4 @@ dist bin flake8.egg-info man +\.Python From 1003eeabe881882ee1da163fa53a85a28296329b Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 21 Feb 2012 10:43:16 +0100 Subject: [PATCH 11/12] make sure the name of the exception is added in the scope of the exception - fixes #10 --- flake8/pyflakes.py | 7 +++++-- flake8/tests/test_flakes.py | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 2 deletions(-) create mode 100644 flake8/tests/test_flakes.py diff --git a/flake8/pyflakes.py b/flake8/pyflakes.py index ec1b1d6..c83d210 100644 --- a/flake8/pyflakes.py +++ b/flake8/pyflakes.py @@ -329,7 +329,10 @@ class Checker(object): EQ = NOTEQ = LT = LTE = GT = GTE = IS = ISNOT = IN = NOTIN = ignore # additional node types - COMPREHENSION = EXCEPTHANDLER = KEYWORD = handleChildren + COMPREHENSION = KEYWORD = handleChildren + + def EXCEPTHANDLER(self, node): + self.scope[node.name] = node def addBinding(self, lineno, value, reportRedef=True): '''Called when a binding is altered. @@ -645,7 +648,7 @@ def checkPath(filename): return 1 -def check(codeString, filename): +def check(codeString, filename='(code)'): """ Check the Python source given by C{codeString} for flakes. diff --git a/flake8/tests/test_flakes.py b/flake8/tests/test_flakes.py new file mode 100644 index 0000000..815e61e --- /dev/null +++ b/flake8/tests/test_flakes.py @@ -0,0 +1,17 @@ +from unittest import TestCase +from flake8.pyflakes import check + + +code = """ +try: + pass +except ValueError as err: + print(err) +""" + + +class TestFlake(TestCase): + + def test_exception(self): + warnings = check(code) + self.assertEqual(warnings, 0) From b29f197d69d0719e8bb114862a408d312d47d75e Mon Sep 17 00:00:00 2001 From: Tarek Ziade Date: Tue, 21 Feb 2012 11:31:16 +0100 Subject: [PATCH 12/12] fixed the mccabe complexity for py3 and py2 as well --- flake8/mccabe.py | 73 ++++++++++++++++++++++++++++++------- flake8/tests/test_mccabe.py | 4 +- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/flake8/mccabe.py b/flake8/mccabe.py index cb54cfe..ee89ed0 100644 --- a/flake8/mccabe.py +++ b/flake8/mccabe.py @@ -4,14 +4,49 @@ MIT License. """ try: - from compiler.visitor import ASTVisitor as Visitor from compiler import parse + iter_child_nodes = None except ImportError: - from ast import NodeVisitor as Visitor - from ast import parse + from ast import parse, iter_child_nodes import optparse import sys +from collections import defaultdict + + +class ASTVisitor: + + VERBOSE = 0 + + def __init__(self): + self.node = None + self._cache = {} + + def default(self, node, *args): + if hasattr(node, 'getChildNodes'): + children = node.getChildNodes() + else: + children = iter_child_nodes(node) + + for child in children: + self.dispatch(child, *args) + + def dispatch(self, node, *args): + self.node = node + klass = node.__class__ + meth = self._cache.get(klass) + if meth is None: + className = klass.__name__ + meth = getattr(self.visitor, 'visit' + className, self.default) + self._cache[klass] = meth + + return meth(node, *args) + + def preorder(self, tree, visitor, *args): + """Do preorder walk of tree using visitor""" + self.visitor = visitor + visitor.visit = self.dispatch + self.dispatch(tree, *args) # XXX *args make sense? class PathNode: @@ -30,16 +65,10 @@ class PathNode: class PathGraph: def __init__(self, name): self.name = name - self.nodes = {} - - def add_node(self, n): - assert n - self.nodes.setdefault(n, []) + self.nodes = defaultdict(list) def connect(self, n1, n2): - assert n1 - assert n2 - self.nodes.setdefault(n1, []).append(n2) + self.nodes[n1].append(n2) def to_dot(self): print('subgraph {') @@ -59,13 +88,13 @@ class PathGraph: return num_edges - num_nodes + 2 -class PathGraphingAstVisitor(Visitor): +class PathGraphingAstVisitor(ASTVisitor): """ A visitor for a parsed Abstract Syntax Tree which finds executable statements. """ def __init__(self): - Visitor.__init__(self) + ASTVisitor.__init__(self) self.classname = "" self.graphs = {} self.reset() @@ -100,6 +129,8 @@ class PathGraphingAstVisitor(Visitor): self.graphs["%s%s" % (self.classname, node.name)] = self.graph self.reset() + visitFunctionDef = visitFunction + def visitClass(self, node): old_classname = self.classname self.classname += node.name + "." @@ -110,7 +141,6 @@ class PathGraphingAstVisitor(Visitor): if not self.tail: return pathnode = PathNode(name) - self.graph.add_node(pathnode) self.graph.connect(self.tail, pathnode) self.tail = pathnode return pathnode @@ -177,6 +207,21 @@ class PathGraphingAstVisitor(Visitor): # TODO: visitTryFinally # TODO: visitWith + # XXX todo: determine which ones can add to the complexity + # py2 + # TODO: visitStmt + # TODO: visitAssName + # TODO: visitCallFunc + # TODO: visitConst + + # py3 + # TODO: visitStore + # TODO: visitCall + # TODO: visitLoad + # TODO: visitNum + # TODO: visitarguments + # TODO: visitExpr + def get_code_complexity(code, min=7, filename='stdin'): complex = [] diff --git a/flake8/tests/test_mccabe.py b/flake8/tests/test_mccabe.py index 5d3f4c1..768accd 100644 --- a/flake8/tests/test_mccabe.py +++ b/flake8/tests/test_mccabe.py @@ -36,6 +36,6 @@ class McCabeTest(unittest.TestCase): self.assertEqual(get_code_complexity(_GLOBAL, 1), 2) self.out.seek(0) res = self.out.read().strip().split('\n') - wanted = ["stdin:5:1: 'a' is too complex (3)", - 'stdin:Loop 2 is too complex (1)'] + wanted = ["stdin:5:1: 'a' is too complex (4)", + 'stdin:Loop 2 is too complex (2)'] self.assertEqual(res, wanted)