Add ChangeLog entry for too-many-nested-blocks and fix the errors in pylint.

6 files changed, 113 insertions, 106 deletions

diff --git a/ChangeLog b/ChangeLog
index 0d12748..6ad85d5 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -2,8 +2,11 @@ ChangeLog for Pylint
 --------------------
 
 --
+    * Add a new refactoring error, 'too-many-nested-blocks', which is emitted
+      when a function or a method has too many nested blocks, which makes the
+      code less readable and harder to understand. Closes issue #668.
 
-    * Add new message, 'unsubscriptable-object', that is emitted when
+    * Add a new error, 'unsubscriptable-object', that is emitted when
       value used in subscription expression doesn't support subscription
       (i.e. doesn't define __getitem__ method).
 
diff --git a/pylint/checkers/base.py b/pylint/checkers/base.py
index bd8a348..6929a7a 100644
--- a/pylint/checkers/base.py
+++ b/pylint/checkers/base.py
@@ -1655,10 +1655,10 @@ class ElifChecker(BaseTokenChecker):
                       'maintainable.'),
            }
     options = (('max-nested-blocks',
-               {'default' : 5, 'type' : 'int', 'metavar' : '<int>',
-                'help': 'Maximum number of nested blocks for function / '
-                'method body'}
-              ),)
+                {'default' : 5, 'type' : 'int', 'metavar' : '<int>',
+                 'help': 'Maximum number of nested blocks for function / '
+                         'method body'}
+               ),)
 
     def __init__(self, linter=None):
         BaseTokenChecker.__init__(self, linter)
@@ -1678,7 +1678,7 @@ class ElifChecker(BaseTokenChecker):
             elif token == 'if':
                 self._elifs.append(False)
 
-    def leave_module(self, node):
+    def leave_module(self, _):
         self._init()
 
     @check_messages('too-many-nested-blocks')
@@ -1689,12 +1689,11 @@ class ElifChecker(BaseTokenChecker):
     visit_while = visit_tryexcept
     visit_for = visit_while
 
-    def visit_ifexp(self, node):
+    def visit_ifexp(self, _):
         self._if_counter += 1
 
     def visit_comprehension(self, node):
-        for if_test in node.ifs:
-            self._if_counter += 1
+        self._if_counter += len(node.ifs)
 
     @check_messages('too-many-nested-blocks')
     def visit_if(self, node):
@@ -1702,7 +1701,7 @@ class ElifChecker(BaseTokenChecker):
         self._if_counter += 1
 
     @check_messages('too-many-nested-blocks')
-    def leave_functiondef(self, node):
+    def leave_functiondef(self, _):
         # new scope = reinitialize the stack of nested blocks
         self._nested_blocks = []
         # if there is a waiting message left, send it
diff --git a/pylint/checkers/format.py b/pylint/checkers/format.py
index 9b0f211..7930074 100644
--- a/pylint/checkers/format.py
+++ b/pylint/checkers/format.py
@@ -530,27 +530,28 @@ class FormatChecker(BaseTokenChecker):
                 depth += 1
             elif token[1] == ')':
                 depth -= 1
-                if not depth:
-                    # ')' can't happen after if (foo), since it would be a syntax error.
-                    if (tokens[i+1][1] in (':', ')', ']', '}', 'in') or
-                            tokens[i+1][0] in (tokenize.NEWLINE,
-                                               tokenize.ENDMARKER,
-                                               tokenize.COMMENT)):
-                        # The empty tuple () is always accepted.
-                        if i == start + 2:
-                            return
-                        if keyword_token == 'not':
-                            if not found_and_or:
-                                self.add_message('superfluous-parens', line=line_num,
-                                                 args=keyword_token)
-                        elif keyword_token in ('return', 'yield'):
+                if depth:
+                    continue
+                # ')' can't happen after if (foo), since it would be a syntax error.
+                if (tokens[i+1][1] in (':', ')', ']', '}', 'in') or
+                        tokens[i+1][0] in (tokenize.NEWLINE,
+                                           tokenize.ENDMARKER,
+                                           tokenize.COMMENT)):
+                    # The empty tuple () is always accepted.
+                    if i == start + 2:
+                        return
+                    if keyword_token == 'not':
+                        if not found_and_or:
                             self.add_message('superfluous-parens', line=line_num,
                                              args=keyword_token)
