Skip to content

Commit

Permalink
add threading classes, docs, and int test
Browse files Browse the repository at this point in the history
  • Loading branch information
clavedeluna committed Oct 6, 2023
1 parent 4883916 commit e440928
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 25 deletions.
21 changes: 21 additions & 0 deletions integration_tests/test_with_threading_lock.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
from core_codemods.with_threading_lock import WithThreadingLock
from integration_tests.base_test import (
BaseIntegrationTest,
original_and_expected_from_code_path,
)


class TestWithThreadingLock(BaseIntegrationTest):
codemod = WithThreadingLock
code_path = "tests/samples/with_threading_lock.py"
original_code, expected_new_code = original_and_expected_from_code_path(
code_path,
[
(1, "lock = threading.Lock()\n"),
(2, "with lock:\n"),
(4, ' print("Hello")\n'),
],
)
expected_diff = '--- \n+++ \n@@ -1,3 +1,4 @@\n import threading\n-with threading.Lock():\n+lock = threading.Lock()\n+with lock:\n print("Hello")\n'
expected_line_change = "2"
change_description = WithThreadingLock.CHANGE_DESCRIPTION
2 changes: 2 additions & 0 deletions src/core_codemods/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from .upgrade_sslcontext_tls import UpgradeSSLContextTLS
from .url_sandbox import UrlSandbox
from .use_walrus_if import UseWalrusIf
from .with_threading_lock import WithThreadingLock

registry = CodemodCollection(
origin="pixee",
Expand Down Expand Up @@ -48,5 +49,6 @@
UpgradeSSLContextTLS,
UrlSandbox,
UseWalrusIf,
WithThreadingLock,
],
)
15 changes: 15 additions & 0 deletions src/core_codemods/docs/pixee_python_bad-lock-with-statement.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
This codemod separates creating a threading lock instance from calling it as a context manager.
Calling `with threading.Lock()` does not have the effect you would expect. The lock is not acquired.
Instead, to correctly acquire a lock, create the instance separately, before calling it as a context manager.

The change will apply to any of these `threading` classes: `Lock`, `RLock`, `Condition`, `Semaphore`, and `BoundedSemaphore`.

The change looks like this:

```diff
import threading
- with threading.Lock():
+ lock = threading.Lock()
+ with lock:
...
```
9 changes: 7 additions & 2 deletions src/core_codemods/with_threading_lock.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

class WithThreadingLock(SemgrepCodemod):
NAME = "bad-lock-with-statement"
SUMMARY = "Replace deprecated usage of threading.Lock context manager"
SUMMARY = "Replace deprecated usage of threading lock classes as context managers"
REVIEW_GUIDANCE = ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW
DESCRIPTION = "Separates threading lock instantiation and call with `with` statement into two steps."

Expand All @@ -20,7 +20,12 @@ def rule(cls):
- metavariable-pattern:
metavariable: $BODY
patterns:
- pattern: threading.Lock()
- pattern-either:
- pattern: threading.Lock()
- pattern: threading.RLock()
- pattern: threading.Condition()
- pattern: threading.Semaphore()
- pattern: threading.BoundedSemaphore()
- pattern-inside: |
import threading
...
Expand Down
63 changes: 40 additions & 23 deletions tests/codemods/test_with_threading_lock.py
Original file line number Diff line number Diff line change
@@ -1,59 +1,75 @@
import pytest
from core_codemods.with_threading_lock import WithThreadingLock
from tests.codemods.base_codemod_test import BaseSemgrepCodemodTest

each_class = pytest.mark.parametrize(
"klass",
[
"Lock",
"RLock",
"Condition",
"Semaphore",
"BoundedSemaphore",
],
)


class TestWithThreadingLock(BaseSemgrepCodemodTest):
codemod = WithThreadingLock

def test_rule_ids(self):
assert self.codemod.name() == "bad-lock-with-statement"

def test_import(self, tmpdir):
input_code = """import threading
with threading.Lock():
@each_class
def test_import(self, tmpdir, klass):
input_code = f"""import threading
with threading.{klass}():
...
"""
expected = """import threading
lock = threading.Lock()
expected = f"""import threading
lock = threading.{klass}()
with lock:
...
"""
self.run_and_assert(tmpdir, input_code, expected)

def test_from_import(self, tmpdir):
input_code = """from threading import Lock
with Lock():
@each_class
def test_from_import(self, tmpdir, klass):
input_code = f"""from threading import {klass}
with {klass}():
...
"""
expected = """from threading import Lock
lock = Lock()
expected = f"""from threading import {klass}
lock = {klass}()
with lock:
...
"""
self.run_and_assert(tmpdir, input_code, expected)

def test_simple_replacement_with_as(self, tmpdir):
input_code = """import threading
with threading.Lock() as foo:
@each_class
def test_simple_replacement_with_as(self, tmpdir, klass):
input_code = f"""import threading
with threading.{klass}() as foo:
...
"""
expected = """import threading
lock = threading.Lock()
expected = f"""import threading
lock = threading.{klass}()
with lock as foo:
...
"""
self.run_and_assert(tmpdir, input_code, expected)

def test_no_effect_sanity_check(self, tmpdir):
input_code = """import threading
with threading.Lock():
@each_class
def test_no_effect_sanity_check(self, tmpdir, klass):
input_code = f"""import threading
with threading.{klass}():
...
with threading_lock():
...
"""
expected = """import threading
lock = threading.Lock()
expected = f"""import threading
lock = threading.{klass}()
with lock:
...
Expand All @@ -62,10 +78,11 @@ def test_no_effect_sanity_check(self, tmpdir):
"""
self.run_and_assert(tmpdir, input_code, expected)

def test_no_effect_multiple_with_clauses(self, tmpdir):
@each_class
def test_no_effect_multiple_with_clauses(self, tmpdir, klass):
"""This is currently an unsupported case"""
input_code = """import threading
with threading.Lock(), foo():
input_code = f"""import threading
with threading.{klass}(), foo():
...
"""
self.run_and_assert(tmpdir, input_code, input_code)
3 changes: 3 additions & 0 deletions tests/samples/with_threading_lock.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import threading
with threading.Lock():
print("Hello")

0 comments on commit e440928

Please sign in to comment.