Skip to content

Commit

Permalink
[pylint]: Implement continue-in-finally (E0116) (#3541)
Browse files Browse the repository at this point in the history
  • Loading branch information
latonis committed Mar 17, 2023
1 parent f5e5caa commit 73df267
Show file tree
Hide file tree
Showing 10 changed files with 383 additions and 0 deletions.
95 changes: 95 additions & 0 deletions crates/ruff/resources/test/fixtures/pylint/continue_in_finally.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
while True:
try:
pass
finally:
continue # [continue-in-finally]

while True:
try:
pass
except Exception:
continue
finally:
try:
pass
finally:
continue # [continue-in-finally]
pass

while True:
try:
pass
finally:
test = "aa"
match test:
case "aa":
continue # [continue-in-finally]

while True:
try:
pass
finally:
with "aa" as f:
continue # [continue-in-finally]

while True:
try:
pass
finally:
if True:
continue # [continue-in-finally]
continue # [continue-in-finally]

def test():
while True:
continue
try:
pass
finally:
continue # [continue-in-finally]


while True:
try:
pass
finally:
continue # [continue-in-finally]

def test():
while True:
continue


while True:
try:
pass
finally:
for i in range(12):
continue
continue # [continue-in-finally]

while True:
pass
else:
continue # [continue-in-finally]

def test():
continue
while True:
continue


while True:
try:
pass
finally:
if True:
pass
elif False:
continue # [continue-in-finally]
else:
continue # [continue-in-finally]
for i in range(10):
pass
else:
continue # [continue-in-finally]
8 changes: 8 additions & 0 deletions crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2041,9 +2041,17 @@ where
}

self.ctx.handled_exceptions.push(handled_exceptions);

if self.settings.rules.enabled(Rule::JumpStatementInFinally) {
flake8_bugbear::rules::jump_statement_in_finally(self, finalbody);
}

if self.settings.rules.enabled(Rule::ContinueInFinally) {
if self.settings.target_version <= PythonVersion::Py38 {
pylint::rules::continue_in_finally(self, finalbody);
}
}