-                        elif keyword_token not in self._keywords_with_parens:
-                            if not (tokens[i+1][1] == 'in' and found_and_or):
-                                self.add_message('superfluous-parens', line=line_num,
-                                                 args=keyword_token)
-                    return
+                    elif keyword_token in ('return', 'yield'):
+                        self.add_message('superfluous-parens', line=line_num,
+                                         args=keyword_token)
+                    elif keyword_token not in self._keywords_with_parens:
+                        if not (tokens[i+1][1] == 'in' and found_and_or):
+                            self.add_message('superfluous-parens', line=line_num,
+                                             args=keyword_token)
+                return
             elif depth == 1:
                 # This is a tuple, which is always acceptable.
                 if token[1] == ',':
diff --git a/pylint/checkers/newstyle.py b/pylint/checkers/newstyle.py
index ee45f45..fe8fa1b 100644
--- a/pylint/checkers/newstyle.py
+++ b/pylint/checkers/newstyle.py
@@ -124,47 +124,46 @@ class NewStyleConflictChecker(BaseChecker):
                 continue
             call = expr.expr
             # skip the test if using super
-            if isinstance(call, astroid.Call) and \
-               isinstance(call.func, astroid.Name) and \
-               call.func.name == 'super':
-                confidence = (INFERENCE if helpers.has_known_bases(klass)
-                              else INFERENCE_FAILURE)
-                if not klass.newstyle:
-                    # super should not be used on an old style class
-                    self.add_message('super-on-old-class', node=node,
-                                     confidence=confidence)
-                else:
-                    # super first arg should be the class
-                    if not call.args and sys.version_info[0] == 3:
-                        # unless Python 3
-                        continue
+            if not (isinstance(call, astroid.Call) and
+                    isinstance(call.func, astroid.Name) and
+                    call.func.name == 'super'):
+                continue
+            confidence = (INFERENCE if helpers.has_known_bases(klass)
+                          else INFERENCE_FAILURE)
+            if not klass.newstyle:
+                # super should not be used on an old style class
+                self.add_message('super-on-old-class', node=node,
+                                 confidence=confidence)
+            else:
+                # super first arg should be the class
+                if not call.args and sys.version_info[0] == 3:
+                    # unless Python 3
+                    continue
 
-                    try:
-                        supcls = (call.args and next(call.args[0].infer())
-                                  or None)
-                    except astroid.InferenceError:
-                        continue
+                try:
+                    supcls = (call.args and next(call.args[0].infer())
+                              or None)
+                except astroid.InferenceError:
+                    continue
 
-                    if supcls is None:
-                        self.add_message('missing-super-argument', node=call,
-                                         confidence=confidence)
-                        continue
+                if supcls is None:
+                    self.add_message('missing-super-argument', node=call,
+                                     confidence=confidence)
+                    continue
 
-                    if klass is not supcls:
-                        name = None
-                        # if supcls is not YES, then supcls was infered
-                        # and use its name. Otherwise, try to look
-                        # for call.args[0].name
-                        if supcls is not astroid.YES:
-                            name = supcls.name
-                        else:
-                            if hasattr(call.args[0], 'name'):
-                                name = call.args[0].name
-                        if name is not None:
-                            self.add_message('bad-super-call',
-                                             node=call,
-                                             args=(name, ),
-                                             confidence=confidence)
+                if klass is not supcls:
+                    name = None
+                    # if supcls is not YES, then supcls was infered
+                    # and use its name. Otherwise, try to look
+                    # for call.args[0].name
+                    if supcls is not astroid.YES:
+                        name = supcls.name
+                    else:
+                        if hasattr(call.args[0], 'name'):
+                            name = call.args[0].name
+                    if name is not None:
+                        self.add_message('bad-super-call', node=call, args=(name, ),
+                                         confidence=confidence)
 
 
 def register(linter):
diff --git a/pylint/checkers/variables.py b/pylint/checkers/variables.py
index b30aaa6..6f0f5a3 100644
--- a/pylint/checkers/variables.py
+++ b/pylint/checkers/variables.py
@@ -579,20 +579,21 @@ builtins. Remember that you should avoid to define new builtins when possible.'
                 continue
             if isinstance(stmt, (astroid.Import, astroid.ImportFrom)):
                 # Detect imports, assigned to global statements.
