diff --git a/docs/intro.rst b/docs/intro.rst index 12f67d7e..4d086923 100644 --- a/docs/intro.rst +++ b/docs/intro.rst @@ -1,4 +1,4 @@ -.. currentmodule:: pycodestyle +].. currentmodule:: pycodestyle Introduction ============ @@ -400,6 +400,12 @@ This is the current list of error and warning codes: +------------+----------------------------------------------------------------------+ | W604 | backticks are deprecated, use 'repr()' | +------------+----------------------------------------------------------------------+ +| **W7** | *Statement warning* | ++------------+----------------------------------------------------------------------+ +| W740 (#) | inconsistent use of return (explicit) | ++------------+----------------------------------------------------------------------+ +| W741 (#) | inconsistent use of return (implicit on reachable end of function) | ++------------+----------------------------------------------------------------------+ **(*)** In the default configuration, the checks **E121**, **E123**, **E126**, @@ -411,6 +417,11 @@ closing`` to report **E133** instead of **E123**. **(^)** These checks can be disabled at the line level using the ``# noqa`` special comment. This possibility should be reserved for special cases. +**(#)** In the default configuration, the checks **W740** and **W741** are +ignored for performance reasons. Indeed, they rely on an AST tree to be +built which is a a slower operation. + + *Special cases aren't special enough to break the rules.* diff --git a/pycodestyle.py b/pycodestyle.py index 70263968..ccc8856f 100755 --- a/pycodestyle.py +++ b/pycodestyle.py @@ -55,7 +55,7 @@ import inspect import keyword import tokenize -import warnings +import ast from optparse import OptionParser from fnmatch import fnmatch try: @@ -67,7 +67,7 @@ __version__ = '2.0.0' DEFAULT_EXCLUDE = '.svn,CVS,.bzr,.hg,.git,__pycache__,.tox' -DEFAULT_IGNORE = 'E121,E123,E126,E226,E24,E704,W503' +DEFAULT_IGNORE = 'E121,E123,E126,E226,E24,E704,W503,W740,W741' try: if sys.platform == 'win32': USER_CONFIG = os.path.expanduser(r'~\.pep8') @@ -147,6 +147,7 @@ def tabs_or_spaces(physical_line, indent_char): for offset, char in enumerate(indent): if char != indent_char: return offset, "E101 indentation contains mixed spaces and tabs" + return None def tabs_obsolete(physical_line): @@ -158,6 +159,7 @@ def tabs_obsolete(physical_line): indent = INDENT_REGEX.match(physical_line).group(1) if '\t' in indent: return indent.index('\t'), "W191 indentation contains tabs" + return None def trailing_whitespace(physical_line): @@ -179,6 +181,7 @@ def trailing_whitespace(physical_line): return len(stripped), "W291 trailing whitespace" else: return 0, "W293 blank line contains whitespace" + return None def trailing_blank_lines(physical_line, lines, line_number, total_lines): @@ -195,6 +198,7 @@ def trailing_blank_lines(physical_line, lines, line_number, total_lines): return 0, "W391 blank line at end of file" if stripped_last_line == physical_line: return len(physical_line), "W292 no newline at end of file" + return None def maximum_line_length(physical_line, max_line_length, multiline): @@ -218,7 +222,7 @@ def maximum_line_length(physical_line, max_line_length, multiline): if ((len(chunks) == 1 and multiline) or (len(chunks) == 2 and chunks[0] == '#')) and \ len(line) - len(chunks[-1]) < max_line_length - 7: - return + return None if hasattr(line, 'decode'): # Python 2 # The line could contain multi-byte characters try: @@ -228,6 +232,7 @@ def maximum_line_length(physical_line, max_line_length, multiline): if length > max_line_length: return (max_line_length, "E501 line too long " "(%d > %d characters)" % (length, max_line_length)) + return None ############################################################################## @@ -771,17 +776,14 @@ def whitespace_around_named_parameter_equals(logical_line, tokens): Don't use spaces around the '=' sign when used to indicate a keyword argument or a default parameter value. - Okay: def complex(real, imag=0.0): - Okay: return magic(r=real, i=imag) + Okay: def complex(real, imag=0.0):\n return magic(r=real, i=imag) Okay: boolean(a == b) Okay: boolean(a != b) Okay: boolean(a <= b) Okay: boolean(a >= b) - Okay: def foo(arg: int = 42): - Okay: async def foo(arg: int = 42): - E251: def complex(real, imag = 0.0): - E251: return magic(r = real, i = imag) + Okay:python3 E901:python2: def foo(arg: int = 42):\n pass + E251: def complex(real, imag = 0.0):\n return magic(r = real, i = imag) """ parens = 0 no_space = False @@ -801,9 +803,9 @@ def whitespace_around_named_parameter_equals(logical_line, tokens): parens += 1 elif text in ')]': parens -= 1 - elif in_def and text == ':' and parens == 1: + elif parens == 1 and in_def and text == ':': annotated_func_arg = True - elif parens and text == ',' and parens == 1: + elif parens == 1 and text == ',': annotated_func_arg = False elif parens and text == '=' and not annotated_func_arg: no_space = True @@ -1035,7 +1037,7 @@ def break_around_binary_operator(logical_line, tokens): Okay: (width == 0 +\n height == 0) Okay: foo(\n -x) - Okay: foo(x\n []) + Okay: foo(x,\n []) Okay: x = '''\n''' + '' Okay: foo(x,\n -y) Okay: foo(x, # comment\n -y) @@ -1078,11 +1080,11 @@ def comparison_to_singleton(logical_line, noqa): Comparisons to singletons like None should always be done with "is" or "is not", never the equality operators. - Okay: if arg is not None: - E711: if arg != None: - E711: if None == arg: - E712: if arg == True: - E712: if False == arg: + Okay: arg is not None + E711: arg != None + E711: None == arg + E712: arg == True + E712: False == arg Also, beware of writing if x when you really mean if x is not None -- e.g. when testing whether a variable or argument that defaults to None was @@ -1132,15 +1134,15 @@ def comparison_type(logical_line, noqa): Do not compare types directly. - Okay: if isinstance(obj, int): - E721: if type(obj) is type(1): + Okay: isinstance(obj, int) + E721: type(obj) is type(1) When checking if an object is a string, keep in mind that it might be a unicode string too! In Python 2.3, str and unicode have a common base class, basestring, so you can do: - Okay: if isinstance(obj, basestring): - Okay: if type(a1) is type(b1): + Okay: isinstance(obj, basestring) + Okay: type(a1) is type(b1) """ match = COMPARE_TYPE_REGEX.search(logical_line) if match and not noqa: @@ -1153,7 +1155,7 @@ def comparison_type(logical_line, noqa): def python_3000_has_key(logical_line, noqa): r"""The {}.has_key() method is removed in Python 3: use the 'in' operator. - Okay: if "alph" in d:\n print d["alph"] + Okay: "alph" in d W601: assert d.has_key('alph') """ pos = logical_line.find('.has_key(') @@ -1179,8 +1181,8 @@ def python_3000_not_equal(logical_line): The older syntax is removed in Python 3. - Okay: if a != 'no': - W603: if a <> 'no': + Okay: a != 'no' + W603: a <> 'no' """ pos = logical_line.find('<>') if pos > -1: @@ -1198,6 +1200,158 @@ def python_3000_backticks(logical_line): yield pos, "W604 backticks are deprecated, use 'repr()'" +class UnconsistentReturns(): + r"""Check that return statement are consistent. + + Functions should either return an explicit value in all return + statements (including the final value-less implicit return if + reachable) or in none of them. + If a return statement returns an explicit value in a function : + * return statements with no explicit values lead to W740. + * end of function (if reachable) leads to W741. + """ + + def __init__(self, tree, filename): + r"""Init.""" + self.tree = tree + self.filename = filename + + def run(self): + r"""Run the check.""" + return UnconsistentReturns.check_in_tree(self.tree) + + @staticmethod + def check_in_tree(tree): + r"""Check for inconsistent returns in tree.""" + assert isinstance(tree, ast.AST) + for node in ast.walk(tree): + if isinstance(node, ast.FunctionDef): + for err in UnconsistentReturns.check_in_func(node): + yield err + + @staticmethod + def check_in_func(func_node): + r"""Check for inconsistent returns (with or without values) in function. + """ + assert isinstance(func_node, ast.FunctionDef) + returns = list(FlowAnalysis.collect_return_nodes(func_node)) + returns_value = [ret for ret in returns if ret.value is not None] + if returns_value: + func_name = func_node.name + for r in returns: + if r.value is None: + yield (r.lineno, + r.col_offset, + "W740 unconsistent return values in %s" % func_name, + "toto") + if FlowAnalysis.end_of_block_is_reachable(func_node.body): + yield (func_node.lineno, + func_node.col_offset, + "W741 unconsistent return values in %s" % func_name, + "toto") + + +class FlowAnalysis(): + r"""Class of utility methods to perform flow analysis. + + Class container for various static methods. This class probably + shouldn't be a class but a module but it seems like being a single + file is one of the pep8 features.""" + + @staticmethod + def collect_return_nodes(func_node): + r"""Collect return nodes from the node describing a function. + + The tricky part is not to get the nodes corresponding to return + statements in nested function definitions. + Heavily based on the ast.walk() function. + """ + from collections import deque + assert isinstance(func_node, ast.FunctionDef) + todo = deque(ast.iter_child_nodes(func_node)) + while todo: + node = todo.popleft() + if not isinstance(node, ast.FunctionDef): + todo.extend(ast.iter_child_nodes(node)) + if isinstance(node, ast.Return): + yield node + + @staticmethod + def end_of_block_is_reachable(tree): + r"""Return true if the end of a block is reachable. + + A block can be a single ast.stmt or a list of them. + Detecting whether the end of some piece of code is reachable or + not corresponds to solving the halting problem which is known to be + impossible. However, we could solve a relaxed version of this : + indeed, we may assume that: + - all code is reachable except for obvious cases. + - only a few kind of statements may break the reachable property: + * return statements + * raise statements + * assert with "obviously"-False values. + We'll consider the end of block to be reachable if nothing breaks + the reachability property. + """ + this_func = FlowAnalysis.end_of_block_is_reachable # shorter name + if isinstance(tree, list): + return all(this_func(stmt) for stmt in tree) + assert isinstance(tree, ast.stmt) + # These stop reachability + if isinstance(tree, (ast.Return, ast.Raise)): + return False + elif isinstance(tree, ast.Assert): + return not FlowAnalysis.expression_must_be_false(tree.test) + # These propagage reachability + elif isinstance(tree, ast.If): + branches = [] + if not FlowAnalysis.expression_must_be_true(tree.test): + branches.append(tree.orelse) + if not FlowAnalysis.expression_must_be_false(tree.test): + branches.append(tree.body) + return any(this_func(brch) for brch in branches) + elif isinstance(tree, getattr(ast, 'TryFinally', ())): + return this_func(tree.finalbody) and this_func(tree.body) + elif isinstance(tree, getattr(ast, 'TryExcept', ())): + # TODO: orelse ignored at the moment + return this_func(tree.body) or \ + any(this_func(handler.body) for handler in tree.handlers) + elif isinstance(tree, getattr(ast, 'Try', ())): + if not this_func(tree.finalbody): + return False + # TODO: orelse ignored at the moment + return this_func(tree.body) or \ + any(this_func(handler.body) for handler in tree.handlers) + elif isinstance(tree, ast.With): + return this_func(tree.body) + # Otherwise, assume reachability hasn't been broken + return True + + @staticmethod + def expression_must_be_true(tree): + assert isinstance(tree, ast.expr) + if isinstance(tree, getattr(ast, 'NameConstant', ())): + return tree.value + elif isinstance(tree, ast.Name): + return tree.id == "True" + elif isinstance(tree, ast.UnaryOp): + if isinstance(tree.op, ast.Not): + return FlowAnalysis.expression_must_be_false(tree.operand) + return False + + @staticmethod + def expression_must_be_false(tree): + assert isinstance(tree, ast.expr) + if isinstance(tree, getattr(ast, 'NameConstant', ())): + return not tree.value + elif isinstance(tree, ast.Name): + return tree.id == "False" + elif isinstance(tree, ast.UnaryOp): + if isinstance(tree.op, ast.Not): + return FlowAnalysis.expression_must_be_true(tree.operand) + return False + + ############################################################################## # Helper functions ############################################################################## @@ -1355,12 +1509,15 @@ def _get_parameters(function): in inspect.signature(function).parameters.values() if parameter.kind == parameter.POSITIONAL_OR_KEYWORD] else: - return inspect.getargspec(function)[0] + getarg = getattr(inspect, 'getfullargspec', inspect.getargspec) + return getarg(function).args def register_check(check, codes=None): """Register a new check object.""" def _add_check(check, kind, codes, args): + if codes is None: + codes = ERRORCODE_REGEX.findall(check.__doc__ or '') if check in _checks[kind]: _checks[kind][check][0].extend(codes or []) else: @@ -1368,12 +1525,15 @@ def _add_check(check, kind, codes, args): if inspect.isfunction(check): args = _get_parameters(check) if args and args[0] in ('physical_line', 'logical_line'): - if codes is None: - codes = ERRORCODE_REGEX.findall(check.__doc__ or '') _add_check(check, args[0], codes, args) elif inspect.isclass(check): - if _get_parameters(check.__init__)[:2] == ['self', 'tree']: - _add_check(check, 'tree', codes, None) + init = getattr(check, '__init__', None) + # Exclude slot wrappers. + # Python 3 uses functions, Python 2 unbound methods. + if inspect.isfunction(init) or inspect.ismethod(init): + args = _get_parameters(init) + if args and args[0] == 'self' and args[1] == 'tree': + _add_check(check, args[1], codes, args) def init_checks_registry(): @@ -1384,6 +1544,8 @@ def init_checks_registry(): mod = inspect.getmodule(register_check) for (name, function) in inspect.getmembers(mod, inspect.isfunction): register_check(function) + for (name, klass) in inspect.getmembers(mod, inspect.isclass): + register_check(klass) init_checks_registry() @@ -1457,21 +1619,16 @@ def readline(self): def run_check(self, check, argument_names): """Run a check plugin.""" - arguments = [] - for name in argument_names: - arguments.append(getattr(self, name)) - return check(*arguments) - - def init_checker_state(self, name, argument_names): - """Prepare custom state for the specific checker plugin.""" if 'checker_state' in argument_names: - self.checker_state = self._checker_states.setdefault(name, {}) + self.checker_state = self._checker_states.setdefault( + check.__name__, {}) + arguments = [getattr(self, name) for name in argument_names] + return check(*arguments) def check_physical(self, line): """Run all physical checks on a raw input line.""" self.physical_line = line - for name, check, argument_names in self._physical_checks: - self.init_checker_state(name, argument_names) + for check, argument_names in self._physical_checks: result = self.run_check(check, argument_names) if result is not None: (offset, text) = result @@ -1527,10 +1684,7 @@ def check_logical(self): self.blank_before = self.blank_lines if self.verbose >= 2: print(self.logical_line[:80].rstrip()) - for name, check, argument_names in self._logical_checks: - if self.verbose >= 4: - print(' ' + name) - self.init_checker_state(name, argument_names) + for check, argument_names in self._logical_checks: for offset, text in self.run_check(check, argument_names) or (): if not isinstance(offset, tuple): for token_offset, pos in mapping: @@ -1549,8 +1703,9 @@ def check_ast(self): try: tree = compile(''.join(self.lines), '', 'exec', PyCF_ONLY_AST) except (ValueError, SyntaxError, TypeError): - return self.report_invalid_syntax() - for name, cls, __ in self._ast_checks: + self.report_invalid_syntax() + return + for cls, __ in self._ast_checks: checker = cls(tree, self.filename) for lineno, offset, text, check in checker.run(): if not self.lines or not noqa(self.lines[lineno - 1]): @@ -1693,7 +1848,7 @@ def error(self, line_number, offset, text, check): """Report an error, according to options.""" code = text[:4] if self._ignore_code(code): - return + return None if code in self.counters: self.counters[code] += 1 else: @@ -1701,7 +1856,7 @@ def error(self, line_number, offset, text, check): self.messages[code] = text[5:] # Don't care about expected errors or warnings if code in self.expected: - return + return None if self.print_filename and not self.file_errors: print(self.filename) self.file_errors += 1 @@ -1813,7 +1968,7 @@ def __init__(self, options): def error(self, line_number, offset, text, check): if line_number not in self._selected[self.filename]: - return + return None return super(DiffReport, self).error(line_number, offset, text, check) @@ -1892,7 +2047,7 @@ def input_dir(self, dirname): """Check all files in this directory and all subdirectories.""" dirname = dirname.rstrip('/') if self.excluded(dirname): - return 0 + return counters = self.options.report.counters verbose = self.options.verbose filepatterns = self.options.filename @@ -1932,11 +2087,11 @@ def ignore_code(self, code): return False. Else, if 'options.ignore' contains a prefix of the error code, return True. """ - if len(code) < 4 and any(s.startswith(code) - for s in self.options.select): + options = self.options + if len(code) < 4 and any(s.startswith(code) for s in options.select): return False - return (code.startswith(self.options.ignore) and - not code.startswith(self.options.select)) + return (code.startswith(options.ignore) and + not code.startswith(options.select)) def get_checks(self, argument_name): """Get all the checks for this category. @@ -1944,12 +2099,10 @@ def get_checks(self, argument_name): Find all globally visible functions where the first argument name starts with argument_name and which contain selected tests. """ - checks = [] - for check, attrs in _checks[argument_name].items(): - (codes, args) = attrs - if any(not (code and self.ignore_code(code)) for code in codes): - checks.append((check.__name__, check, args)) - return sorted(checks) + checks = [(check, args) + for check, (codes, args) in _checks[argument_name].items() + if any(not self.ignore_code(code) for code in codes)] + return sorted(checks, key=lambda arg: arg[0].__name__) def get_parser(prog='pep8', version=__version__): diff --git a/setup.py b/setup.py index b77770e5..d1bb2bfc 100644 --- a/setup.py +++ b/setup.py @@ -8,6 +8,7 @@ def get_version(): for line in f: if line.startswith('__version__'): return eval(line.split('=')[-1]) + return None def get_long_description(): diff --git a/testsuite/E10.py b/testsuite/E10.py index cd142e39..c2c476c0 100644 --- a/testsuite/E10.py +++ b/testsuite/E10.py @@ -1,8 +1,8 @@ -#: E101 W191 +#: E101 W191 E901:python3 for a in 'abc': for b in 'xyz': - print a # indented with 8 spaces - print b # indented with 1 tab + print(a) # indented with 8 spaces + print(b) # indented with 1 tab #: E101 E122 W191 W191 if True: pass @@ -35,7 +35,7 @@ def tearDown(self): def test_keys(self): """areas.json - All regions are accounted for.""" expected = set([ - u'Norrbotten', - u'V\xe4sterbotten', + 'Norrbotten', + 'V\xe4sterbotten', ]) #: diff --git a/testsuite/E11.py b/testsuite/E11.py index 4ed10963..c08bceb5 100644 --- a/testsuite/E11.py +++ b/testsuite/E11.py @@ -1,13 +1,13 @@ #: E111 if x > 2: - print x + print(x) #: E111 if True: print -#: E112 +#: E112 E901 if False: print -#: E113 +#: E113 E901 print print #: E114 E116 diff --git a/testsuite/E12.py b/testsuite/E12.py index a995c955..8b4e2569 100644 --- a/testsuite/E12.py +++ b/testsuite/E12.py @@ -1,22 +1,22 @@ #: E121 -print "E121", ( - "dent") +print("E121", ( + "dent")) #: E122 -print "E122", ( -"dent") +print("E122", ( +"dent")) #: E123 my_list = [ 1, 2, 3, 4, 5, 6, ] #: E124 -print "E124", ("visual", +print("E124", ("visual", "indent_two" - ) + )) #: E124 -print "E124", ("visual", +print("E124", ("visual", "indent_five" -) +)) #: E124 a = (123, ) @@ -25,20 +25,20 @@ col < 0 or self.moduleCount <= col): raise Exception("%s,%s - %s" % (row, col, self.moduleCount)) #: E126 -print "E126", ( - "dent") +print("E126", ( + "dent")) #: E126 -print "E126", ( - "dent") +print("E126", ( + "dent")) #: E127 -print "E127", ("over-", - "over-indent") +print("E127", ("over-", + "over-indent")) #: E128 -print "E128", ("visual", - "hanging") +print("E128", ("visual", + "hanging")) #: E128 -print "E128", ("under-", - "under-indent") +print("E128", ("under-", + "under-indent")) #: @@ -63,11 +63,11 @@ 4 + \ 5 + 6 #: E131 -print "hello", ( +print("hello", ( "there", # "john", - "dude") + "dude")) #: E126 part = set_mimetype(( a.get('mime_type', 'text')), @@ -100,11 +100,11 @@ or another_very_long_variable_name: raise Exception() #: E122 -dictionary = [ +dictionary = { "is": { "nested": yes(), }, -] +} #: E122 setup('', scripts=[''], @@ -117,9 +117,9 @@ #: E123 W291 -print "E123", ( +print("E123", ( "bad", "hanging", "close" - ) + )) # #: E123 E123 E123 result = { @@ -358,7 +358,7 @@ def example_issue254(): """.strip().split(): print(foo) #: E122:6:5 E122:7:5 E122:8:1 -print dedent( +print(dedent( ''' mkdir -p ./{build}/ mv ./build/ ./{build}/%(revision)s/ @@ -366,8 +366,8 @@ def example_issue254(): build='build', # more stuff ) -) -#: E701:1:8 E122:2:1 E203:4:8 E128:5:1 +)) +#: E701:1:8 E122:2:1 E203:4:8 E128:5:1 E901:python2 if True:\ print(True) diff --git a/testsuite/E12not.py b/testsuite/E12not.py index e76ef134..67836292 100644 --- a/testsuite/E12not.py +++ b/testsuite/E12not.py @@ -46,34 +46,34 @@ "indented for visual indent") -print "OK", ("visual", - "indent") +print("OK", ("visual", + "indent")) -print "Okay", ("visual", +print("Okay", ("visual", "indent_three" - ) + )) -print "a-ok", ( +print("a-ok", ( "there", "dude", -) +)) -print "hello", ( +print("hello", ( "there", - "dude") + "dude")) -print "hello", ( +print("hello", ( "there", # "john", - "dude") + "dude")) -print "hello", ( - "there", "dude") +print("hello", ( + "there", "dude")) -print "hello", ( +print("hello", ( "there", "dude", -) +)) # Aligned with opening delimiter foo = long_function_name(var_one, var_two, @@ -188,12 +188,12 @@ def long_function_name( if ((foo.bar("baz") and foo.bar("frop") )): - print "yes" + print("yes") # also ok, but starting to look like LISP if ((foo.bar("baz") and foo.bar("frop"))): - print "yes" + print("yes") if (a == 2 or b == "abc def ghi" @@ -213,12 +213,12 @@ def long_function_name( # -print 'l.{line}\t{pos}\t{name}\t{text}'.format( +print('l.{line}\t{pos}\t{name}\t{text}'.format( line=token[2][0], pos=pos, name=tokenize.tok_name[token[0]], text=repr(token[1]), -) +)) print('%-7d %s per second (%d total)' % ( options.counters[key] / elapsed, key, @@ -429,7 +429,7 @@ def unicode2html(s): help = "print total number of errors " \ "to standard error" - +#: E901:python3 help = u"print total number of errors " \ u"to standard error" @@ -521,8 +521,7 @@ def unicode2html(s): 'reasonComment_de', 'reasonComment_it'), '?'), "foo", context={'alpha': 4, 'beta': 53242234, 'gamma': 17}) - - +#: E901 def f(): try: if not Debug: diff --git a/testsuite/E20.py b/testsuite/E20.py index 2f1ecc28..85d98ca6 100644 --- a/testsuite/E20.py +++ b/testsuite/E20.py @@ -37,18 +37,18 @@ #: E203:1:10 if x == 4 : - print x, y + print(x, y) x, y = y, x -#: E203:2:15 E702:2:16 +#: E203:2:16 E702:2:17 if x == 4: - print x, y ; x, y = y, x + print(x, y) ; x, y = y, x #: E203:3:13 if x == 4: - print x, y + print(x, y) x, y = y , x #: Okay if x == 4: - print x, y + print(x, y) x, y = y, x a[b1, :] == a[b1, ...] b = a[:, b1] diff --git a/testsuite/E22.py b/testsuite/E22.py index 9d8efda5..5a80e2fd 100644 --- a/testsuite/E22.py +++ b/testsuite/E22.py @@ -142,7 +142,7 @@ def halves(n): print >>sys.stderr, "x is out of range." print >> sys.stdout, "x is an integer." x = x / 2 - 1 - +#: E901:python2 if alpha[:-i]: *a, b = (1, 2, 3) diff --git a/testsuite/E25.py b/testsuite/E25.py index 7a536b57..dddf2078 100644 --- a/testsuite/E25.py +++ b/testsuite/E25.py @@ -31,10 +31,6 @@ def foo(bar = False): d[type(None)] = _deepcopy_atomic # Annotated Function Definitions -#: Okay -def munge(input: AnyStr, sep: AnyStr = None, limit=1000, - extra: Union[str, dict] = None) -> AnyStr: +#: E901:python2 +def munge(input: AnyStr, sep: AnyStr = None, limit=1000) -> AnyStr: pass -#: Okay -async def add(a: int = 0, b: int = 0) -> int: - return a + b diff --git a/testsuite/E27.py b/testsuite/E27.py index 888b3a80..32077b1c 100644 --- a/testsuite/E27.py +++ b/testsuite/E27.py @@ -6,6 +6,7 @@ True and False #: E271 if 1: + pass #: E273 True and False #: E273 E274 diff --git a/testsuite/E90.py b/testsuite/E90.py index 1db3d0e6..22511056 100644 --- a/testsuite/E90.py +++ b/testsuite/E90.py @@ -1,14 +1,14 @@ -#: E901 +#: E901 E901 } -#: E901 +#: E901 E901 = [x -#: E901 E101 W191 +#: E901 E901 E101 W191 while True: try: pass except: print 'Whoops' -#: E122 E225 E251 E251 E701 +#: E122 E225 E251 E251 E701 E901 # Do not crash if code is invalid if msg: diff --git a/testsuite/W19.py b/testsuite/W19.py index afdfb767..c952e3e4 100644 --- a/testsuite/W19.py +++ b/testsuite/W19.py @@ -70,12 +70,12 @@ def long_function_name( if ((foo.bar("baz") and foo.bar("frop") )): - print "yes" + print("yes") #: E101 W191 # also ok, but starting to look like LISP if ((foo.bar("baz") and foo.bar("frop"))): - print "yes" + print("yes") #: E101 W191 if (a == 2 or b == "abc def ghi" @@ -135,8 +135,8 @@ def long_function_name( def test_keys(self): """areas.json - All regions are accounted for.""" expected = set([ - u'Norrbotten', - u'V\xe4sterbotten', + 'Norrbotten', + 'V\xe4sterbotten', ]) #: W191 x = [ diff --git a/testsuite/W39.py b/testsuite/W39.py index 68f886be..2b7d9749 100644 --- a/testsuite/W39.py +++ b/testsuite/W39.py @@ -5,7 +5,7 @@ # Two additional empty lines -#: W391:4:1 W293:3:1 W293:4:1 noeol +#: W391:4:1 W293:3:1 W293:4:1 E901:python2.6 noeol # The last lines contain space diff --git a/testsuite/W60.py b/testsuite/W60.py index 973d22ff..2050e761 100644 --- a/testsuite/W60.py +++ b/testsuite/W60.py @@ -1,15 +1,15 @@ -#: W601 +#: W601 E901:python3 if a.has_key("b"): print a -#: W602 +#: W602 E901:python3 raise DummyError, "Message" -#: W602 +#: W602 E901:python3 raise ValueError, "hello %s %s" % (1, 2) -#: Okay +#: E901:python3 raise type_, val, tb raise Exception, Exception("f"), t -#: W603 +#: W603 E901:python3 if x <> 0: x = 0 -#: W604 +#: W604 E901:python3 val = `1 + 2` diff --git a/testsuite/W74.py b/testsuite/W74.py new file mode 100644 index 00000000..9efdc6e3 --- /dev/null +++ b/testsuite/W74.py @@ -0,0 +1,100 @@ +# Test suite to work on issue https://github.com/PyCQA/pep8/issues/399 +import math +import itertools + +# Code examples from GvR +# https://mail.python.org/pipermail/python-dev/2015-April/139054.html + +# Correct ### + + +def foo_ok(x): + if x >= 0: + return math.sqrt(x) + else: + return None + + +def bar_ok(x): + if x < 0: + return None + return math.sqrt(x) + + +def foobar_ok(x): + if True: + return None + else: + pass + + +# Not correct ### +#: W741:1:1 +def foo_ko(x): + if x >= 0: + return math.sqrt(x) +#: W740:3:9 +def bar_ko(x): + if x < 0: + return + return math.sqrt(x) + +# More examples for the sake of testings + +# Correct ### + + +def foo_ok(x): + if x >= 0: + return math.sqrt(x) + elif x == 0: + return 0 + else: + return None + + +def goldbach_conjecture_ok(): + for i in itertools.count(2): + if not can_be_expressed_as_prime_sum(i): + return i + assert not True + + +def outer_function(): + + def nested_function(): + return 6 * 9 + + print(42 == nested_function()) + return +# Not correct ### +#: W741:1:1 +def foo_ko(x): + if x >= 0: + return math.sqrt(x) + elif x == 0: + return 0 +#: W741:1:1 +def goldbach_conjecture_ko(): + for i in itertools.count(2): + if not can_be_expressed_as_prime_sum(i): + return i + + +def return_finally1(): # return 1 + try: + return 1 + finally: + pass +#: W740:5:9 +def return_finally2(): # return None + try: + return 2 + finally: + return +#: W740:3:9 +def return_finally3(): # return 4 + try: + return + finally: + return 4 diff --git a/testsuite/python3.py b/testsuite/python3.py index 88818800..efa4898a 100644 --- a/testsuite/python3.py +++ b/testsuite/python3.py @@ -2,5 +2,6 @@ # Annotated function (Issue #29) +#: E901:python2 def foo(x: int) -> int: return x + 1 diff --git a/testsuite/support.py b/testsuite/support.py index 003f181b..0c3eafb2 100644 --- a/testsuite/support.py +++ b/testsuite/support.py @@ -38,7 +38,7 @@ def error(self, line_number, offset, text, check): detailed_code = '%s:%s:%s' % (code, line_number, offset + 1) # Don't care about expected errors or warnings if code in self.expected or detailed_code in self.expected: - return + return None self._deferred_print.append( (line_number, offset, detailed_code, text[5:], check.__doc__)) self.file_errors += 1 @@ -91,7 +91,7 @@ def selftest(options): report = BaseReport(options) counters = report.counters checks = options.physical_checks + options.logical_checks - for name, check, argument_names in checks: + for check, argument_names in checks: for line in check.__doc__.splitlines(): line = line.lstrip() match = SELFTEST_REGEX.match(line) @@ -162,10 +162,16 @@ def run_tests(filename): if codes and index: if 'noeol' in codes: testcase[-1] = testcase[-1].rstrip('\n') - codes = [c for c in codes - if c not in ('Okay', 'noeol')] + expected = [] + for c in codes: + c, sep, version = c.partition(':python') + if not sys.version.startswith(version): + continue + if c in ('Okay', 'noeol'): + continue + expected.append(c) # Run the checker - runner(filename, testcase, expected=codes, + runner(filename, testcase, expected=expected, line_offset=line_offset) # output the real line numbers line_offset = index + 1 diff --git a/testsuite/test_api.py b/testsuite/test_api.py index 6549a46c..bfc59130 100644 --- a/testsuite/test_api.py +++ b/testsuite/test_api.py @@ -54,7 +54,7 @@ def check_dummy(physical_line, line_number): options = pycodestyle.StyleGuide().options self.assertTrue(any(func == check_dummy - for name, func, args in options.physical_checks)) + for func, args in options.physical_checks)) def test_register_logical_check(self): def check_dummy(logical_line, tokens): @@ -75,7 +75,7 @@ def check_dummy(logical_line, tokens): options = pycodestyle.StyleGuide().options self.assertTrue(any(func == check_dummy - for name, func, args in options.logical_checks)) + for func, args in options.logical_checks)) def test_register_ast_check(self): pycodestyle.register_check(DummyChecker, ['Z701']) @@ -83,11 +83,11 @@ def test_register_ast_check(self): self.assertTrue(DummyChecker in pycodestyle._checks['tree']) codes, args = pycodestyle._checks['tree'][DummyChecker] self.assertTrue('Z701' in codes) - self.assertTrue(args is None) + self.assertEqual(args, ['self', 'tree', 'filename']) options = pycodestyle.StyleGuide().options self.assertTrue(any(cls == DummyChecker - for name, cls, args in options.ast_checks)) + for cls, args in options.ast_checks)) def test_register_invalid_check(self): class InvalidChecker(DummyChecker): @@ -133,7 +133,7 @@ def test_styleguide(self): report = pycodestyle.StyleGuide(paths=[E11]).check_files() stdout = sys.stdout.getvalue().splitlines() self.assertEqual(len(stdout), report.total_errors) - self.assertEqual(report.total_errors, 17) + self.assertEqual(report.total_errors, 18) self.assertFalse(sys.stderr) self.reset() @@ -182,7 +182,8 @@ def parse_argv(argstring): self.assertEqual(options.select, ()) self.assertEqual( options.ignore, - ('E121', 'E123', 'E126', 'E226', 'E24', 'E704', 'W503') + ('E121', 'E123', 'E126', 'E226', 'E24', 'E704', 'W503', 'W740', + 'W741') ) options = parse_argv('--doctest').options @@ -254,14 +255,12 @@ def test_styleguide_checks(self): # Default lists of checkers self.assertTrue(len(pep8style.options.physical_checks) > 4) self.assertTrue(len(pep8style.options.logical_checks) > 10) - self.assertEqual(len(pep8style.options.ast_checks), 0) + self.assertTrue(len(pep8style.options.ast_checks) > 0) # Sanity check - for name, check, args in pep8style.options.physical_checks: - self.assertEqual(check.__name__, name) + for check, args in pep8style.options.physical_checks: self.assertEqual(args[0], 'physical_line') - for name, check, args in pep8style.options.logical_checks: - self.assertEqual(check.__name__, name) + for check, args in pep8style.options.logical_checks: self.assertEqual(args[0], 'logical_line') # Do run E11 checks