self.visit_body(body);
self.ctx.handled_exceptions.pop();

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/codes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<Rule> {
(Pylint, "C3002") => Rule::UnnecessaryDirectLambdaCall,
(Pylint, "E0100") => Rule::YieldInInit,
(Pylint, "E0101") => Rule::ReturnInInit,
(Pylint, "E0116") => Rule::ContinueInFinally,
(Pylint, "E0117") => Rule::NonlocalWithoutBinding,
(Pylint, "E0118") => Rule::UsedPriorGlobalDeclaration,
(Pylint, "E0604") => Rule::InvalidAllObject,
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ ruff_macros::register_rules!(
rules::pylint::rules::BidirectionalUnicode,
rules::pylint::rules::BadStrStripCall,
rules::pylint::rules::CollapsibleElseIf,
rules::pylint::rules::ContinueInFinally,
rules::pylint::rules::UselessImportAlias,
rules::pylint::rules::UnnecessaryDirectLambdaCall,
rules::pylint::rules::NonlocalWithoutBinding,
Expand Down
15 changes: 15 additions & 0 deletions crates/ruff/src/rules/pylint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod tests {

use crate::registry::Rule;
use crate::rules::pylint;
use crate::settings::types::PythonVersion;
use crate::settings::Settings;
use crate::test::test_path;

Expand All @@ -37,6 +38,7 @@ mod tests {
#[test_case(Rule::ConsiderUsingSysExit, Path::new("consider_using_sys_exit_4.py"); "PLR1722_4")]
#[test_case(Rule::ConsiderUsingSysExit, Path::new("consider_using_sys_exit_5.py"); "PLR1722_5")]
#[test_case(Rule::ConsiderUsingSysExit, Path::new("consider_using_sys_exit_6.py"); "PLR1722_6")]
#[test_case(Rule::ContinueInFinally, Path::new("continue_in_finally.py"); "PLE0116")]
#[test_case(Rule::MagicValueComparison, Path::new("magic_value_comparison.py"); "PLR2004")]
#[test_case(Rule::UselessElseOnLoop, Path::new("useless_else_on_loop.py"); "PLW0120")]
#[test_case(Rule::GlobalVariableNotAssigned, Path::new("global_variable_not_assigned.py"); "PLW0602")]
Expand Down Expand Up @@ -65,6 +67,19 @@ mod tests {
Ok(())
}

#[test]
fn continue_in_finally() -> Result<()> {
let diagnostics = test_path(
Path::new("pylint/continue_in_finally.py"),
&Settings {
target_version: PythonVersion::Py37,
..Settings::for_rules(vec![Rule::ContinueInFinally])
},
)?;
assert_yaml_snapshot!(diagnostics);
Ok(())
}

#[test]
fn allow_magic_value_types() -> Result<()> {
let diagnostics = test_path(
Expand Down
80 changes: 80 additions & 0 deletions crates/ruff/src/rules/pylint/rules/continue_in_finally.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
use rustpython_parser::ast::{Stmt, StmtKind};

use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::types::Range;

use crate::checkers::ast::Checker;

/// ## What it does
/// Checks for `continue` statements inside `finally`
///
/// ## Why is this bad?
/// `continue` statements were not allowed within `finally` clauses prior to
/// Python 3.8. Using a `continue` statement within a `finally` clause can
/// cause a `SyntaxError`.
///
/// ## Example
/// ```python
/// while True:
/// try:
/// pass
/// finally:
/// continue
/// ```
///
/// Use instead:
/// ```python
/// while True:
/// try:
/// pass
/// except Exception:
/// pass
/// else:
/// continue
/// ```
#[violation]
pub struct ContinueInFinally;

impl Violation for ContinueInFinally {
#[derive_message_formats]
fn message(&self) -> String {
format!("`continue` not supported inside `finally` clause")
}
}

fn traverse_body(checker: &mut Checker, body: &[Stmt]) {
for stmt in body {
if matches!(stmt.node, StmtKind::Continue { .. }) {
checker
.diagnostics
.push(Diagnostic::new(ContinueInFinally, Range::from(stmt)));
}

match &stmt.node {
StmtKind::If { body, orelse, .. }
| StmtKind::Try { body, orelse, .. }
| StmtKind::TryStar { body, orelse, .. } => {
traverse_body(checker, body);
traverse_body(checker, orelse);
}
StmtKind::For { orelse, .. }
| StmtKind::AsyncFor { orelse, .. }
| StmtKind::While { orelse, .. } => traverse_body(checker, orelse),
StmtKind::With { body, .. } | StmtKind::AsyncWith { body, .. } => {
traverse_body(checker, body);
}
StmtKind::Match { cases, .. } => {
for case in cases {
traverse_body(checker, &case.body);
}
}
_ => {}
}
}
}

/// PLE0116
pub fn continue_in_finally(checker: &mut Checker, body: &[Stmt]) {
traverse_body(checker, body);
}
2 changes: 2 additions & 0 deletions crates/ruff/src/rules/pylint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub use collapsible_else_if::{collapsible_else_if, CollapsibleElseIf};
pub use compare_to_empty_string::{compare_to_empty_string, CompareToEmptyString};
pub use comparison_of_constant::{comparison_of_constant, ComparisonOfConstant};
pub use consider_using_sys_exit::{consider_using_sys_exit, ConsiderUsingSysExit};
pub use continue_in_finally::{continue_in_finally, ContinueInFinally};
pub use global_statement::{global_statement, GlobalStatement};
pub use global_variable_not_assigned::GlobalVariableNotAssigned;
pub use invalid_all_format::{invalid_all_format, InvalidAllFormat};
Expand Down Expand Up @@ -42,6 +43,7 @@ mod collapsible_else_if;
mod compare_to_empty_string;
mod comparison_of_constant;
mod consider_using_sys_exit;
mod continue_in_finally;
mod global_statement;
mod global_variable_not_assigned;
mod invalid_all_format;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
source: crates/ruff/src/rules/pylint/mod.rs
expression: diagnostics
---
[]

0 comments on commit 73df267

Please sign in to comment.