Report unnecessary else after continue and break

Close #2327

16 files changed, 357 insertions, 29 deletions

diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt
index 068b119f5..8bc868d9b 100644
--- a/CONTRIBUTORS.txt
+++ b/CONTRIBUTORS.txt
@@ -319,3 +319,5 @@ contributors:
 * Danny Hermes: contributor
 
 * Eric Froemling: contributor
+
+* Robert Schweizer: contributor
diff --git a/ChangeLog b/ChangeLog
index 1bfec7da3..a8467aa91 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -20,6 +20,13 @@ Release date: TBA
 
   Close #3064
 
+* Added new checks, ``no-else-break`` and ``no-else-continue``
+
+  These checks highlight unnecessary ``else`` and ``elif`` blocks after
+  ``break`` and ``continue`` statements.
+
+  Close #2327
+
 * Don't emit ``protected-access`` when a single underscore prefixed attribute
   is used inside a special method
 
diff --git a/doc/whatsnew/2.4.rst b/doc/whatsnew/2.4.rst
index 4c279ca9a..e5bee7942 100644
--- a/doc/whatsnew/2.4.rst
+++ b/doc/whatsnew/2.4.rst
@@ -28,6 +28,13 @@ New checkers
 
   Close #2975
 
+* Added new checks, ``no-else-break`` and ``no-else-continue``
+
+  These checks highlight unnecessary ``else`` and ``elif`` blocks after
+  ``break`` and ``continue`` statements.
+
+  Close #2327
+
 * Added `unnecessary-comprehension` that detects unnecessary comprehensions.
 
   This check is emitted when ``pylint`` finds list-, set- or dict-comprehensions,
diff --git a/pylint/checkers/newstyle.py b/pylint/checkers/newstyle.py
index bc4947c6e..5985e222d 100644
--- a/pylint/checkers/newstyle.py
+++ b/pylint/checkers/newstyle.py
@@ -79,18 +79,14 @@ class NewStyleConflictChecker(BaseChecker):
             ):
                 continue
 
-            if not klass.newstyle and has_known_bases(klass):
-                # super should not be used on an old style class
-                continue
-            else:
+            # super should not be used on an old style class
+            if klass.newstyle or not has_known_bases(klass):
                 # super first arg should be the class
                 if not call.args:
-                    if sys.version_info[0] == 3:
+                    if sys.version_info[0] == 2:
                         # unless Python 3
-                        continue
-                    else:
                         self.add_message("missing-super-argument", node=call)
-                        continue
+                    continue
 
                 # calling super(type(self), self) can lead to recursion loop
                 # in derived classes
diff --git a/pylint/checkers/refactoring.py b/pylint/checkers/refactoring.py
index 98dd80827..05fef1f11 100644
--- a/pylint/checkers/refactoring.py
+++ b/pylint/checkers/refactoring.py
@@ -278,6 +278,24 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             "consider-using-sys-exit",
             "Instead of using exit() or quit(), consider using the sys.exit().",
         ),
+        "R1723": (
+            'Unnecessary "%s" after "break"',
+            "no-else-break",
+            "Used in order to highlight an unnecessary block of "
+            "code following an if containing a break statement. "
+            "As such, it will warn when it encounters an else "
+            "following a chain of ifs, all of them containing a "
+            "break statement.",
+        ),
+        "R1724": (
+            'Unnecessary "%s" after "continue"',
+            "no-else-continue",
+            "Used in order to highlight an unnecessary block of "
+            "code following an if containing a continue statement. "
+            "As such, it will warn when it encounters an else "
+            "following a chain of ifs, all of them containing a "
+            "continue statement.",
+        ),
     }
     options = (
         (
@@ -510,6 +528,16 @@ class RefactoringChecker(checkers.BaseTokenChecker):
             node, msg_id="no-else-raise", returning_node_class=astroid.Raise
         )
 
+    def _check_superfluous_else_break(self, node):
+        return self._check_superfluous_else(
+            node, msg_id="no-else-break", returning_node_class=astroid.Break
+        )
+
+    def _check_superfluous_else_continue(self, node):
+        return self._check_superfluous_else(
+            node, msg_id="no-else-continue", returning_node_class=astroid.Continue
+        )
+
     def _check_consider_get(self, node):
         def type_and_name_are_equal(node_a, node_b):
             for _type in [astroid.Name, astroid.AssignName]:
@@ -550,6 +578,8 @@ class RefactoringChecker(checkers.BaseTokenChecker):
         "simplifiable-if-statement",
         "no-else-return",
         "no-else-raise",
+        "no-else-break",
+        "no-else-continue",
         "consider-using-get",
     )
     def visit_if(self, node):