-                if global_names:
-                    skip = False
-                    for import_name, import_alias in stmt.names:
-                        # If the import uses an alias, check only that.
-                        # Otherwise, check only the import name.
-                        if import_alias:
-                            if import_alias in global_names:
-                                skip = True
-                                break
-                        elif import_name in global_names:
+                if not global_names:
+                    continue
+                skip = False
+                for import_name, import_alias in stmt.names:
+                    # If the import uses an alias, check only that.
+                    # Otherwise, check only the import name.
+                    if import_alias:
+                        if import_alias in global_names:
                             skip = True
                             break
-                    if skip:
-                        continue
+                    elif import_name in global_names:
+                        skip = True
+                        break
+                if skip:
+                    continue
 
             # care about functions with unknown argument (builtins)
             if name in argnames:
@@ -887,6 +888,7 @@ builtins. Remember that you should avoid to define new builtins when possible.'
             start_index = len(self._to_consume) - 1
         # iterates through parent scopes, from the inner to the outer
         base_scope_type = self._to_consume[start_index][-1]
+        # pylint: disable=too-many-nested-blocks; refactoring this block is a pain.
         for i in range(start_index, -1, -1):
             to_consume, consumed, scope_type = self._to_consume[i]
             # if the current scope is a class scope but it's not the inner
@@ -958,11 +960,11 @@ builtins. Remember that you should avoid to define new builtins when possible.'
                         #      f = 42
                         if isinstance(frame, astroid.ClassDef) and name in frame.locals:
                             if isinstance(node.parent, astroid.Arguments):
-                                # Doing the following is fine:
-                                #   class A:
-                                #      x = 42
-                                #      y = lambda attr=x: attr
                                 if stmt.fromlineno <= defstmt.fromlineno:
+                                    # Doing the following is fine:
+                                    #   class A:
+                                    #      x = 42
+                                    #      y = lambda attr=x: attr
                                     self.add_message('used-before-assignment',
                                                      args=name, node=node)
                             else:
diff --git a/pylint/utils.py b/pylint/utils.py
index 2f8de80..9cf4405 100644
--- a/pylint/utils.py
+++ b/pylint/utils.py
@@ -568,29 +568,32 @@ class FileState(object):
         for msgid, lines in six.iteritems(msg_state):
             for lineno, state in list(lines.items()):
                 original_lineno = lineno
-                if first <= lineno <= last:
-                    # Set state for all lines for this block, if the
-                    # warning is applied to nodes.
-                    if  msgs_store.check_message_id(msgid).scope == WarningScope.NODE:
-                        if lineno > firstchildlineno:
-                            state = True
-                        first_, last_ = node.block_range(lineno)
-                    else:
-                        first_ = lineno
-                        last_ = last
-                    for line in range(first_, last_+1):
-                        # do not override existing entries
-                        if not line in self._module_msgs_state.get(msgid, ()):
-                            if line in lines: # state change in the same block
-                                state = lines[line]
-                                original_lineno = line
-                            if not state:
-                                self._suppression_mapping[(msgid, line)] = original_lineno
-                            try:
-                                self._module_msgs_state[msgid][line] = state
-                            except KeyError:
-                                self._module_msgs_state[msgid] = {line: state}
-                    del lines[lineno]
+                # pylint: disable=superfluous-parens
+                if not (first <= lineno <= last):
+                    continue
+                # Set state for all lines for this block, if the
+                # warning is applied to nodes.
+                if  msgs_store.check_message_id(msgid).scope == WarningScope.NODE:
+                    if lineno > firstchildlineno:
+                        state = True
+                    first_, last_ = node.block_range(lineno)
+                else:
+                    first_ = lineno
+                    last_ = last
+                for line in range(first_, last_+1):
+                    # do not override existing entries
+                    if line in self._module_msgs_state.get(msgid, ()):
+                        continue
+                    if line in lines: # state change in the same block
+                        state = lines[line]
+                        original_lineno = line
+                    if not state:
+                        self._suppression_mapping[(msgid, line)] = original_lineno
+                    try:
+                        self._module_msgs_state[msgid][line] = state
+                    except KeyError:
+                        self._module_msgs_state[msgid] = {line: state}
+                del lines[lineno]
 
     def set_msg_status(self, msg, line, status):
         """Set status (enabled/disable) for a given message at a given line"""