diff options
author | Martin <MartinBasti@users.noreply.github.com> | 2021-11-03 22:49:04 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-11-03 22:49:04 +0100 |
commit | de307ccf20c69ef1cfba05c000de19ec2407b5cd (patch) | |
tree | 84cc363659730c3819ca9710ecf4bce6f67954a9 | |
parent | be9e34722f521608a24a0d00e6991913bbad835a (diff) | |
download | pylint-git-de307ccf20c69ef1cfba05c000de19ec2407b5cd.tar.gz |
Inspection for `with threading.Lock():` (#5245)
Using `with threading.Lock():` directly has no effect.
Correct usage is:
```
lock = threading.Lock()
with lock:
...
```
This applies for:
* threading.Lock
* threading.RLock
* threading.Condition
* threading.Semaphore
* threading.BoundedSemaphore
Signed-off-by: Martin Basti <mbasti@redhat.com>
-rw-r--r-- | CONTRIBUTORS.txt | 1 | ||||
-rw-r--r-- | ChangeLog | 5 | ||||
-rw-r--r-- | doc/whatsnew/2.12.rst | 4 | ||||
-rw-r--r-- | pylint/checkers/threading_checker.py | 55 | ||||
-rw-r--r-- | tests/functional/u/useless/useless_with_lock.py | 58 | ||||
-rw-r--r-- | tests/functional/u/useless/useless_with_lock.txt | 11 |
6 files changed, 134 insertions, 0 deletions
diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 773c1b70a..64dc3f349 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -154,6 +154,7 @@ contributors: * Martin Bašti: contributor Added new check for shallow copy of os.environ + Added new check for useless `with threading.Lock():` statement * Jacques Kvam: contributor @@ -140,6 +140,11 @@ Release date: TBA Closes #5216 +* Added new checker ``useless-with-lock`` to find incorrect usage of with statement and threading module locks. + Emitted when ``with threading.Lock():`` is used instead of ``with lock_instance:``. + + Closes #5208 + What's New in Pylint 2.11.2? ============================ diff --git a/doc/whatsnew/2.12.rst b/doc/whatsnew/2.12.rst index 7fbb4ab71..b2e57a8a7 100644 --- a/doc/whatsnew/2.12.rst +++ b/doc/whatsnew/2.12.rst @@ -48,6 +48,10 @@ New checkers Closes #3370 +* Added new checker ``useless-with-lock`` to find incorrect usage of with statement and threading module locks. + Emitted when ``with threading.Lock():`` is used instead of ``with lock_instance:``. + + Closes #5208 Removed checkers ================ diff --git a/pylint/checkers/threading_checker.py b/pylint/checkers/threading_checker.py new file mode 100644 index 000000000..b7ba91d41 --- /dev/null +++ b/pylint/checkers/threading_checker.py @@ -0,0 +1,55 @@ +# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html +# For details: https://github.com/PyCQA/pylint/blob/main/LICENSE + +from astroid import nodes + +from pylint import interfaces +from pylint.checkers import BaseChecker +from pylint.checkers.utils import check_messages, safe_infer + + +class ThreadingChecker(BaseChecker): + """Checks for threading module + + - useless with lock - locking used in wrong way that has no effect (with threading.Lock():) + """ + + __implements__ = interfaces.IAstroidChecker + name = "threading" + + LOCKS = frozenset( + ( + "threading.Lock", + "threading.RLock", + "threading.Condition", + "threading.Semaphore", + "threading.BoundedSemaphore", + ) + ) + + msgs = { + "W2101": ( + "'%s()' directly created in 'with' has no effect", + "useless-with-lock", + "Used when a new lock instance is created by using with statement " + "which has no effect. Instead, an existing instance should be used to acquire lock.", + ), + } + + @check_messages("useless-with-lock") + def visit_with(self, node: nodes.With) -> None: + + context_managers = (c for c, _ in node.items if isinstance(c, nodes.Call)) + for context_manager in context_managers: + if isinstance(context_manager, nodes.Call): + infered_function = safe_infer(context_manager.func) + if infered_function is None: + continue + qname = infered_function.qname() + if qname in self.LOCKS: + self.add_message("useless-with-lock", node=node, args=qname) + + +def register(linter): + """required method to auto register this checker""" + linter.register_checker(ThreadingChecker(linter)) diff --git a/tests/functional/u/useless/useless_with_lock.py b/tests/functional/u/useless/useless_with_lock.py new file mode 100644 index 000000000..19d664084 --- /dev/null +++ b/tests/functional/u/useless/useless_with_lock.py @@ -0,0 +1,58 @@ +"""Tests for the useless-with-lock message""" +# pylint: disable=missing-docstring +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 + ... diff --git a/tests/functional/u/useless/useless_with_lock.txt b/tests/functional/u/useless/useless_with_lock.txt new file mode 100644 index 000000000..51435c35e --- /dev/null +++ b/tests/functional/u/useless/useless_with_lock.txt @@ -0,0 +1,11 @@ +useless-with-lock:7:0::'threading.Lock()' directly created in 'with' has no effect:HIGH +useless-with-lock:10:0::'threading.Lock()' directly created in 'with' has no effect:HIGH +useless-with-lock:13:0::'threading.Lock()' directly created in 'with' has no effect:HIGH +useless-with-lock:16:0::'threading.RLock()' directly created in 'with' has no effect:HIGH +useless-with-lock:19:0::'threading.RLock()' directly created in 'with' has no effect:HIGH +useless-with-lock:22:0::'threading.Condition()' directly created in 'with' has no effect:HIGH +useless-with-lock:25:0::'threading.Condition()' directly created in 'with' has no effect:HIGH +useless-with-lock:28:0::'threading.Semaphore()' directly created in 'with' has no effect:HIGH +useless-with-lock:31:0::'threading.Semaphore()' directly created in 'with' has no effect:HIGH +useless-with-lock:34:0::'threading.BoundedSemaphore()' directly created in 'with' has no effect:HIGH +useless-with-lock:37:0::'threading.BoundedSemaphore()' directly created in 'with' has no effect:HIGH |