@@ -557,6 +587,8 @@ class RefactoringChecker(checkers.BaseTokenChecker):
         self._check_nested_blocks(node)
         self._check_superfluous_else_return(node)
         self._check_superfluous_else_raise(node)
+        self._check_superfluous_else_break(node)
+        self._check_superfluous_else_continue(node)
         self._check_consider_get(node)
 
     @utils.check_messages("simplifiable-if-expression")
diff --git a/pylint/checkers/stdlib.py b/pylint/checkers/stdlib.py
index 35f5b4e0b..39a65a48d 100644
--- a/pylint/checkers/stdlib.py
+++ b/pylint/checkers/stdlib.py
@@ -289,7 +289,7 @@ class StdlibChecker(BaseChecker):
             for inferred in node.func.infer():
                 if inferred is astroid.Uninferable:
                     continue
-                elif inferred.root().name == OPEN_MODULE:
+                if inferred.root().name == OPEN_MODULE:
                     if getattr(node.func, "name", None) in OPEN_FILES:
                         self._check_open_mode(node)
                 elif inferred.root().name == UNITTEST_CASE:
diff --git a/pylint/lint.py b/pylint/lint.py
index 4066f8ef0..eb3aef61e 100644
--- a/pylint/lint.py
+++ b/pylint/lint.py
@@ -1425,9 +1425,7 @@ def fix_import_path(args):
     changes = []
     for arg in args:
         path = _get_python_path(arg)
-        if path in changes:
-            continue
-        else:
+        if path not in changes:
             changes.append(path)
     sys.path[:] = changes + ["."] + sys.path
     try:
