feat: rename pytest.mark.parametrize parameters (#165)#167
Merged
Conversation
Add an LSP rename provider for parametrized test parameters. Triggering rename from the parametrize decorator string, the test signature parameter, or a body usage rewrites all three together. Handles every argname form (single, comma string, list, tuple, argnames= keyword) and stacked decorators. Indirect (indirect=True) params and plain fixtures are declined so a general Python language server handles them.
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 08219c8e63
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #167 +/- ##
==========================================
+ Coverage 80.99% 81.95% +0.96%
==========================================
Files 28 29 +1
Lines 4363 4684 +321
==========================================
+ Hits 3534 3839 +305
- Misses 829 845 +16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address review findings: - Locate argname tokens from source text instead of assuming a single opening quote, so prefixed (r/b/u/f) and triple-quoted argname strings get correct ranges; reject non-identifier names (implicit concatenation) rather than emit a corrupting edit. - Detect indirect params for keyword argnames=, list/tuple argnames, and positional indirect, so those are declined instead of locally renamed. - Make body-usage collection scope-aware: nested function/lambda params and comprehension targets that shadow the name are no longer renamed, while enclosing-scope refs (defaults, annotations, first comprehension iterable, closures) still are.
… rustpython-ast license - Replace extract_word_at_position (char-index, end-exclusive) with a byte-offset, end-inclusive identifier lookup so rename resolves when the caret rests just past the parameter and on non-ASCII lines (Copilot). - Add tests covering nested scopes (all comprehension forms, lambda, nested sync/async functions, *args/**kwargs, unpacking targets), closures and enclosing-scope refs (defaults, annotations, return annotation, comp ifs), caret-at-end, decline paths (module level, syntax error, unanalyzed file), and decorator argname/indirect edge cases. - Exempt rustpython-ast from the dependency-review license check; it is MIT but GitHub's dependency graph reports it as unknown.
Apply review polish: skip non-whole-word matches in the position_of test helper so a future fixture edit can't silently trigger on the wrong token, assert a superset identifier (foobar) is left untouched, and note the global/nonlocal scope gap in the collector doc comment.
Address Copilot review: - Select the innermost *parametrized* function containing the cursor, so a rename triggered from inside a nested closure that references the parameter resolves to the enclosing parametrized test and still rewrites the decorator string, instead of declining because the closure has no parametrize decorator. - Make is_plain_identifier ASCII-only to match identifier_at and is_valid_python_identifier, so Unicode argnames are handled consistently (declined) rather than half-supported.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #165.
Renaming a parametrized parameter now updates the name in the
@pytest.mark.parametrize(...)decorator string, the test signature, and the test body in one edit. The rename works from any of those three spots, matching what PyCharm does. There was no rename provider before this.Scope and design
This server runs alongside a general Python LSP, so the design avoids stepping on it:
prepareRename/renamereturn nothing, so pyright/pylsp answers instead.indirect=Trueparameters are skipped. They route to a fixture, and a local-only rename would quietly break the test.What it handles
All argname forms: a single name, a comma string (
"a,b"and"a, b"), a list, a tuple, and theargnames=keyword. Stacked parametrize decorators work too. Attribute access (obj.foo), string literals, and keyword-arg names in the body are left alone since they aren't references to the parameter.Body usages are collected with the generated
rustpython-astVisitor(enabled via thevisitorfeature) rather than a hand-rolled walk, so no node type slips through and leaves a stale reference.Known gap: a nested scope inside the test body that shadows the parameter name will be over-collected. It doesn't happen in practice.
Tests
tests/test_decorators.rs: argname extraction for every form, asserting the ranges land on the identifier and not the quotes or whitespace.tests/test_lsp.rs: rename across all forms and all three trigger sites, stacked decorators, plus the guards (indirect, fixture, invalid identifier, prepareRename).tests/test_project/test_parametrize.py: sample file for the end-to-end scans.