mirror of
https://github.com/astral-sh/ruff.git
synced 2026-10-28 10:45:37 +08:00
[suspicious] Add shared-clock-mock (#29074)
## Summary
This PR adds a `shared-clock-mock` rule in preview. Patching a clock or
sleep function on a shared module can affect unrelated threads or tasks,
including when the target is qualified with the consuming module:
```python
from unittest.mock import Mock, patch
from example import worker
sleep = Mock()
with patch.object(worker.time, "sleep", sleep):
worker.retry()
```
Instead, you should replace the consuming module's binding to keep the
fake local:
```python
from types import SimpleNamespace
from unittest.mock import Mock, patch
from example import worker
sleep = Mock()
with patch.object(worker, "time", SimpleNamespace(sleep=sleep)):
worker.retry()
```
We also flag patches of wall-clock, monotonic, and performance-counter
readings, plus `asyncio.sleep`. The rule recognizes `patch`,
`patch.object`, pytest-mock fixtures, and `monkeypatch.setattr` /
`delattr`. (Nested `.time` and `.asyncio` targets use a naming
heuristic, so a fixture that already installed a private clock can
produce a false positive.)
## Case analysis
The ecosystem run adds 601 diagnostics across eight projects.
### Counting sleep calls in LangChain
This reduced version of LangChain’s device-login test records the
polling interval before and after the server asks the client to slow
down.
```python
sleeps = []
monkeypatch.setattr(oauth_module.time, "sleep", sleeps.append)
login_chatgpt_device(store_path=tmp_path / "x.json", poll_interval=2.0)
assert sleeps[:2] == [2.0, 7.0]
```
The OAuth module does `import time`, so `oauth_module.time` and `time`
are the same object. If another thread calls `time.sleep(1)` between the
two polls, the recorder sees `[2.0, 1, 7.0]`. Installing
`SimpleNamespace(sleep=sleeps.append)` on `oauth_module.time` keeps that
call out of the list.
### Running out of clock readings in Superset
Superset tests how long a screenshot request waits for a lock. It
patches `superset.dashboards.api.time.monotonic` with
`side_effect=[10.0, 10.5, 11.0]` and patches `time.sleep` on the same
module. An unrelated `time.monotonic()` can consume the next reading.
The request can then see the deadline early or exhaust the iterator and
raise `StopIteration`. A private clock for `superset.dashboards.api`
needs both `monotonic` and `sleep`.
### Retrying an async request in Prefect
Prefect simulates three failed SQS requests and expects one two-second
backoff. The test uses
`@patch("prefect_aws.observers.ecs.asyncio.sleep")`, but the observer
does `import asyncio`. A second task awaiting `asyncio.sleep(0)` would
also call the mock and break `assert_called_once_with(2)`.
### A fixture that already installed a private clock
Ruff can see the same patch in two tests even though only one reaches
the shared module. For example, an autouse fixture can replace
`worker.time` before the test starts.
```python
@pytest.fixture(autouse=True)
def private_clock(monkeypatch):
monkeypatch.setattr(worker, "time", SimpleNamespace(sleep=lambda _: None))
def test_retry(monkeypatch):
sleep = Mock()
monkeypatch.setattr(worker.time, "sleep", sleep) # shared-clock-mock
worker.retry()
```
The warning in `test_retry` is a false positive. At runtime the receiver
is the fixture’s `SimpleNamespace`. If the fixture returns that
namespace, the test can use `monkeypatch.setattr(private_clock, "sleep",
sleep)` and Ruff accepts it.
### A package with its own `time` module
Gevent ships a `gevent.time` module with its own `sleep` binding. The
dotted string is enough for Ruff to flag this reduced example.
```python
import time
from unittest.mock import patch
original_sleep = time.sleep
with patch("gevent.time.sleep"):
assert time.sleep is original_sleep
```
The assertion passes. Callers that look up `gevent.time.sleep` see the
patch. A caller of standard-library `time.sleep` still gets the original
function. Ruff infers the owner from the `.time.sleep` suffix in a
string target, so it reports the wrong module here.
This commit is contained in:
Charlie Marsh authored and GitHub committed
1 parent
538b55873b
commit
9525a0c4e8
7 files changed
+524
No files matched your search
@@ -0,0 +1,287 @@
|
||||
# `shared-clock-mock`
|
||||
|
||||
```toml
|
||||
lint.preview = true
|
||||
lint.select = ["shared-clock-mock"]
|
||||
```
|
||||
|
||||
## Shared sleep functions
|
||||
|
||||
Patching a shared module's sleep function can intercept unrelated callers, even
|
||||
when the target is qualified with the consuming module.
|
||||
|
||||
```py
|
||||
import time
|
||||
from unittest.mock import patch
|
||||
|
||||
from example import worker
|
||||
|
||||
with patch("time.sleep"): # snapshot: shared-clock-mock
|
||||
pass
|
||||
|
||||
with patch.object(time, "sleep"): # error: [shared-clock-mock]
|
||||
pass
|
||||
|
||||
with patch("example.worker.time.sleep"): # error: [shared-clock-mock]
|
||||
pass
|
||||
|
||||
with patch.object(worker.time, "sleep"): # error: [shared-clock-mock]
|
||||
pass
|
||||
```
|
||||
|
||||
```snapshot
|
||||
error[shared-clock-mock]: Changing `time.sleep` may affect unrelated threads or tasks
|
||||
--> src/mdtest_snippet.py:6:12
|
||||
|
|
||||
6 | with patch("time.sleep"): # snapshot: shared-clock-mock
|
||||
| ^^^^^^^^^^^^
|
||||
help: Replace the binding in the consuming module with a private clock
|
||||
```
|
||||
|
||||
## Clock readings
|
||||
|
||||
Unrelated callers can also consume a clock mock's finite sequence of readings.
|
||||
|
||||
```py
|
||||
import time
|
||||
from unittest.mock import patch
|
||||
|
||||
patch.object(time, "time") # error: [shared-clock-mock]
|
||||
patch.object(time, "time_ns") # error: [shared-clock-mock]
|
||||
patch.object(time, "monotonic") # error: [shared-clock-mock]
|
||||
patch.object(time, "monotonic_ns") # error: [shared-clock-mock]
|
||||
patch.object(time, "perf_counter") # error: [shared-clock-mock]
|
||||
patch.object(time, "perf_counter_ns") # error: [shared-clock-mock]
|
||||
patch("example.worker.time.perf_counter") # error: [shared-clock-mock]
|
||||
|
||||
# Other time APIs are outside this rule's scope.
|
||||
patch.object(time, "strftime")
|
||||
patch("time.localtime")
|
||||
```
|
||||
|
||||
## Import aliases and keyword arguments
|
||||
|
||||
```py
|
||||
import asyncio as aio
|
||||
import time as clock
|
||||
from unittest import mock as testing
|
||||
from unittest.mock import patch as replace
|
||||
|
||||
testing.patch.object(clock, "sleep") # error: [shared-clock-mock]
|
||||
testing.patch(target="asyncio.sleep") # error: [shared-clock-mock]
|
||||
replace.object(target=aio, attribute="sleep") # error: [shared-clock-mock]
|
||||
replace(target="time.monotonic", return_value=0) # error: [shared-clock-mock]
|
||||
|
||||
|
||||
@replace("example.worker.asyncio.sleep") # error: [shared-clock-mock]
|
||||
async def example(sleep):
|
||||
pass
|
||||
```
|
||||
|
||||
## Pytest fixtures
|
||||
|
||||
The conventional fixture names identify pytest's monkeypatch API and pytest-mock's
|
||||
patch APIs without requiring an import in the test file.
|
||||
|
||||
```py
|
||||
import time
|
||||
|
||||
from example import worker
|
||||
|
||||
|
||||
def example(monkeypatch, mocker, class_mocker, module_mocker, package_mocker, session_mocker):
|
||||
monkeypatch.setattr(time, "sleep", lambda _: None) # error: [shared-clock-mock]
|
||||
monkeypatch.setattr(worker.time, "sleep", lambda _: None) # error: [shared-clock-mock]
|
||||
monkeypatch.setattr(worker.time, "monotonic", lambda: 0) # error: [shared-clock-mock]
|
||||
monkeypatch.setattr("example.worker.asyncio.sleep", fake_sleep) # error: [shared-clock-mock]
|
||||
monkeypatch.setattr(target=time, name="perf_counter", value=lambda: 0) # error: [shared-clock-mock]
|
||||
|
||||
mocker.patch("time.sleep") # error: [shared-clock-mock]
|
||||
class_mocker.patch("time.sleep") # error: [shared-clock-mock]
|
||||
module_mocker.patch("time.sleep") # error: [shared-clock-mock]
|
||||
package_mocker.patch("time.sleep") # error: [shared-clock-mock]
|
||||
session_mocker.patch("time.sleep") # error: [shared-clock-mock]
|
||||
|
||||
mocker.patch.object(time, "sleep") # error: [shared-clock-mock]
|
||||
class_mocker.patch.object(time, "sleep") # error: [shared-clock-mock]
|
||||
module_mocker.patch.object(time, "sleep") # error: [shared-clock-mock]
|
||||
package_mocker.patch.object(time, "sleep") # error: [shared-clock-mock]
|
||||
session_mocker.patch.object(time, "sleep") # error: [shared-clock-mock]
|
||||
```
|
||||
|
||||
## Writes to clock attributes
|
||||
|
||||
Assigning or deleting a shared attribute changes it for every caller. Reading
|
||||
the attribute, annotating it without a value, or replacing the consuming
|
||||
module's binding does not.
|
||||
|
||||
```py
|
||||
import asyncio as aio
|
||||
import time as clock
|
||||
from types import SimpleNamespace
|
||||
|
||||
from example import worker
|
||||
|
||||
clock.sleep = fake # error: [shared-clock-mock]
|
||||
worker.time.monotonic = fake # error: [shared-clock-mock]
|
||||
del aio.sleep # error: [shared-clock-mock]
|
||||
worker.time.time: object = fake # error: [shared-clock-mock]
|
||||
|
||||
saved = worker.time.sleep
|
||||
worker.time.sleep: object
|
||||
worker.time = SimpleNamespace(sleep=fake)
|
||||
```
|
||||
|
||||
Attribute targets also write to the shared module when bound by a loop, a
|
||||
context manager, or a comprehension.
|
||||
|
||||
```py
|
||||
import time
|
||||
|
||||
for time.sleep in replacements: # error: [shared-clock-mock]
|
||||
pass
|
||||
with replacement() as time.monotonic: # error: [shared-clock-mock]
|
||||
pass
|
||||
readings = [time.time() for time.time in clocks] # error: [shared-clock-mock]
|
||||
```
|
||||
|
||||
## Removing attributes with monkeypatch
|
||||
|
||||
```py
|
||||
import time
|
||||
|
||||
def example(monkeypatch):
|
||||
monkeypatch.delattr(time, "sleep") # error: [shared-clock-mock]
|
||||
monkeypatch.delattr("example.worker.time.monotonic") # error: [shared-clock-mock]
|
||||
monkeypatch.delattr("example.worker.time")
|
||||
```
|
||||
|
||||
## Constructed monkeypatch instances
|
||||
|
||||
```py
|
||||
import time
|
||||
from pytest import MonkeyPatch
|
||||
|
||||
mp = MonkeyPatch()
|
||||
mp.setattr(time, "sleep", lambda _: None) # error: [shared-clock-mock]
|
||||
```
|
||||
|
||||
## Multiline patch targets
|
||||
|
||||
The diagnostic spans both the module and the attribute that will change.
|
||||
|
||||
```py
|
||||
from example import worker
|
||||
|
||||
def test_retry(monkeypatch):
|
||||
monkeypatch.setattr(
|
||||
worker.time, # snapshot: shared-clock-mock
|
||||
"sleep",
|
||||
lambda _: None,
|
||||
)
|
||||
```
|
||||
|
||||
```snapshot
|
||||
error[shared-clock-mock]: Changing `time.sleep` may affect unrelated threads or tasks
|
||||
--> src/mdtest_snippet.py:5:9
|
||||
|
|
||||
5 | / worker.time, # snapshot: shared-clock-mock
|
||||
6 | | "sleep",
|
||||
| |_______________^
|
||||
help: Replace the binding in the consuming module with a private clock
|
||||
```
|
||||
|
||||
## Module-local replacements
|
||||
|
||||
Replacing the consuming module's binding leaves the shared module intact. A local
|
||||
sleep binding and a directly constructed clock can also be patched independently.
|
||||
|
||||
```py
|
||||
import asyncio
|
||||
import time
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, Mock, patch
|
||||
|
||||
from example import worker
|
||||
|
||||
|
||||
def example(monkeypatch, mocker):
|
||||
monkeypatch.setattr(worker, "time", SimpleNamespace(sleep=Mock()))
|
||||
monkeypatch.setattr("example.worker.time", SimpleNamespace(sleep=Mock()))
|
||||
mocker.patch.object(worker, "time", SimpleNamespace(sleep=Mock()))
|
||||
mocker.patch("example.worker.time", SimpleNamespace(sleep=Mock()))
|
||||
monkeypatch.setattr(worker, "asyncio", SimpleNamespace(sleep=AsyncMock()))
|
||||
monkeypatch.setattr(worker, "sleep", Mock())
|
||||
mocker.patch("example.worker.sleep")
|
||||
|
||||
|
||||
with patch.object(worker, "time", SimpleNamespace(sleep=Mock())):
|
||||
pass
|
||||
|
||||
with patch("example.worker.asyncio", SimpleNamespace(**(vars(asyncio) | {"sleep": AsyncMock()}))):
|
||||
pass
|
||||
|
||||
time = SimpleNamespace(sleep=Mock())
|
||||
patch.object(time, "sleep")
|
||||
```
|
||||
|
||||
## Unrelated methods and shadowed patch APIs
|
||||
|
||||
```py
|
||||
import time
|
||||
from unittest.mock import patch
|
||||
|
||||
from example import mocker, monkeypatch
|
||||
|
||||
mocker.patch("time.sleep")
|
||||
monkeypatch.setattr(time, "sleep", fake)
|
||||
other.patch("time.sleep")
|
||||
other.setattr(time, "sleep", fake)
|
||||
|
||||
|
||||
def example(patch):
|
||||
patch("time.sleep")
|
||||
patch.object(time, "sleep")
|
||||
|
||||
|
||||
patch("asyncio.wait")
|
||||
patch("example.sleep")
|
||||
patch("example.clock.sleep")
|
||||
patch.object(time, dynamic_attribute)
|
||||
patch(dynamic_target)
|
||||
patch(f"{module_name}.time.sleep")
|
||||
```
|
||||
|
||||
## Attribute heuristic
|
||||
|
||||
Attribute targets ending in `time` or `asyncio` are treated as shared modules,
|
||||
including on dynamically loaded modules. An already-isolated attribute is a known
|
||||
false positive and can be suppressed by rule name.
|
||||
|
||||
```py
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import patch
|
||||
|
||||
module = load_module()
|
||||
patch.object(module.time, "sleep") # error: [shared-clock-mock]
|
||||
|
||||
module.time = SimpleNamespace(sleep=fake)
|
||||
patch.object(module.time, "sleep") # ruff: ignore[shared-clock-mock]
|
||||
```
|
||||
|
||||
A clock stored directly on an instance or class is usually part of its own
|
||||
state. A module stored on an instance can still expose the shared clock.
|
||||
|
||||
```py
|
||||
from unittest.mock import patch
|
||||
|
||||
class Worker:
|
||||
def set_clock(self):
|
||||
self.time.sleep = fake
|
||||
patch.object(self.time, "monotonic")
|
||||
patch.object(self.module.time, "sleep") # error: [shared-clock-mock]
|
||||
|
||||
@classmethod
|
||||
def reset_clock(cls):
|
||||
del cls.asyncio.sleep
|
||||
```
|
||||
@@ -399,6 +399,11 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
|
||||
}
|
||||
}
|
||||
Expr::Attribute(attribute) => {
|
||||
if checker.is_rule_enabled(Rule::SharedClockMock)
|
||||
&& matches!(attribute.ctx, ExprContext::Store | ExprContext::Del)
|
||||
{
|
||||
ruff::rules::shared_clock_attribute(checker, attribute);
|
||||
}
|
||||
if attribute.ctx == ExprContext::Load {
|
||||
if checker.any_rule_enabled(&[
|
||||
Rule::SuspiciousPickleUsage,
|
||||
@@ -1026,6 +1031,9 @@ pub(crate) fn expression(expr: &Expr, checker: &Checker) {
|
||||
if checker.is_rule_enabled(Rule::PytestPatchWithLambda) {
|
||||
flake8_pytest_style::rules::patch_with_lambda(checker, call);
|
||||
}
|
||||
if checker.is_rule_enabled(Rule::SharedClockMock) {
|
||||
ruff::rules::shared_clock_mock(checker, call);
|
||||
}
|
||||
if checker.any_rule_enabled(&[
|
||||
Rule::PytestParametrizeNamesWrongType,
|
||||
Rule::PytestParametrizeValuesWrongType,
|
||||
|
||||
@@ -1371,6 +1371,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleStatus, Rule)> {
|
||||
|
||||
// Rules that belong only to categories, without linter groups or codes.
|
||||
() => rules::ruff::rules::PytestFixtureAutouse,
|
||||
() => rules::ruff::rules::SharedClockMock,
|
||||
() => rules::ruff::rules::UnnecessaryRegularExpressionCompile,
|
||||
|
||||
_ => return None,
|
||||
|
||||
@@ -53,6 +53,7 @@ pub(crate) use redirected_noqa::*;
|
||||
pub(crate) use redundant_bool_literal::*;
|
||||
pub(crate) use rule_codes_in_selectors::*;
|
||||
pub(crate) use rule_codes_in_suppression_comments::*;
|
||||
pub(crate) use shared_clock_mock::*;
|
||||
pub(crate) use sort_dunder_all::*;
|
||||
pub(crate) use sort_dunder_slots::*;
|
||||
pub(crate) use starmap_zip::*;
|
||||
@@ -137,6 +138,7 @@ mod redundant_bool_literal;
|
||||
mod rule_codes_in_selectors;
|
||||
mod rule_codes_in_suppression_comments;
|
||||
mod sequence_sorting;
|
||||
mod shared_clock_mock;
|
||||
mod sort_dunder_all;
|
||||
mod sort_dunder_slots;
|
||||
mod starmap_zip;
|
||||
|
||||
@@ -0,0 +1,224 @@
|
||||
use ruff_macros::{ViolationMetadata, derive_message_formats};
|
||||
use ruff_python_ast::name::UnqualifiedName;
|
||||
use ruff_python_ast::{Expr, ExprAttribute, ExprCall, Stmt};
|
||||
use ruff_python_semantic::analyze::typing;
|
||||
use ruff_text_size::{Ranged, TextRange};
|
||||
|
||||
use crate::Violation;
|
||||
use crate::checkers::ast::Checker;
|
||||
use crate::codes::Category;
|
||||
|
||||
/// ## What it does
|
||||
/// Checks for patches, assignments, and deletions of shared clock and sleep
|
||||
/// functions in `time` and `asyncio`.
|
||||
///
|
||||
/// ## Why is this bad?
|
||||
/// If `worker` uses `import time`, then `worker.time` and `time` refer to
|
||||
/// the same module. Patching `worker.time.sleep` can make an
|
||||
/// `assert_called_once()` check fail when another thread calls `time.sleep`.
|
||||
/// The same can happen when another task calls a patched `asyncio.sleep`.
|
||||
///
|
||||
/// To keep the patch local to `worker`, replace `worker.time` or
|
||||
/// `worker.asyncio` with an object that provides the functions `worker` uses.
|
||||
///
|
||||
/// ## Example
|
||||
/// ```python
|
||||
/// from unittest.mock import Mock, patch
|
||||
///
|
||||
/// from example import worker
|
||||
///
|
||||
/// sleep = Mock()
|
||||
/// with patch.object(worker.time, "sleep", sleep):
|
||||
/// worker.retry()
|
||||
/// ```
|
||||
///
|
||||
/// Use instead:
|
||||
/// ```python
|
||||
/// from types import SimpleNamespace
|
||||
/// from unittest.mock import Mock, patch
|
||||
///
|
||||
/// from example import worker
|
||||
///
|
||||
/// sleep = Mock()
|
||||
/// with patch.object(worker, "time", SimpleNamespace(sleep=sleep)):
|
||||
/// worker.retry()
|
||||
/// ```
|
||||
///
|
||||
/// ## Known problems
|
||||
/// Ruff can flag a patch to `worker.time.sleep` even if a fixture has
|
||||
/// replaced `worker.time` with a private clock. It can miss a patch when
|
||||
/// the target is a dynamically built string or when the test assigns
|
||||
/// `clock = worker.time` and then patches `clock.sleep`.
|
||||
///
|
||||
/// ## References
|
||||
/// - [Python documentation: Where to patch](https://docs.python.org/3/library/unittest.mock.html#where-to-patch)
|
||||
/// - [pytest documentation: Monkeypatching](https://docs.pytest.org/en/stable/how-to/monkeypatch.html)
|
||||
#[derive(ViolationMetadata)]
|
||||
#[violation_metadata(preview_since = "NEXT_RUFF_VERSION", category = Category::Suspicious)]
|
||||
pub(crate) struct SharedClockMock {
|
||||
function: String,
|
||||
}
|
||||
|
||||
impl Violation for SharedClockMock {
|
||||
#[derive_message_formats]
|
||||
fn message(&self) -> String {
|
||||
let Self { function } = self;
|
||||
format!("Changing `{function}` may affect unrelated threads or tasks")
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
enum PatchApi {
|
||||
String,
|
||||
Object,
|
||||
MonkeyPatch,
|
||||
}
|
||||
|
||||
impl PatchApi {
|
||||
/// Recognize imported patch functions before falling back to fixture names.
|
||||
fn from_call(checker: &Checker, call: &ExprCall) -> Option<Self> {
|
||||
if let Some(name) = checker
|
||||
.semantic()
|
||||
.resolve_qualified_name(&call.func)
|
||||
.or_else(|| typing::resolve_assignment(&call.func, checker.semantic()))
|
||||
{
|
||||
return match name.segments() {
|
||||
["unittest", "mock", "patch"] | ["mock", "patch"] => Some(Self::String),
|
||||
["unittest", "mock", "patch", "object"] | ["mock", "patch", "object"] => {
|
||||
Some(Self::Object)
|
||||
}
|
||||
["pytest", "MonkeyPatch", "setattr" | "delattr"]
|
||||
| [
|
||||
"_pytest",
|
||||
"monkeypatch",
|
||||
"MonkeyPatch",
|
||||
"setattr" | "delattr",
|
||||
] => Some(Self::MonkeyPatch),
|
||||
_ => None,
|
||||
};
|
||||
}
|
||||
|
||||
let name = UnqualifiedName::from_expr(&call.func)?;
|
||||
match name.segments() {
|
||||
["monkeypatch", "setattr" | "delattr"] => Some(Self::MonkeyPatch),
|
||||
[
|
||||
"mocker" | "class_mocker" | "module_mocker" | "package_mocker" | "session_mocker",
|
||||
"patch",
|
||||
] => Some(Self::String),
|
||||
[
|
||||
"mocker" | "class_mocker" | "module_mocker" | "package_mocker" | "session_mocker",
|
||||
"patch",
|
||||
"object",
|
||||
] => Some(Self::Object),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// `shared-clock-mock`
|
||||
pub(crate) fn shared_clock_mock(checker: &Checker, call: &ExprCall) {
|
||||
let Some(api) = PatchApi::from_call(checker, call) else {
|
||||
return;
|
||||
};
|
||||
let Some(target) = call.arguments.find_argument_value("target", 0) else {
|
||||
return;
|
||||
};
|
||||
|
||||
let (module, member, range) = match api {
|
||||
PatchApi::String | PatchApi::MonkeyPatch
|
||||
if let Some(target) = target.as_string_literal_expr() =>
|
||||
{
|
||||
let Some((module, member)) = target.value.to_str().rsplit_once('.') else {
|
||||
return;
|
||||
};
|
||||
(module.rsplit('.').next(), member, target.range())
|
||||
}
|
||||
PatchApi::Object | PatchApi::MonkeyPatch => {
|
||||
let attribute = match api {
|
||||
PatchApi::MonkeyPatch => "name",
|
||||
_ => "attribute",
|
||||
};
|
||||
let Some(member) = call
|
||||
.arguments
|
||||
.find_argument_value(attribute, 1)
|
||||
.and_then(Expr::as_string_literal_expr)
|
||||
else {
|
||||
return;
|
||||
};
|
||||
(
|
||||
clock_module(checker, target),
|
||||
member.value.to_str(),
|
||||
target.range().cover(member.range()),
|
||||
)
|
||||
}
|
||||
PatchApi::String => return,
|
||||
};
|
||||
|
||||
report_shared_clock(checker, module, member, range);
|
||||
}
|
||||
|
||||
/// `shared-clock-mock`
|
||||
pub(crate) fn shared_clock_attribute(checker: &Checker, attribute: &ExprAttribute) {
|
||||
// An annotation without a value does not write to the attribute.
|
||||
if let Stmt::AnnAssign(assignment) = checker.semantic().current_statement()
|
||||
&& assignment.value.is_none()
|
||||
&& assignment.target.range() == attribute.range()
|
||||
{
|
||||
return;
|
||||
}
|
||||
report_shared_clock(
|
||||
checker,
|
||||
clock_module(checker, &attribute.value),
|
||||
attribute.attr.as_str(),
|
||||
attribute.range(),
|
||||
);
|
||||
}
|
||||
|
||||
fn report_shared_clock(checker: &Checker, module: Option<&str>, member: &str, range: TextRange) {
|
||||
if let Some(module) = module
|
||||
&& matches!(
|
||||
(module, member),
|
||||
(
|
||||
"time",
|
||||
"sleep"
|
||||
| "time"
|
||||
| "time_ns"
|
||||
| "monotonic"
|
||||
| "monotonic_ns"
|
||||
| "perf_counter"
|
||||
| "perf_counter_ns"
|
||||
) | ("asyncio", "sleep")
|
||||
)
|
||||
{
|
||||
let mut diagnostic = checker.report_diagnostic(
|
||||
SharedClockMock {
|
||||
function: format!("{module}.{member}"),
|
||||
},
|
||||
range,
|
||||
);
|
||||
diagnostic.help("Replace the binding in the consuming module with a private clock");
|
||||
}
|
||||
}
|
||||
|
||||
/// Resolve direct imports and use the final attribute as a heuristic for consumers.
|
||||
fn clock_module<'a>(checker: &Checker, target: &'a Expr) -> Option<&'a str> {
|
||||
if let Some(name) = checker.semantic().resolve_qualified_name(target) {
|
||||
match name.segments() {
|
||||
["time"] => return Some("time"),
|
||||
["asyncio"] => return Some("asyncio"),
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
let attribute = target.as_attribute_expr()?;
|
||||
if let Some(receiver) = attribute.value.as_name_expr()
|
||||
&& matches!(receiver.id.as_str(), "self" | "cls")
|
||||
&& checker
|
||||
.semantic()
|
||||
.resolve_name(receiver)
|
||||
.is_some_and(|id| checker.semantic().binding(id).kind.is_argument())
|
||||
{
|
||||
return None;
|
||||
}
|
||||
Some(attribute.attr.as_str())
|
||||
}
|
||||
@@ -1059,6 +1059,7 @@ mod tests {
|
||||
invalid-suppression-comment (RUF103),
|
||||
unmatched-suppression-comment (RUF104),
|
||||
rule-codes-in-suppression-comments (RUF106),
|
||||
shared-clock-mock,
|
||||
unnecessary-regular-expression-compile,
|
||||
]
|
||||
|
||||
|
||||
Generated
+1
@@ -5310,6 +5310,7 @@
|
||||
"self-or-cls-assignment",
|
||||
"set-attr-with-constant",
|
||||
"shallow-copy-environ",
|
||||
"shared-clock-mock",
|
||||
"shebang-leading-whitespace",
|
||||
"shebang-missing-executable-file",
|
||||
"shebang-missing-python",
|
||||
|
||||
Reference in new issue
Block a user