diff --git a/pylint/test/functional/no_else_break.py b/pylint/test/functional/no_else_break.py
new file mode 100644
index 000000000..cf23f313a
--- /dev/null
+++ b/pylint/test/functional/no_else_break.py
@@ -0,0 +1,137 @@
+""" Test that superfluous else return are detected. """
+
+# pylint:disable=invalid-name,missing-docstring,unused-variable
+
+
+def foo1(x, y, z):
+    for i in x:
+        if i > y:  # [no-else-break]
+            break
+        else:
+            a = z
+
+
+def foo2(x, y, w, z):
+    for i in x:
+        if i > y:  # [no-else-break]
+            break
+        elif i > w:
+            break
+        else:
+            a = z
+
+
+def foo3(x, y, z):
+    for i in x:
+        if i > y:
+            a = 1
+            if z:  # [no-else-break]
+                b = 2
+                break
+            else:
+                c = 3
+                break
+        else:
+            d = 4
+            break
+
+
+def foo4(x, y):
+    for i in x:
+        if i > x:  # [no-else-break]
+            if y:
+                a = 4
+            else:
+                b = 2
+            break
+        else:
+            c = 3
+        break
+
+
+def foo5(x, y, z):
+    for i in x:
+        if i > y:  # [no-else-break]
+            if y:
+                a = 4
+            else:
+                b = 2
+            break
+        elif z:
+            c = 2
+        else:
+            c = 3
+        break
+
+
+def foo6(x, y):
+    for i in x:
+        if i > x:
+            if y:  # [no-else-break]
+                a = 4
+                break
+            else:
+                b = 2
+        else:
+            c = 3
+        break
+
+
+def bar1(x, y, z):
+    for i in x:
+        if i > y:
+            break
+        return z
+
+
+def bar2(w, x, y, z):
+    for i in w:
+        if i > x:
+            break
+        if z < i:
+            a = y
+        else:
+            break
+        return
+
+
+def bar3(x, y, z):
+    for i in x:
+        if i > y:
+            if z:
+                break
+        else:
+            return z
+        return None
+
+
+def bar4(x):
+    for i in range(10):
+        if x:  # [no-else-break]
+            break
+        else:
+            try:
+                return
+            except ValueError:
+                break
+
+
+def nested1(x, y, z):
+    for i in x:
+        if i > x:
+            for j in y:
+                if j < z:
+                    break
+        else:
+            a = z
+
+
+def nested2(x, y, z):
+    for i in x:
+        if i > x:
+            for j in y:
+                if j > z:
+                    break
+                break
+        else:
+            a = z
diff --git a/pylint/test/functional/no_else_break.txt b/pylint/test/functional/no_else_break.txt
new file mode 100644
index 000000000..e08264fb1
--- /dev/null
+++ b/pylint/test/functional/no_else_break.txt
@@ -0,0 +1,7 @@
+no-else-break:7:foo1:Unnecessary "else" after "break"
+no-else-break:15:foo2:Unnecessary "elif" after "break"
+no-else-break:27:foo3:Unnecessary "else" after "break"
+no-else-break:40:foo4:Unnecessary "else" after "break"
+no-else-break:53:foo5:Unnecessary "elif" after "break"
+no-else-break:69:foo6:Unnecessary "else" after "break"
+no-else-break:109:bar4:Unnecessary "else" after "break"
diff --git a/pylint/test/functional/no_else_continue.py b/pylint/test/functional/no_else_continue.py
new file mode 100644
index 000000000..37f70f121
--- /dev/null
+++ b/pylint/test/functional/no_else_continue.py
@@ -0,0 +1,137 @@
+""" Test that superfluous else return are detected. """
+
+# pylint:disable=invalid-name,missing-docstring,unused-variable
+
+
+def foo1(x, y, z):
+    for i in x:
+        if i < y:  # [no-else-continue]
+            continue
+        else:
+            a = z
+
+
+def foo2(x, y, w, z):
+    for i in x:
+        if i < y:  # [no-else-continue]
+            continue
+        elif i < w:
+            continue
+        else:
+            a = z
+
+
+def foo3(x, y, z):
+    for i in x:
+        if i < y:
+            a = 1
+            if z:  # [no-else-continue]
+                b = 2
+                continue
+            else:
+                c = 3
+                continue
+        else:
+            d = 4
+            continue
+
+
+def foo4(x, y):
+    for i in x:
+        if i < x:  # [no-else-continue]
+            if y:
+                a = 4
+            else:
+                b = 2
+            continue
+        else:
+            c = 3
+        continue
+
+
+def foo5(x, y, z):
+    for i in x:
+        if i < y:  # [no-else-continue]
+            if y:
+                a = 4
+            else:
+                b = 2
+            continue
+        elif z:
+            c = 2
+        else:
+            c = 3
+        continue
+
+
+def foo6(x, y):
+    for i in x:
+        if i < x:
+            if y:  # [no-else-continue]
+                a = 4
+                continue
+            else:
+                b = 2
+        else:
+            c = 3
+        continue
+
+
+def bar1(x, y, z):
+    for i in x:
+        if i < y:
+            continue
+        return z
+
+
+def bar2(w, x, y, z):
+    for i in w:
+        if i < x:
+            continue
+        if z < i:
+            a = y
+        else:
+            continue
+        return
+
+
+def bar3(x, y, z):
+    for i in x:
+        if i < y:
+            if z:
+                continue
+        else:
+            return z
+        return None
+
+
+def bar4(x):
+    for i in range(10):
+        if x:  # [no-else-continue]
+            continue
+        else:
+            try:
+                return
+            except ValueError:
+                continue
+
+
+def nested1(x, y, z):
+    for i in x:
+        if i < x:
+            for j in y:
+                if j < z:
+                    continue
+        else:
+            a = z
+
+
+def nested2(x, y, z):
+    for i in x:
+        if i < x:
+            for j in y:
+                if j < z:
+                    continue
+                continue
+        else:
+            a = z
diff --git a/pylint/test/functional/no_else_continue.txt b/pylint/test/functional/no_else_continue.txt
new file mode 100644
index 000000000..33ddbf9d1
--- /dev/null
+++ b/pylint/test/functional/no_else_continue.txt
@@ -0,0 +1,7 @@
+no-else-continue:7:foo1:Unnecessary "else" after "continue"
+no-else-continue:15:foo2:Unnecessary "elif" after "continue"
+no-else-continue:27:foo3:Unnecessary "else" after "continue"
+no-else-continue:40:foo4:Unnecessary "else" after "continue"
+no-else-continue:53:foo5:Unnecessary "elif" after "continue"
+no-else-continue:69:foo6:Unnecessary "else" after "continue"
+no-else-continue:109:bar4:Unnecessary "else" after "continue"
diff --git a/tests/functional/inconsistent_returns.py b/tests/functional/inconsistent_returns.py
index 619165611..c0a4aed04 100644
--- a/tests/functional/inconsistent_returns.py
+++ b/tests/functional/inconsistent_returns.py
@@ -1,4 +1,4 @@
-#pylint: disable=missing-docstring, no-else-return, invalid-name, unused-variable, superfluous-parens, try-except-raise
+#pylint: disable=missing-docstring, no-else-return, no-else-break, invalid-name, unused-variable, superfluous-parens, try-except-raise
 """Testing inconsistent returns"""
 import math
 import sys
