Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pylint]: Implement continue-in-finally (E0116) #3541

Merged
merged 14 commits into from
Mar 17, 2023
Merged
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);
MichaReiser marked this conversation as resolved.
Show resolved Hide resolved
}
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
---
[]

Loading