astral-sh / astral-sh/ruff

Rule idea: flag literal attributes in monkeypatching/mocking APIs

Open
#22,013 9 comments 1 reaction 0 assignees View on GitHub
needs-decision rule
Dominant language
Rust
Stars
49.6k
Forks
2.4k
Avg merge
2d 1h
Merged PRs (30d)
445

Description

### Summary

This is a rough idea that probably needs refining/discussion 🙂

## Overview

Pytest and others expose monkeypatch/mocking APIs like:

```python
monkeypatch.setattr(some_module, "my_important_api", replacement)
```

(and same for `delattr`.)

When called, this temporarily monkeypatches `some_module.my_important_api` with `replacement` (which could be an object, callable, etc.).

## Problem statement

The above works, but has a footgun: if the user performs a refactor (e.g. from `my_important_api` to `my_critical_api`), then may miss this monkeypatch (since it's a string literal, not a usage/reference). This is particularly likely to happen with IDE/LSP refactors, since syntax-aware rename-all functionality can't look at these kinds of non-type string literals by design.

The impact of this depends: sometimes users notice it immediately (since their monkeypatch stops working), while other times it makes tests subtly incorrect or unreliable in ways that aren't noticed until much later.

## Solution statement

In general, users should _probably_ write:

```python
monkeypatch.setattr(some_module, some_module.my_important_api.__name__, replacement)
```

That'll behave the same as above, _but_ will ensure that refactors/mechanical renames propagate as expected.

## Problems

1. This won't work with dynamic attributes; not sure if this is a problem or not.
2. Using `__name__` is arguably pretty unsightly and is maybe considered unidiomatic, not sure 🙂

## Alternatives

1. Do nothing.
2. Maybe Pytest and others with monkeypatching APIs should support functions as first-class "nameable" objects? In other words, Ruff could instead recommend:

```python
monkeypatch.setattr(some_module, some_module.my_important_api.__name__, replacement)
```

...and Pytest would do the `__name__` access internally. This would require upstream changes, but would be a lot visually cleaner from the user's side.

## Other references

xref https://github.com/astral-sh/ruff/issues/12850 for another (but I think unrelated) mocking/monkeypatching feature request.

Contributor guide

Open the contributing guide

Research direction

Start by reviewing the proposed monkeypatch.setattr and delattr examples and the related issue xref, then clarify which APIs and dynamic-attribute cases are in scope. Done means the rough idea has a settled rule behavior and implementation scope that can be picked up without further design discussion.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, rust
Domain
testing, tooling
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.