diff --git a/tests/functional/lost_exception.py b/tests/functional/lost_exception.py
index fe7deddb7..c8fd3fe1f 100644
--- a/tests/functional/lost_exception.py
+++ b/tests/functional/lost_exception.py
@@ -8,13 +8,11 @@ def insidious_break_and_return():
             my_var += 1.0/i
             if i < -3:
                 break
-            else:
-                return my_var
+            return my_var
         finally:
             if i > -2:
                 break # [lost-exception]
-            else:
-                return my_var # [lost-exception]
+            return my_var # [lost-exception]
     return None
 
 
@@ -37,5 +35,4 @@ def break_and_return():
                 strange()
         if i:
             break
-        else:
-            return
+        return
diff --git a/tests/functional/lost_exception.txt b/tests/functional/lost_exception.txt
index 4fb37ec84..182ea4134 100644
--- a/tests/functional/lost_exception.txt
+++ b/tests/functional/lost_exception.txt
@@ -1,2 +1,2 @@
-lost-exception:15:insidious_break_and_return:break statement in finally block may swallow exception
-lost-exception:17:insidious_break_and_return:return statement in finally block may swallow exception
\ No newline at end of file
+lost-exception:14:insidious_break_and_return:break statement in finally block may swallow exception
+lost-exception:15:insidious_break_and_return:return statement in finally block may swallow exception
diff --git a/tests/functional/not_in_loop.py b/tests/functional/not_in_loop.py
index 3ea7573d2..f7319eef7 100644
--- a/tests/functional/not_in_loop.py
+++ b/tests/functional/not_in_loop.py
@@ -1,6 +1,7 @@
 """Test that not-in-loop is detected properly."""
 # pylint: disable=missing-docstring, invalid-name, too-few-public-methods
 # pylint: disable=useless-else-on-loop, using-constant-test, useless-object-inheritance
+# pylint: disable=no-else-continue
 
 while True:
     def ala():
diff --git a/tests/functional/not_in_loop.txt b/tests/functional/not_in_loop.txt
index 6730bfccd..293ab2f07 100644
--- a/tests/functional/not_in_loop.txt
+++ b/tests/functional/not_in_loop.txt
@@ -1,8 +1,8 @@
-not-in-loop:7:ala:'continue' not properly in loop
-not-in-loop:12::'continue' not properly in loop
-not-in-loop:15:lala:'continue' not properly in loop
-not-in-loop:19:A:'continue' not properly in loop
-not-in-loop:24::'continue' not properly in loop
-not-in-loop:29::'break' not properly in loop
-not-in-loop:32::'continue' not properly in loop
-not-in-loop:34::'break' not properly in loop
\ No newline at end of file
+not-in-loop:8:ala:'continue' not properly in loop
+not-in-loop:13::'continue' not properly in loop
+not-in-loop:16:lala:'continue' not properly in loop
+not-in-loop:20:A:'continue' not properly in loop
+not-in-loop:25::'continue' not properly in loop
+not-in-loop:30::'break' not properly in loop
+not-in-loop:33::'continue' not properly in loop
+not-in-loop:35::'break' not properly in loop