Add completion for request by benediktziegler · Pull Request #129 · bellini666/pytest-language-server

benediktziegler · 2026-03-26T13:31:27Z

This PR adds the request fixture manually to the FixtureDatabase.

Fixes #128

Merge #126 first.

codecov · 2026-03-26T13:37:51Z

Codecov Report

❌ Patch coverage is 85.71429% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.16%. Comparing base (cb86f3d) to head (b0eb009).

Files with missing lines	Patch %	Lines
src/fixtures/scanner.rs	84.21%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #129      +/-   ##
==========================================
+ Coverage   77.08%   77.16%   +0.07%     
==========================================
  Files          28       28              
  Lines        4338     4357      +19     
==========================================
+ Hits         3344     3362      +18     
- Misses        994      995       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copilot

Pull request overview

This PR special-cases pytest’s built-in request fixture by synthesizing a FixtureDefinition in the FixtureDatabase, so it can participate in completion/hover/inlay hints/code actions even though it isn’t declared via @pytest.fixture. It also includes broader LSP/code-action and analysis infrastructure changes (matching what’s described in PR #126).

Changes:

Register a synthetic request fixture during pytest internal/venv scanning, including FixtureRequest return type + import spec.
Add return-type import resolution plumbing (TypeImportSpec, import-map building, type-alias expansion) to support typed code actions and isort-aware import insertion.
Extract LanguageServer for Backend into a shared module for integration testing, add extensive tests/docs, and gate forced-exit shutdown behavior behind a feature.

Reviewed changes

Copilot reviewed 22 out of 24 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/fixtures/scanner.rs`	Registers synthetic `request` fixture after scanning `_pytest` internals.
`src/fixtures/analyzer.rs`	Adds return-type import resolution + `is_parameter` tracking for usages.
`src/fixtures/types.rs`	Introduces `TypeImportSpec` and extends `FixtureDefinition`/`FixtureUsage`.
`src/fixtures/imports.rs`	Adds free `is_stdlib_module()` and `build_name_to_import_map()` support.
`src/providers/code_action.rs`	Adds new code-action kinds + import insertion/adaptation logic.
`src/providers/inlay_hint.rs`	Skips non-parameter usages for type inlay hints.
`src/providers/language_server.rs` / `src/providers/mod.rs` / `src/main.rs` / `src/lib.rs`	Moves LanguageServer impl into providers for lib+bin builds and testing.
`tests/test_fixtures.rs`	Adds request-specific tests and `is_parameter` behavior tests.
`tests/test_language_server.rs`	Adds integration coverage for `LanguageServer for Backend` methods.
`Cargo.toml` / `pyproject.toml` / `.github/workflows/ci.yml`	Adds `lsp_force_exit` feature and updates build steps.
`README.md` + extension READMEs + `CLAUDE.md`	Documents expanded code-action capabilities and architecture.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T11:12:35Z

Cargo.toml

 [[bin]]
 name = "pytest-language-server"
 path = "src/main.rs"
+required-features = ["lsp_force_exit"]


required-features = ["lsp_force_exit"] on the [[bin]] target means cargo build / cargo install will no longer produce the pytest-language-server binary unless users explicitly pass --features lsp_force_exit. If the goal is only to avoid process::exit() when used as a library/test dependency, consider an approach that keeps the binary buildable by default (e.g. move the forced-exit behavior into main.rs, or gate it via a runtime flag/env var), so downstream tooling doesn’t unexpectedly lose the binary.

Copilot · 2026-03-28T11:12:36Z

src/fixtures/scanner.rs

+        // `request` is not defined via @pytest.fixture anywhere in _pytest/ —
+        // pytest injects it programmatically via FixtureManager.  Register a
+        // synthetic definition so that hover, inlay hints, completion and code
+        // actions all know its type.
+        self.register_request_builtin_fixture(&pytest_internal);
+    }


PR description says this change only special-cases the request fixture, but this PR also includes large unrelated changes (e.g. LanguageServer impl extraction, new code-action kinds/import insertion, binary feature gating). If those are intended to come from #126, it may be better to rebase onto/merge that PR first (as noted in the description) or split this PR so the request fix can be reviewed/landed independently.

Copilot · 2026-03-28T11:12:36Z

README.md

- **Code actions** to automatically add missing fixture parameters
+- **Code actions** to automatically add missing fixture parameters with type annotations
 - Intelligent parameter insertion (handles both empty and existing parameter lists)
 - Works with both single-line and multi-line function signatures


This documentation claims diagnostics/quick-fix insertion works for multi-line function signatures, but the current quick-fix implementation in handle_code_action still relies on finding "):" on a single line and won’t handle multi-line signatures. Either adjust the implementation to support multi-line signatures, or soften this statement so docs match actual behavior.

Suggested change

- Works with both single-line and multi-line function signatures

- Supports single-line function signatures (multi-line support may be limited)

Copilot · 2026-03-28T11:12:36Z

src/providers/code_action.rs

+                let Some(func_line_content) = lines.get(function_line as usize) else {
+                    warn!(
+                        "Function line {} is out of range in {:?}",
+                        function_line, file_path
+                    );
+                    continue;
+                };
+
+                // Locate the closing `):` of the function signature.
+                let Some(paren_pos) = func_line_content.find("):") else {
+                    continue;
+                };
+
+                if !func_line_content[..paren_pos].contains('(') {
+                    continue;
+                }
+
+                let param_start = match func_line_content.find('(') {
+                    Some(pos) => pos + 1,
+                    None => {
+                        warn!(
+                            "Invalid function signature at {:?}:{}",
+                            file_path, function_line
                        );
+                        continue;
+                    }
+                };
+
+                let params_section = &func_line_content[param_start..paren_pos];
+                let has_params = !params_section.trim().is_empty();
+


The quick-fix parameter insertion logic here only works when the entire function signature (including the closing "):") is on the same line as fixture.function_line. For multi-line signatures this will fail to offer/apply the quick fix. Consider reusing FixtureDatabase::get_function_param_insertion_info() (already used by completion auto-add) so code actions support multi-line signatures consistently and avoid duplicating signature-parsing logic.

Suggested change

let Some(func_line_content) = lines.get(function_line as usize) else {

warn!(

"Function line {} is out of range in {:?}",

function_line, file_path

);

continue;

};

// Locate the closing `):` of the function signature.

let Some(paren_pos) = func_line_content.find("):") else {

continue;

};

if !func_line_content[..paren_pos].contains('(') {

continue;

}

let param_start = match func_line_content.find('(') {

Some(pos) => pos + 1,

None => {

warn!(

"Invalid function signature at {:?}:{}",

file_path, function_line

);

continue;

}

};

let params_section = &func_line_content[param_start..paren_pos];

let has_params = !params_section.trim().is_empty();

// Use shared helper logic (also used by completion auto-add) so we

// correctly handle multi-line function signatures and avoid

// duplicating parsing logic here.

let Some(insertion_info) = backend

.fixture_db()

.get_function_param_insertion_info(&file_path, fixture.function_line)

else {

warn!(

"Could not determine parameter insertion info for fixture at {:?}:{}",

file_path, function_line

);

continue;

};

// Map helper result into the local variables expected by the rest

// of this function.

let param_start = insertion_info.param_start_col as usize;

let paren_pos = insertion_info.insertion_col as usize;

let has_params = insertion_info.has_existing_params;

bellini666 · 2026-04-02T16:08:26Z

@benediktziegler I merged the previous MR, can you rebase this one so I can review it (and also trigger copilot to do the same)

I'll release both together when this gets merged

pytest's `request` fixture is injected programmatically by `FixtureManager` and never appears as a `@pytest.fixture`-decorated function, so the AST scanner cannot discover it. A synthetic `FixtureDefinition` is now registered at venv-scan time, enabling hover, inlay hints, completions, go-to-definition, and code actions to work with it correctly.

benediktziegler · 2026-04-02T20:14:06Z

done

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-03T09:35:39Z

src/fixtures/scanner.rs

+        // Guard: skip registration if an identical synthetic definition
+        // (same file_path) is already present.  This prevents duplicate
+        // entries when scan_venv_fixtures / scan_pytest_internal_fixtures
+        // is called more than once (e.g. on workspace reload).
+        if let Some(existing) = self.definitions.get("request") {
+            if existing.iter().any(|d| d.file_path == file_path) {
+                debug!(
+                    "Synthetic 'request' fixture already registered for {:?}, skipping",
+                    file_path
+                );
+                return;
+            }
+        }


The duplicate-prevention guard only skips when an existing request definition has the exact same file_path. If a first scan registers the sentinel path (because fixtures.py is missing) and a later scan finds fixtures.py, this will register a second synthetic request definition and both will be returned in completion/hover resolution. Consider replacing any prior synthetic request definitions (or de-duplicating by name + is_plugin/is_third_party) rather than keying solely on file_path.

Copilot · 2026-04-03T09:35:40Z

src/fixtures/analyzer.rs

            self.record_fixture_definition(definition);

            // Record each dependency as a usage
            for arg in Self::all_args(args) {
                let arg_name = arg.def.arg.as_str();



This block is now recording all parameters (including request) as usages, not just dependencies. The leading comment still says “Record each dependency as a usage”, which is misleading after the change; please update it to reflect the actual behavior to avoid future regressions.

Copilot · 2026-04-03T09:35:40Z

src/fixtures/analyzer.rs

            for arg in Self::all_args(args) {
                let arg_name = arg.def.arg.as_str();

-                if arg_name != "self" && arg_name != "request" {
+                // `request` is excluded from *dependencies* (it is a special pytest
+                // injection, not a regular fixture), but we DO record it as a usage
+                // so that inlay hints and type-annotation code actions work on it.
+                if arg_name != "self" {


After widening the condition to include request, the surrounding wording/logging still frames these as “dependencies” (e.g. the loop comment and the later info!("Found fixture dependency: ...")). Since request is explicitly excluded from the dependencies list above, consider renaming the comment/log to “parameter usage” (or only logging as dependency when arg_name != "request") to keep behavior and diagnostics consistent.

Copilot · 2026-04-03T09:35:40Z

tests/test_fixtures.rs

+#[timeout(30000)]
+fn test_request_completion_available() {
+    // `request` must appear in the list of available fixtures for a test file
+    // once the venv has been scanned (simulated here by direct registration).


The test comment says venv scanning is “simulated here by direct registration”, but this test actually creates a .venv/.../site-packages/_pytest tree and calls scan_workspace. Please adjust the comment to match what the test is doing to reduce confusion during future maintenance.

Suggested change

// once the venv has been scanned (simulated here by direct registration).

// once `scan_workspace` discovers the venv's `_pytest` package.

bellini666 · 2026-04-03T10:20:30Z

@benediktziegler some remarks from copilot 🙏🏼

bellini666 requested a review from Copilot March 28, 2026 11:07

Copilot started reviewing on behalf of bellini666 March 28, 2026 11:07 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

benediktziegler force-pushed the add-completion-for-request branch 2 times, most recently from 8cd1ec2 to 0ad2b90 Compare March 31, 2026 14:32

benediktziegler closed this Apr 2, 2026

benediktziegler force-pushed the add-completion-for-request branch from 0ad2b90 to cb86f3d Compare April 2, 2026 19:44

benediktziegler added 2 commits April 2, 2026 21:55

test: fix code action kind typo to source.fixAll.pytest-ls

b0eb009

benediktziegler reopened this Apr 2, 2026

Copilot AI mentioned this pull request Apr 2, 2026

feat: first-class support for pytest's built-in request fixture benediktziegler/pytest-language-server#4

Draft

bellini666 requested a review from Copilot April 3, 2026 09:30

Copilot started reviewing on behalf of bellini666 April 3, 2026 09:31 View session

Copilot AI reviewed Apr 3, 2026

View reviewed changes

	- Works with both single-line and multi-line function signatures
	- Supports single-line function signatures (multi-line support may be limited)

	// once the venv has been scanned (simulated here by direct registration).
	// once `scan_workspace` discovers the venv's `_pytest` package.

Conversation

benediktziegler commented Mar 26, 2026

Uh oh!

codecov bot commented Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

bellini666 commented Apr 2, 2026

Uh oh!

benediktziegler commented Apr 2, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 3, 2026

Choose a reason for hiding this comment

Uh oh!

bellini666 commented Apr 3, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Mar 26, 2026 •

edited

Loading