Implement F701 and F702 (#169)

README.md

@@ -124,13 +124,12 @@ ruff's goal is to achieve feature-parity with Flake8 when used (1) without any p
 stylistic checks; limiting to Python 3 obviates the need for certain compatibility checks.)
 
 Under those conditions, Flake8 implements about 58 rules, give or take. At time of writing, ruff
-implements 34 rules. (Note that these 34 rules likely cover a disproportionate share of errors:
+implements 36 rules. (Note that these 36 rules likely cover a disproportionate share of errors:
 unused imports, undefined variables, etc.)
 
 Of the unimplemented rules, ruff is missing:
 
 - 15 rules related to string `.format` calls.
-- 6 rules related to misplaced `yield`, `break`, and `return` statements.
 - 3 rules related to syntax errors in doctests and annotations.
 - 2 rules related to redefining unused names.
 
@@ -169,6 +168,8 @@ Beyond rule-set parity, ruff suffers from the following limitations vis-à-vis F
 | F622 | TwoStarredExpressions | two starred expressions in assignment |
 | F631 | AssertTuple | Assert test is a non-empty tuple, which is always `True` |
 | F634 | IfTuple | If test is a tuple, which is always `True` |
+| F701 | BreakOutsideLoop | `break` outside loop |
+| F702 | ContinueOutsideLoop | `continue` not properly in loop |
 | F704 | YieldOutsideFunction | a `yield` or `yield from` statement outside of a function/method |
 | F706 | ReturnOutsideFunction | a `return` statement outside of a function/method |
 | F707 | DefaultExceptNotLast | an `except:` block as not the last exception handler |

examples/generate_rules_table.rs

@@ -3,10 +3,12 @@ use ruff::checks::{CheckKind, RejectedCmpop};
 
 fn main() {
     let mut check_kinds: Vec<CheckKind> = vec![
-        CheckKind::AmbiguousVariableName("...".to_string()),
         CheckKind::AmbiguousClassName("...".to_string()),
         CheckKind::AmbiguousFunctionName("...".to_string()),
+        CheckKind::AmbiguousVariableName("...".to_string()),
         CheckKind::AssertTuple,
+        CheckKind::BreakOutsideLoop,
+        CheckKind::ContinueOutsideLoop,
         CheckKind::DefaultExceptNotLast,
         CheckKind::DoNotAssignLambda,
         CheckKind::DoNotUseBareExcept,

resources/test/fixtures/F701.py

@@ -0,0 +1,23 @@
+for i in range(10):
+    break
+else:
+    break
+
+i = 0
+while i < 10:
+    i += 1
+    break
+
+
+def f():
+    for i in range(10):
+        break
+
+    break
+
+
+class Foo:
+    break
+
+
+break

resources/test/fixtures/F702.py

@@ -0,0 +1,23 @@
+for i in range(10):
+    continue
+else:
+    continue
+
+i = 0
+while i < 10:
+    i += 1
+    continue
+
+
+def f():
+    for i in range(10):
+        continue
+
+    continue
+
+
+class Foo:
+    continue
+
+
+continue

resources/test/fixtures/pyproject.toml

@@ -25,6 +25,8 @@ select = [
     "F622",
     "F631",
     "F634",
+    "F701",
+    "F702",
     "F704",
     "F706",
     "F707",

src/ast/checks.rs

@@ -3,7 +3,7 @@ use std::collections::BTreeSet;
 use itertools::izip;
 use rustpython_parser::ast::{
     Arg, Arguments, Cmpop, Constant, Excepthandler, ExcepthandlerKind, Expr, ExprKind, Keyword,
-    Location, Stmt, Unaryop,
+    Location, Stmt, StmtKind, Unaryop,
 };
 
 use crate::ast::operations::SourceCodeLocator;
@@ -468,3 +468,77 @@ pub fn check_starred_expressions(
 
     None
 }
+
+/// Check BreakOutsideLoop compliance.
+pub fn check_break_outside_loop(
+    stmt: &Stmt,
+    parents: &[&Stmt],
+    parent_stack: &[usize],
+) -> Option<Check> {
+    let mut allowed: bool = false;
+    let mut parent = stmt;
+    for index in parent_stack.iter().rev() {
+        let child = parent;
+        parent = parents[*index];
+        match &parent.node {
+            StmtKind::For { orelse, .. }
+            | StmtKind::AsyncFor { orelse, .. }
+            | StmtKind::While { orelse, .. } => {
+                if !orelse.contains(child) {
+                    allowed = true;
+                    break;
+                }
+            }
+
+            StmtKind::FunctionDef { .. }
+            | StmtKind::AsyncFunctionDef { .. }
+            | StmtKind::ClassDef { .. } => {
+                break;
+            }
+            _ => {}
+        }
+    }
+
+    if !allowed {
+        Some(Check::new(CheckKind::BreakOutsideLoop, stmt.location))
+    } else {
+        None
+    }
+}
+
+/// Check ContinueOutsideLoop compliance.
+pub fn check_continue_outside_loop(
+    stmt: &Stmt,
+    parents: &[&Stmt],
+    parent_stack: &[usize],
+) -> Option<Check> {
+    let mut allowed: bool = false;
+    let mut parent = stmt;
+    for index in parent_stack.iter().rev() {
+        let child = parent;
+        parent = parents[*index];
+        match &parent.node {
+            StmtKind::For { orelse, .. }
+            | StmtKind::AsyncFor { orelse, .. }
+            | StmtKind::While { orelse, .. } => {
+                if !orelse.contains(child) {
+                    allowed = true;
+                    break;
+                }
+            }
+
+            StmtKind::FunctionDef { .. }
+            | StmtKind::AsyncFunctionDef { .. }
+            | StmtKind::ClassDef { .. } => {
+                break;
+            }
+            _ => {}
+        }
+    }
+
+    if !allowed {
+        Some(Check::new(CheckKind::ContinueOutsideLoop, stmt.location))
+    } else {
+        None
+    }
+}

src/check_ast.rs

@@ -194,6 +194,24 @@ where
                     }));
                 }
             }
+            StmtKind::Break => {
+                if self.settings.select.contains(&CheckCode::F701) {
+                    if let Some(check) =
+                        checks::check_break_outside_loop(stmt, &self.parents, &self.parent_stack)
+                    {
+                        self.checks.push(check);
+                    }
+                }
+            }
+            StmtKind::Continue => {
+                if self.settings.select.contains(&CheckCode::F702) {
+                    if let Some(check) =
+                        checks::check_continue_outside_loop(stmt, &self.parents, &self.parent_stack)
+                    {
+                        self.checks.push(check);
+                    }
+                }
+            }
             StmtKind::FunctionDef {
                 name,
                 decorator_list,