impl pytest add type #2201
impl pytest add type #2201
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements code actions to automatically add type annotations for pytest fixture parameters in test functions, as well as return type annotations for pytest fixture functions themselves.
Changes:
- Added new module
pytest_fixture.rsimplementing logic to detect pytest fixtures and test functions, resolve fixtures from conftest files, and generate type annotation code actions - Integrated the new functionality into the LSP server to offer code actions when cursor is on fixture parameters or fixture functions
- Added test coverage for the new feature with conftest fixture resolution
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pyrefly/lib/state/lsp/quick_fixes/pytest_fixture.rs | Core implementation of pytest fixture type annotation logic including fixture detection, conftest resolution, and code action generation |
| pyrefly/lib/state/lsp/quick_fixes/mod.rs | Registers the new pytest_fixture module |
| pyrefly/lib/state/lsp.rs | Adds Transaction method to expose pytest fixture type annotation code actions |
| pyrefly/lib/lsp/non_wasm/server.rs | Integrates pytest fixture code actions into the LSP server's code action handler |
| pyrefly/lib/test/lsp/code_actions.rs | Adds test case for pytest fixture type annotations and updates extract actions helper to include pytest stub |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if candidates.is_empty() { | ||
| return None; | ||
| } | ||
|
|
There was a problem hiding this comment.
The function returns None when no fixture return type annotation candidates are found, but this prevents the code from checking for test function parameter annotations. This means if a user's cursor is on a test function parameter with no fixture functions needing annotations, no actions will be offered. The early return on line 350 should be removed, and the logic should continue to check for parameter annotation candidates.
| if candidates.is_empty() { | |
| return None; | |
| } |
| fn collect_fixture_functions<'a>(stmts: &'a [Stmt], out: &mut Vec<&'a StmtFunctionDef>) { | ||
| for stmt in stmts { | ||
| match stmt { | ||
| Stmt::FunctionDef(func) => out.push(func), | ||
| Stmt::ClassDef(class_def) => collect_fixture_functions(&class_def.body, out), | ||
| _ => {} | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The function collect_fixture_functions collects all function definitions, not just pytest fixtures. This means it will include non-fixture functions as well. The function should be renamed to collect_all_functions or the logic should be modified to only collect functions within the scope of fixture detection (the filtering is currently done outside this function).
| import_edits | ||
| } | ||
|
|
||
| /// Builds code actions that add inferred return annotations to pytest fixtures. |
There was a problem hiding this comment.
The comment on line 301 states "Builds code actions that add inferred return annotations to pytest fixtures" but the function also handles parameter type annotations for test functions. The comment should be updated to reflect both responsibilities.
| /// Builds code actions that add inferred return annotations to pytest fixtures. | |
| /// Builds code actions that add inferred type annotations for pytest fixtures and test functions. |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
1 participant
Summary
Fixes part of #1589
The code action now adds type annotations for fixture parameters only in test functions, resolving fixtures first in the current module then upward to conftest files (closest wins).
Haven't implemented full pytest fixture resolution outside conftest but the current approach covers typical use.
But problem: should LSP just infer the version as inlay hint, then click to insert?
relevant DetachHead/basedpyright#311
Test Plan
pytest_fixture_type_annotation_code_actions