feat: "Pin to locked version" code action for all supported registries#129
feat: "Pin to locked version" code action for all supported registries#129rassie wants to merge 11 commits into
Conversation
98e316f to
f70f66b
Compare
f70f66b to
2e5ce71
Compare
|
Thanks! Rebased, waiting on #128. |
2e5ce71 to
a41e1f7
Compare
Add the plumbing for a workspace-aware "Pin to locked version" code action: the LockResolver trait, the LockError variant, a shared TOML [[package]] scanner reused by multiple lock formats, the PackageResolver lock-resolver registration mechanism, and the code-action wiring that translates a resolved locked version into an LSP TextEdit. format_pinned_spec is exhaustive over RegistryType so adding a new registry breaks the build until a pinning contract is decided. The in-file rstest table locks the contract per registry: - Npm / PnpmCatalog / CratesIo / Jsr / GitHubActions: bare version - PyPI: ==<version> (operator embedded in PEP 508 strings) - GoProxy: v<version> (go.mod requires v-prefix), idempotent No concrete LockResolver implementations are registered yet; those follow in per-registry-family commits.
First concrete LockResolver: scans Cargo.lock for the first [[package]] block whose name matches the manifest dependency and returns its version. Walks parent directories from the Cargo.toml to find the lock file (Cargo workspaces typically keep one Cargo.lock at the workspace root). The TOML [[package]] scanner is split out into version::lock::extract_package_version_from_toml_lock so the Python lock formats (uv/poetry/pdm) can reuse it in a follow-up commit.
Adds NpmLockResolver: walks `packages` first (lockfileVersion 2/3 with `node_modules/<name>` keys) and falls back to the legacy `dependencies` map. Wired up as the lone resolver for RegistryType::Npm; future commits will add pnpm and yarn ahead of it in the priority chain.
Adds PnpmLockResolver: parses pnpm-lock.yaml entries, follows catalog references (`'pkg@catalog:'` → resolved versions block), and strips the peer-dep suffix that pnpm appends to deduplicate package keys (`18.2.0(react@18.2.0)` → `18.2.0`). Registered ahead of NpmLockResolver for RegistryType::Npm so a workspace with both lock files pins against pnpm. PnpmLockResolver is also wired up for RegistryType::PnpmCatalog so pnpm catalog entries in pnpm-workspace.yaml can be pinned.
Adds YarnLockResolver: handles both v1 (`version "X"` inside key-quoted blocks) and Berry (`version: X` under YAML-ish entries), multi-spec keys like `"lodash@^4.0, lodash@^4.17"`, scoped packages, and strips Berry's peer-dep dedup suffix (`18.2.0_react@npm:18.2.0` → `18.2.0`). Registered between pnpm and npm for RegistryType::Npm, completing the priority chain pnpm > yarn > npm.
Adds UvLockResolver: reuses the shared `[[package]]` TOML scanner from version::lock, so the implementation is just the trait wiring. Registered as the lone PyPI lock resolver; poetry/pdm/Pipfile follow in subsequent commits.
Adds PoetryLockResolver: poetry.lock uses the same TOML `[[package]]` layout as Cargo and uv, so the implementation reuses `extract_package_version_from_toml_lock` and adds only the trait wiring. Registered after UvLockResolver for RegistryType::PyPI so a workspace with both lock files pins against uv (the more specific package manager). Also pulls in a `boot_pypi_service` test helper shared by the small PyPI lock-format e2e tests (poetry/pdm/Pipfile coming next).
Adds PdmLockResolver: pdm.lock uses the same TOML `[[package]]` layout as Cargo/uv/poetry, so the implementation reuses `extract_package_version_from_toml_lock`. Registered after PoetryLockResolver in the PyPI priority chain: **uv > poetry > pdm**.
Adds PipfileLockResolver: Pipfile.lock is JSON, so the scanner walks the `default` section first and falls back to `develop`. Pipfile.lock stores versions with the operator embedded (`"==1.2.3"`), so the resolver strips the leading `==` before returning the version — the PyPI pin formatter then re-adds the operator via the contract test table. Registered last in the PyPI priority chain: **uv > poetry > pdm > pipfile**.
Adds DenoLockResolver for the JSR ecosystem. deno.lock v3/v4 is JSON with a `specifiers` map keyed by `jsr:<scope>/<name>@<range>`; the resolver finds the entry matching the requested package and strips the peer-dep suffix Deno appends to deduplicate (`1.0.1_other@2.0.0` → `1.0.1`). Completes the lock-pinning coverage for all supported registries. Note: the deno e2e test depends on the JSR parser cursor-positioning fix (skanehira#128) — without it, the cursor lookup misses the version token and the code action is not offered.
Rebase fixup: main introduced a new RegistryType::Docker variant (compose.yaml support, skanehira#70). format_pinned_spec's exhaustive match required a deliberate decision for it. Docker tags are bare strings without an operator prefix, so the locked value is emitted as-is.
a41e1f7 to
a41c6a5
Compare
skanehira
left a comment
There was a problem hiding this comment.
Two design concerns around lock-file scoping — please see inline.
| /// (e.g. `(react@18.0.0)`) stripped. | ||
| fn extract_locked_version(yaml: &Value, package_name: &str) -> Option<String> { | ||
| if let Some(importers) = yaml.get("importers").and_then(Value::as_mapping) { | ||
| for (_, importer) in importers { |
There was a problem hiding this comment.
This iterates every importer and returns the first match, but the workspace path is discarded. In a pnpm monorepo where the same package is pinned to different versions across workspaces, this returns the wrong version for any manifest that isn't the first one in iteration order.
Repro
repo/
├── pnpm-lock.yaml
└── packages/
├── app-old/package.json # "react": "^17.0.0"
└── app-new/package.json # "react": "^18.0.0"
# pnpm-lock.yaml
importers:
packages/app-old:
dependencies:
react: { specifier: ^17.0.0, version: 17.0.2 }
packages/app-new:
dependencies:
react: { specifier: ^18.0.0, version: 18.2.0 }Requesting the code action on react inside packages/app-new/package.json currently surfaces Pin to locked version: 17.0.2 (whichever importer iterates first), not 18.2.0.
Suggestion
Compute the manifest's path relative to the lock file's directory and look up that importer first:
let lock_dir = lock_path.parent();
let importer_key = manifest_path
.parent()
.and_then(|p| p.strip_prefix(lock_dir).ok())
.map(|p| if p.as_os_str().is_empty() { ".".into() } else { p.to_string_lossy().replace('\\', "/") });Then try importers[importer_key] first. Falling back to other importers is debatable — being strict (return None when the matching importer doesn't list the package) is probably safer because a fallback can again surface a wrong version. Either way, the workspace path needs to participate in the lookup.
| }; | ||
|
|
||
| let mut dir = start.to_path_buf(); | ||
| loop { |
There was a problem hiding this comment.
The walk continues all the way to / (or C:\\). If a developer keeps multiple unrelated projects under a shared parent that happens to contain a stray lock file, we'll resolve against it.
Repro
~/work/
├── Cargo.lock # leftover from an old experiment
└── unrelated-project/
└── Cargo.toml # no Cargo.lock of its own
Opening unrelated-project/Cargo.toml makes find_lock_file walk past the project root and pick up ~/work/Cargo.lock, producing pin suggestions for crates the project doesn't even use.
It's also a non-trivial amount of stat syscalls on deep paths every code-action request, which compounds with the per-resolver loop noted elsewhere.
Suggestion
Stop the walk at the project boundary. Two reasonable signals (in priority order):
.gitdirectory — the most reliable VCS-root marker; covers the overwhelming majority of repos.- Workspace root sentinel — e.g. presence of
pnpm-workspace.yaml,Cargo.tomlwith[workspace], etc.
Something like:
loop {
let candidate = dir.join(lock_filename);
if candidate.is_file() {
return Ok(Some(candidate));
}
if dir.join(".git").exists() {
// VCS root reached; do not escape the repo.
return Ok(None);
}
if !dir.pop() {
return Ok(None);
}
}.git exists as either a directory (normal repo) or a file (worktree / submodule), so .exists() is the right check. If we want to support repos without .git (rare in practice), we can add a hard cap (e.g. 16 ancestors) as a backstop.
Closes #121.
Summary
Adds a "Pin to locked version" code action that pins a dependency in the manifest to the version actually resolved in the workspace's lock file.
Supported lock files (one resolver per format, registered per registry):
Cargo.lockpnpm-lock.yaml>yarn.lock(v1 + Berry) >package-lock.jsonuv.lock>poetry.lock>pdm.lock>Pipfile.lockdeno.lock(v3 + v4)pnpm-lock.yamlWhen multiple lock files coexist in a workspace, the more specific package manager wins — a repo with both
pnpm-lock.yamlandpackage-lock.jsonpins against pnpm.Design notes
LockResolvertrait (src/version/lock.rs) is registered perPackageResolverand runs only on a code-action request — it does not participate in the diagnostics pipeline.format_pinned_spec(src/lsp/code_action/lock.rs) is exhaustive overRegistryType, so adding a new registry breaks the build until its pinning contract is decided. The case table locks the contract per registry: bare version for npm / pnpm-catalog / crates / JSR / GitHub Actions / Docker,==<v>for PyPI (PEP 508 embeds the operator),v<v>for Go (idempotent on already-prefixed input).[[package]]TOML scanner (extract_package_version_from_toml_lock)."lodash@^4, lodash@^4.17"), and strips Berry's peer-dep dedup suffix (18.2.0_react@npm:18.2.0→18.2.0).Pipfile.lockstores"==1.2.3"— the resolver returns bare1.2.3so PyPI'sformat_pinned_specre-adds the operator centrally.specifierskeys, scans both lock v3 and v4 formats.The branch is split into 11 commits — one per lock format plus scaffolding and a rebase fixup for the new
RegistryType::Dockervariant introduced onmain— so each can be reviewed in isolation.Test plan
cargo test— 14 new e2e tests intests/e2e_lock_pin.rs(one per format + omit-cases + priority tests) plus unit tests in eachlocks/*.rscargo fmt -- --checkcargo clippy --all-targetsuv.lockandpackage-lock.json; the other formats are covered by e2e tests but not yet by manual editor testing