mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-29 19:17:12 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

[pylint] Implement useless-with-lock (#8321)

Browse source
This commit is contained in:
Harutaka Kawamura 2023-10-30 01:24:52 +09:00 • committed by GitHub
parent cda1c5dd35
commit 44e21cfada
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 253 additions and 0 deletions
Download patch file Download diff file Expand all files Collapse all files

56
crates/ruff_linter/resources/test/fixtures/pylint/useless_with_lock.py vendored Normal file
View file

@ -0,0 +1,56 @@
import threading
from threading import Lock, RLock, Condition, Semaphore, BoundedSemaphore
with threading.Lock(): # [useless-with-lock]
...
with Lock(): # [useless-with-lock]
...
with threading.Lock() as this_shouldnt_matter: # [useless-with-lock]
...
with threading.RLock(): # [useless-with-lock]
...
with RLock(): # [useless-with-lock]
...
with threading.Condition(): # [useless-with-lock]
...
with Condition(): # [useless-with-lock]
...
with threading.Semaphore(): # [useless-with-lock]
...
with Semaphore(): # [useless-with-lock]
...
with threading.BoundedSemaphore(): # [useless-with-lock]
...
with BoundedSemaphore(): # [useless-with-lock]
...
lock = threading.Lock()
with lock: # this is ok
...
rlock = threading.RLock()
with rlock: # this is ok
...
cond = threading.Condition()
with cond: # this is ok
...
sem = threading.Semaphore()
with sem: # this is ok
...
b_sem = threading.BoundedSemaphore()
with b_sem: # this is ok
...

3
crates/ruff_linter/src/checkers/ast/analyze/statement.rs
View file

@ -1186,6 +1186,9 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
if checker.enabled(Rule::ReadWholeFile) {
refurb::rules::read_whole_file(checker, with_stmt);
}
if checker.enabled(Rule::UselessWithLock) {
pylint::rules::useless_with_lock(checker, with_stmt);
}
}
Stmt::While(ast::StmtWhile { body, orelse, .. }) => {
if checker.enabled(Rule::FunctionUsesLoopVariable) {

1
crates/ruff_linter/src/codes.rs
View file

@ -277,6 +277,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Pylint, "W1514") => (RuleGroup::Preview, rules::pylint::rules::UnspecifiedEncoding),
#[allow(deprecated)]
(Pylint, "W1641") => (RuleGroup::Nursery, rules::pylint::rules::EqWithoutHash),
(Pylint, "W2101") => (RuleGroup::Preview, rules::pylint::rules::UselessWithLock),
(Pylint, "R0904") => (RuleGroup::Preview, rules::pylint::rules::TooManyPublicMethods),
(Pylint, "W2901") => (RuleGroup::Stable, rules::pylint::rules::RedefinedLoopName),
#[allow(deprecated)]

1
crates/ruff_linter/src/rules/pylint/mod.rs
View file

@ -115,6 +115,7 @@ mod tests {
#[test_case(Rule::UselessElseOnLoop, Path::new("useless_else_on_loop.py"))]
#[test_case(Rule::UselessImportAlias, Path::new("import_aliasing.py"))]
#[test_case(Rule::UselessReturn, Path::new("useless_return.py"))]
#[test_case(Rule::UselessWithLock, Path::new("useless_with_lock.py"))]
#[test_case(
Rule::YieldFromInAsyncFunction,
Path::new("yield_from_in_async_function.py")

2
crates/ruff_linter/src/rules/pylint/rules/mod.rs
View file

@ -64,6 +64,7 @@ pub(crate) use unspecified_encoding::*;
pub(crate) use useless_else_on_loop::*;
pub(crate) use useless_import_alias::*;
pub(crate) use useless_return::*;
pub(crate) use useless_with_lock::*;
pub(crate) use yield_from_in_async_function::*;
pub(crate) use yield_in_init::*;
@ -133,5 +134,6 @@ mod unspecified_encoding;
mod useless_else_on_loop;
mod useless_import_alias;
mod useless_return;
mod useless_with_lock;
mod yield_from_in_async_function;
mod yield_in_init;

86
crates/ruff_linter/src/rules/pylint/rules/useless_with_lock.rs Normal file
View file

@ -0,0 +1,86 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast};
use ruff_text_size::Ranged;
use crate::checkers::ast::Checker;
/// ## What it does
/// Checks for direct uses of lock objects in `with` statements.
///
/// ## Why is this bad?
/// Creating a lock (via `threading.Lock` or similar) in a `with` statement
/// has no effect, as locks are only relevant when shared between threads.
///
/// Instead, assign the lock to a variable outside the `with` statement,
/// and share that variable between threads.
///
/// ## Example
/// ```python
/// import threading
///
/// counter = 0
///
///
/// def increment():
/// global counter
///
/// with threading.Lock():
/// counter += 1
/// ```
///
/// Use instead:
/// ```python
/// import threading
///
/// counter = 0
/// lock = threading.Lock()
///
///
/// def increment():
/// global counter
///
/// with lock:
/// counter += 1
/// ```
///
/// ## References
/// - [Python documentation: `Lock Objects`](https://docs.python.org/3/library/threading.html#lock-objects)
#[violation]
pub struct UselessWithLock;
impl Violation for UselessWithLock {
#[derive_message_formats]
fn message(&self) -> String {
format!("Threading lock directly created in `with` statement has no effect")
}
}
/// PLW2101
pub(crate) fn useless_with_lock(checker: &mut Checker, with: &ast::StmtWith) {
for item in &with.items {
let Some(call) = item.context_expr.as_call_expr() else {
continue;
};
if !checker
.semantic()
.resolve_call_path(call.func.as_ref())
.is_some_and(|call_path| {
matches!(
call_path.as_slice(),
[
"threading",
"Lock" | "RLock" | "Condition" | "Semaphore" | "BoundedSemaphore"
]
)
})
{
return;
}
checker
.diagnostics
.push(Diagnostic::new(UselessWithLock, call.range()));
}
}

101
crates/ruff_linter/src/rules/pylint/snapshots/ruff_linter__rules__pylint__tests__PLW2101_useless_with_lock.py.snap Normal file
View file

@ -0,0 +1,101 @@
---
source: crates/ruff_linter/src/rules/pylint/mod.rs
---
useless_with_lock.py:5:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
5 | with threading.Lock(): # [useless-with-lock]
| ^^^^^^^^^^^^^^^^ PLW2101
6 | ...
|
useless_with_lock.py:8:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
6 | ...
7 |
8 | with Lock(): # [useless-with-lock]
| ^^^^^^ PLW2101
9 | ...
|
useless_with_lock.py:11:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
9 | ...
10 |
11 | with threading.Lock() as this_shouldnt_matter: # [useless-with-lock]
| ^^^^^^^^^^^^^^^^ PLW2101
12 | ...
|
useless_with_lock.py:14:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
12 | ...
13 |
14 | with threading.RLock(): # [useless-with-lock]
| ^^^^^^^^^^^^^^^^^ PLW2101
15 | ...
|
useless_with_lock.py:17:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
15 | ...
16 |
17 | with RLock(): # [useless-with-lock]
| ^^^^^^^ PLW2101
18 | ...
|
useless_with_lock.py:20:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
18 | ...
19 |
20 | with threading.Condition(): # [useless-with-lock]
| ^^^^^^^^^^^^^^^^^^^^^ PLW2101
21 | ...
|
useless_with_lock.py:23:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
21 | ...
22 |
23 | with Condition(): # [useless-with-lock]
| ^^^^^^^^^^^ PLW2101
24 | ...
|
useless_with_lock.py:26:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
24 | ...
25 |
26 | with threading.Semaphore(): # [useless-with-lock]
| ^^^^^^^^^^^^^^^^^^^^^ PLW2101
27 | ...
|
useless_with_lock.py:29:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
27 | ...
28 |
29 | with Semaphore(): # [useless-with-lock]
| ^^^^^^^^^^^ PLW2101
30 | ...
|
useless_with_lock.py:32:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
30 | ...
31 |
32 | with threading.BoundedSemaphore(): # [useless-with-lock]
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PLW2101
33 | ...
|
useless_with_lock.py:35:6: PLW2101 Threading lock directly created in `with` statement has no effect
|
33 | ...
34 |
35 | with BoundedSemaphore(): # [useless-with-lock]
| ^^^^^^^^^^^^^^^^^^ PLW2101
36 | ...
|