From 8da04df18d9268abe0239a13dfd0beb4e7977d4d Mon Sep 17 00:00:00 2001 From: owa <20108042+owayo@users.noreply.github.com> Date: Fri, 9 Oct 2026 12:26:09 +0900 Subject: [PATCH] =?UTF-8?q?fix:=20Cargo=20=E3=81=AE=E5=90=8C=E3=81=98?= =?UTF-8?q?=E5=8F=96=E5=BE=97=E5=85=83=E3=83=BB=E5=90=8C=E3=81=98=E7=89=88?= =?UTF-8?q?=E3=81=AE=E5=AE=A3=E8=A8=80=E3=82=92=E3=81=BE=E3=81=A8=E3=82=81?= =?UTF-8?q?=E3=81=A6=E6=9B=B8=E3=81=8D=E6=8F=9B=E3=81=88=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 同じ依存を [dependencies] と [dev-dependencies] に同じ取得元・同じ版で書くと (dev 側にだけ features を足す、Rust でよくある書き方)、writer の曖昧判定で 更新を拒否していた。「2 updates」と表示しながら Cargo.toml を 1 バイトも 書けず、exit code 2 になる。更新の行とエラーも宣言の数だけ並んでいた。 - Cargo の parse で、実パッケージ名・依存キー・版の指定の生表記・git の取得元 (URL・参照・公開用に併記した version) が一致する宣言を 1 依存にまとめる。 update_version / update_git_tag は全依存セクションの同名の宣言を書き換える ので、1 回の書き換えで全宣言が新しい版になる。is_dev は、本番の宣言が 1 つでもあれば本番にする - Cargo.toml を書き換えない git の branch / rev / 既定ブランチの更新には、 曖昧判定を当てない - 版や取得元が食い違って拒否するときも、エラーは依存キーごとに 1 行にする Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 3 +- docs/ecosystems.ja.md | 2 + docs/ecosystems.md | 2 + docs/usage.ja.md | 2 +- docs/usage.md | 2 +- src/domain/git_source.rs | 2 +- src/manifest/cargo_toml.rs | 197 +++++++++++++++++++++++++++++--- src/manifest/writer.rs | 223 ++++++++++++++++++++++++++++++++----- 8 files changed, 386 insertions(+), 47 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index bafd274..10f3a21 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -255,7 +255,8 @@ make ci # CI と同じ検査 (fmt-check + lint + test、書き換 - OSV フォールバックの警告 (`X vulnerable, falling back`) は設計どおりの正常動作の通知であり、exit code には影響しない (exit code 2 は OSV 警告以外のエラーがある場合のみ) - npm alias 依存 (`"react": "npm:@preact/compat@^17"`) は実パッケージ名 (`@preact/compat`) でレジストリ照会し、書き戻しには JSON キー (alias 名) を使う (Cargo の rename 依存と同じ name / manifest_name パターン)。alias 接頭辞 `npm:@` は更新後も保持される - Bun Catalogs は root `package.json` のトップレベル `catalog` / `catalogs` と `workspaces.catalog` / `workspaces.catalogs` を解析・更新する。workspace package 側の `catalog:` / `catalog:` 参照は、参照先 catalog 定義で一元管理されるため書き換えず保持する。複数の catalog object を更新するときは、JSON 内の出現順に範囲をソートしてから後方順に置換し、先行置換による byte offset のずれで後続 catalog を壊さない。pnpm の `pnpm-workspace.yaml` catalogs は現時点では未対応で、`package.json` 内の `catalog:` 参照は安全側でスキップする -- 同じマニフェストキーの依存が複数箇所に宣言されている場合 (Cargo.toml の `[dependencies]` + `[dev-dependencies]`、package.json の dependencies + devDependencies、pyproject の main + dev group、Gemfile の通常宣言 + `group :test` 等)、名前だけでは更新位置を一意に決められないため writer は曖昧な更新を拒否する。複数の Gradle 依存が同じバージョン変数または version catalog の `version.ref` を共有する場合も last-write-wins を避けて拒否し、スキップ・固定された別宣言を暗黙に書き換えない +- 同じマニフェストキーの依存が複数箇所に宣言されている場合 (Cargo.toml の `[dependencies]` + `[dev-dependencies]`、package.json の dependencies + devDependencies、pyproject の main + dev group、Gemfile の通常宣言 + `group :test` 等)、名前だけでは更新位置を一意に決められないため writer は曖昧な更新を拒否する。複数の Gradle 依存が同じバージョン変数または version catalog の `version.ref` を共有する場合も last-write-wins を避けて拒否し、スキップ・固定された別宣言を暗黙に書き換えない。拒否のエラーは、同じ依存キーの宣言が別々の更新になっても依存キーごとに 1 行だけ出す。git 依存の branch / rev / 既定ブランチの更新は Cargo.toml を書き換えないので、曖昧判定より前に抜けて拒否しない +- Cargo.toml では、取得元と版の指定が一致する宣言を parse の段階で 1 依存にまとめる (`CargoDeclarations`。Gradle の `push_unique_declaration` と同じ考え方)。dev 側にだけ `features` を足して同じ依存を `[dependencies]` と `[dev-dependencies]` に書く構成は Rust でよくあり、宣言ごとに 1 依存を返すと writer の曖昧性ガードに必ず掛かって「2 updates」と表示しながら 1 バイトも書けず exit code 2 になっていた (更新の行とエラーも宣言の数だけ並んでいた)。`update_version` / `update_git_tag` は全依存セクションの同名の宣言をすべて書き換えるので、まとめた 1 依存の 1 回の書き換えで全宣言が新しい版になる。キーは実パッケージ名・依存キー・版の指定の生表記・git の取得元 (URL・参照・公開用に併記した `version`)。生表記で比べるので `"1.0"` と `"^1.0"` はまとめない (writer が拒否する)。`features` などの付帯指定と `is_dev` はキーに含めず、`is_dev` はどれか 1 つでも本番の宣言があれば本番にする (`[dev-dependencies]` が target 固有の `[dependencies]` より先に読まれるため、最初の宣言を残すだけだと本番の依存が dev 扱いになる) - Cargo.toml の複数行テーブル (`[dependencies.]`) はセクション追跡の行ベースで更新され、`features = [...]` が `version` より前にあるキー順でも、テーブル内コメントに `version = "..."` 文字列があっても正しく動く。`update_git_tag` は inline table と複数行 `tag` の両方で同方式を使い、依存/patch セクション外の同名キーを触らず、単一引用符も保持する - Package.swift の URL マッチは末尾境界付き (`grpc/grpc-swift` の更新が `grpc/grpc-swift-nio` の宣言に前方一致しない) - Composer は `~>` を拒否する (composer/semver の tilde 分岐は正規表現 `{^~>?$}i` にマッチした**後**に `Invalid operator "~>"` を throw する) ため、PHP パーサは `~>` を parse 時点でスキップする。受理すると「更新しました」と報告しつつ composer が読めない制約を書き戻すことになる。node-semver は `LONETILDE = (?:~>?)` で `~>` を valid とするので Node パーサは従来どおり受理し、更新後も綴りを保つ (言語間で挙動が違うのは仕様差によるもの) diff --git a/docs/ecosystems.ja.md b/docs/ecosystems.ja.md index f8fb8b6..f798e2b 100644 --- a/docs/ecosystems.ja.md +++ b/docs/ecosystems.ja.md @@ -94,6 +94,8 @@ PEP 440 のプレリリースは、区切り文字なしで書かれた場合（ `alias = { package = "actual-crate", version = "1" }` のような Cargo のリネーム依存は、実際のパッケージ名でバージョンを取得し、マニフェスト上のキーへ書き戻します。`--only` / `--exclude` はどちらの名前でも指定できます。 +テストでだけ `features` を足すために、同じ依存を `[dependencies]` と `[dev-dependencies]` の両方に書くことがよくあります。こうした宣言が同じ取得元（同じ表記の crates.io のバージョン指定、または同じ git の URL と参照）を指すなら、depup は 1 つの依存として扱います。問い合わせは 1 回、表示する更新は 1 件で、すべての宣言を書き換えます。バージョン指定は書かれた表記のまま比べるので、`"1.0"` と `"^1.0"` は別物として扱います。バージョン指定、git の URL や参照、`git` と併記した `version`、`package` で指すクレートのどれかが食い違う宣言はまとめず、その更新は曖昧として拒否します。 + path 依存（`{ path = "../common" }`）は、公開用に `version` を併記していても対象外です。ローカルのクレートで解決されるためです。crates.io 以外のレジストリを指す依存（`crates-io` 以外の `registry = "..."`、または `registry-index = "..."`）も、depup が crates.io しか問い合わせないため対象外です。 git 依存は、レジストリの代わりに `git ls-remote` で確認します。 diff --git a/docs/ecosystems.md b/docs/ecosystems.md index 6bfe2ae..ed6967d 100644 --- a/docs/ecosystems.md +++ b/docs/ecosystems.md @@ -94,6 +94,8 @@ In `Cargo.toml`, dependency updates are limited to dependency tables such as `[d Cargo renamed dependencies such as `alias = { package = "actual-crate", version = "1" }` are fetched by the real package name and written back through the manifest key. `--only` and `--exclude` accept either name. +A dependency is often declared in `[dependencies]` and again in `[dev-dependencies]` to add `features` for tests. When such declarations point to the same source — the same crates.io version requirement written the same way, or the same git URL and reference — depup treats them as one dependency: it looks the dependency up once, shows one update, and rewrites every declaration. The version requirement is compared as written, so `"1.0"` and `"^1.0"` count as different. Declarations that differ in version requirement, git URL or reference, the `version` written beside `git`, or the `package` they point to are not combined, and an update to them is refused as ambiguous. + Path dependencies (`{ path = "../common" }`) are not processed, even when they also declare a `version` for publishing, because they resolve to the local crate. Dependencies that point at a registry other than crates.io — a non-`crates-io` `registry = "..."` or any `registry-index = "..."` — are not processed either, because depup only queries crates.io. Git dependencies are checked with `git ls-remote` instead of a registry: diff --git a/docs/usage.ja.md b/docs/usage.ja.md index 89157c6..4437bff 100644 --- a/docs/usage.ja.md +++ b/docs/usage.ja.md @@ -325,7 +325,7 @@ npm には `!=` という比較演算子がないため、npm の制約に `!=` depup が書き換えるのは、解析した依存宣言だけです。依存宣言以外の場所は、パッケージ名やバージョンが書かれていても変更しません。たとえば `package.json` の `overrides`、`composer.json` の `replace` / `provide` / `conflict`、`Cargo.toml` や `pyproject.toml` のメタデータのテーブルがこれにあたります（対象のセクションは[エコシステム別の詳細](ecosystems.ja.md#エコシステム別の詳細)を参照）。値を書き換えるときは、周囲の書き方も保ちます。TOML マニフェストでは、基本文字列（`"..."`）とリテラル文字列（`'...'`）のどちらも、引用符の種類を変えません。 -書き込み先が曖昧なとき、depup は書き込みを拒否し、エラーとして報告します（終了コードは 2）。同じ依存キーがマニフェスト内で複数回宣言されている場合と、複数の Gradle 依存が 1 つのバージョン変数や version catalog の `version.ref` を共有している場合がこれにあたります。固定指定の宣言のように変えてはいけない宣言を、巻き添えで書き換えないためです。 +書き込み先が曖昧なとき、depup は書き込みを拒否し、エラーとして報告します（終了コードは 2）。同じ依存キーがマニフェスト内で複数回宣言されている場合と、複数の Gradle 依存が 1 つのバージョン変数や version catalog の `version.ref` を共有している場合がこれにあたります。固定指定の宣言のように変えてはいけない宣言を、巻き添えで書き換えないためです。複数の宣言がそれぞれ更新になっても、エラーは依存ごとに 1 行だけ出します。`Cargo.toml` では、同じ取得元を指し、バージョンを同じ表記で書いた宣言は 1 つの依存として扱い、まとめて書き換えるので拒否しません（[Rust](ecosystems.ja.md#rust) を参照）。ブランチを追う Cargo の git 依存のように、マニフェストを書き換えない更新は拒否しません。 ## パッケージマネージャーの実行（`--install`） diff --git a/docs/usage.md b/docs/usage.md index 951110f..6420ec1 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -324,7 +324,7 @@ Prereleases (alpha, beta, rc, canary, dev, and similar) are removed from the can depup only rewrites the dependency declarations it parsed. Other sections of a manifest are left untouched even when they contain package names and versions — for example `overrides` in `package.json`, `replace` / `provide` / `conflict` in `composer.json`, and metadata tables in `Cargo.toml` or `pyproject.toml` ([Ecosystem Details](ecosystems.md#ecosystem-details) lists the exact sections). When a value is rewritten, the surrounding syntax is kept; in TOML manifests, both basic strings (`"..."`) and literal strings (`'...'`) keep their quote style. -If the same dependency key is declared more than once in a manifest, or several dependencies share one Gradle version variable or version-catalog `version.ref`, depup cannot tell which declaration to change. It refuses the write and reports an error (exit code 2) rather than silently changing a declaration that should stay as it is, such as a pinned one. +If the same dependency key is declared more than once in a manifest, or several dependencies share one Gradle version variable or version-catalog `version.ref`, depup cannot tell which declaration to change. It refuses the write and reports an error (exit code 2) rather than silently changing a declaration that should stay as it is, such as a pinned one. The error is reported once per dependency, even when several of its declarations had an update. In `Cargo.toml`, declarations that point to the same source with an identically written version count as one dependency and are rewritten together, so they are not refused (see [Rust](ecosystems.md#rust)). Updates that leave the manifest as written, such as a Cargo git dependency that follows a branch, are never refused. ## Running the Package Manager (`--install`) diff --git a/src/domain/git_source.rs b/src/domain/git_source.rs index 356d4ea..82bb3ad 100644 --- a/src/domain/git_source.rs +++ b/src/domain/git_source.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use std::fmt; /// Git 依存が参照する対象種別 -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize, Deserialize)] #[serde(tag = "type", content = "value", rename_all = "snake_case")] pub enum GitReference { /// ブランチ指定 (例: `branch = "main"`) diff --git a/src/manifest/cargo_toml.rs b/src/manifest/cargo_toml.rs index b3f8ee3..b1a8568 100644 --- a/src/manifest/cargo_toml.rs +++ b/src/manifest/cargo_toml.rs @@ -210,7 +210,7 @@ impl ManifestParser for CargoTomlParser { } })?; - let mut dependencies = Vec::new(); + let mut dependencies = CargoDeclarations::default(); let parser = get_parser(Language::Rust); // 通常の依存関係を読む @@ -256,7 +256,7 @@ impl ManifestParser for CargoTomlParser { parse_cargo_dependencies(deps, parser.as_ref(), false, &mut dependencies); } - Ok(dependencies) + Ok(dependencies.dependencies) } fn language(&self) -> Language { @@ -522,14 +522,14 @@ fn parse_cargo_dependencies( deps: &toml::map::Map, parser: &dyn VersionParser, is_dev: bool, - output: &mut Vec, + output: &mut CargoDeclarations, ) { for (name, value) in deps { match value { // 単純な文字列: `package = "1.0.0"` Value::String(s) => { if let Some(spec) = parser.parse(s) { - push_dependency(output, name, spec, is_dev, None, None); + output.push(new_dependency(name, spec, is_dev, None, None), None); } } // inline table 形式: `package = { version = "1.0.0", features = [...] }` @@ -540,13 +540,12 @@ fn parse_cargo_dependencies( // git 依存の検出を先に試みる if let Some(git_source) = try_parse_git_source(t) { let spec = git_reference_spec(&git_source.reference); - push_dependency( - output, - package_name, - spec, - is_dev, - manifest_name, - Some(git_source), + // 公開用に併記した `version` は git 依存の Dependency には残らないが、 + // 宣言をまとめてよいかの判定には要る + let published_version = t.get("version").and_then(|v| v.as_str()); + output.push( + new_dependency(package_name, spec, is_dev, manifest_name, Some(git_source)), + published_version, ); continue; } @@ -571,7 +570,10 @@ fn parse_cargo_dependencies( if let Some(version_str) = t.get("version").and_then(|v| v.as_str()) && let Some(spec) = parser.parse(version_str) { - push_dependency(output, package_name, spec, is_dev, manifest_name, None); + output.push( + new_dependency(package_name, spec, is_dev, manifest_name, None), + None, + ); } } _ => {} @@ -579,14 +581,13 @@ fn parse_cargo_dependencies( } } -fn push_dependency( - output: &mut Vec, +fn new_dependency( name: &str, spec: VersionSpec, is_dev: bool, manifest_name: Option<&str>, git_source: Option, -) { +) -> Dependency { let mut dep = if is_dev { Dependency::development(name.to_string(), spec, Language::Rust) } else { @@ -598,7 +599,63 @@ fn push_dependency( if let Some(gs) = git_source { dep = dep.with_git_source(gs); } - output.push(dep); + dep +} + +/// 宣言をまとめるときのキー: (実パッケージ名, マニフェスト上の依存キー, 版の指定の生表記, +/// git の取得元 (URL, 参照, 公開用に併記した `version` の生表記)) +type DeclarationKey = ( + String, + String, + String, + Option<(String, GitReference, Option)>, +); + +/// parse した依存の一覧。同じ取得元・同じ版を指す宣言は 1 依存にまとめる。 +/// +/// `[dependencies]` と `[dev-dependencies]` に同じ依存を同じ版で書き、dev 側にだけ +/// `features` を足すのは Rust でよくある書き方。宣言ごとに 1 依存を返すと writer の +/// 曖昧性ガード (同じ依存キーの宣言が 2 件以上あれば更新を拒否) に必ず掛かり、 +/// 「2 updates」と表示しながら 1 バイトも書けずに exit code 2 になっていた。 +/// `update_version` / `update_git_tag` は全依存セクションの同名の宣言をすべて書き換えるので、 +/// 取得元と版の指定が一致する宣言は「1 依存 = 1 書き換え」として扱ってよい。 +/// まとめればレジストリや `git ls-remote` への問い合わせも 1 回で済む。 +/// +/// キー ([`DeclarationKey`]) が 1 つでも食い違う宣言はまとめない。マニフェストを書き換える +/// 更新は、従来どおり writer が曖昧として拒否する (git の branch / rev / 既定ブランチの更新は +/// マニフェストを書き換えないので、曖昧判定の対象外)。版の指定は生表記で比べるので、 +/// Cargo では同じ意味の `"1.0"` と `"^1.0"` もまとめない。`features` などの付帯指定と +/// `is_dev` はキーに含めない。 +/// 残すのは最初の宣言だが、`is_dev` はどれか 1 つでも本番の宣言があれば本番にする +/// (`[dev-dependencies]` とターゲット固有の `[dependencies]` の組では dev 側が先に読まれる)。 +#[derive(Default)] +struct CargoDeclarations { + dependencies: Vec, + positions: std::collections::HashMap, +} + +impl CargoDeclarations { + /// 依存を足す。git 依存では、公開用に併記した `version` も `published_version` で渡す + fn push(&mut self, dependency: Dependency, published_version: Option<&str>) { + let key = ( + dependency.name.clone(), + dependency.manifest_name().to_string(), + dependency.version_spec.raw.clone(), + dependency.git_source.as_ref().map(|git| { + ( + git.url.clone(), + git.reference.clone(), + published_version.map(str::to_string), + ) + }), + ); + if let Some(&position) = self.positions.get(&key) { + self.dependencies[position].is_dev &= dependency.is_dev; + } else { + self.positions.insert(key, self.dependencies.len()); + self.dependencies.push(dependency); + } + } } /// inline table から git 依存を検出して `GitSource` を組み立てる @@ -2208,4 +2265,112 @@ toml = { version = "0.8.0", features = ["derive"] } ); assert!(!table.replace("\r\n", "").contains('\n')); } + + #[test] + fn test_parse_merges_declarations_with_same_source_and_version() { + // 回帰 (#20): 同じ依存を [dependencies] と [dev-dependencies] に同じ取得元・ + // 同じ版で書き、dev 側にだけ features を足す構成は 1 依存にまとめる。 + // 宣言ごとに返すと writer の曖昧性ガードで更新を拒否していた + let content = r#" +[dependencies] +tagged = { git = "https://github.com/example/tagged", tag = "v1.0.100" } +anyhow = "1.0.0" +tracked = { git = "https://github.com/example/tracked", branch = "master" } +quoted = "2.0.0" + +[dev-dependencies] +tagged = { git = "https://github.com/example/tagged", tag = "v1.0.100", features = ["preserve_order"] } +anyhow = { version = "1.0.0", features = ["backtrace"] } +tracked = { git = "https://github.com/example/tracked", branch = "master", features = ["preserve_order"] } +quoted = '2.0.0' +"#; + let deps = parse(content).unwrap(); + assert_eq!(deps.len(), 4, "{deps:?}"); + + let tagged = deps.iter().find(|d| d.name == "tagged").unwrap(); + assert!(!tagged.is_dev); + assert_eq!( + tagged.git_source.as_ref().unwrap().reference, + GitReference::Tag("v1.0.100".to_string()) + ); + + let anyhow = deps.iter().find(|d| d.name == "anyhow").unwrap(); + assert!(!anyhow.is_dev); + assert_eq!(anyhow.version_spec.raw, "1.0.0"); + + let tracked = deps.iter().find(|d| d.name == "tracked").unwrap(); + assert!(!tracked.is_dev); + assert_eq!( + tracked.git_source.as_ref().unwrap().reference, + GitReference::Branch("master".to_string()) + ); + } + + #[test] + fn test_parse_keeps_declarations_with_different_source_or_version() { + // 版の指定・タグ・URL・取得元の種類・git に併記した version のどれかが食い違う宣言は + // まとめない (writer が従来どおり曖昧として拒否する)。版の指定は生表記で比べるので、 + // Cargo では同じ意味の "1.0" と "^1.0" もまとめない + let content = r#" +[dependencies] +ranged = "^1.0.0" +caret = "1.0" +tagged = { git = "https://github.com/example/tagged", tag = "v1.0.0" } +forked = { git = "https://github.com/example/forked", tag = "v1.0.0" } +mixed = { git = "https://github.com/example/mixed", branch = "main" } +published = { git = "https://github.com/example/published", tag = "v1.0.0", version = "1.0" } + +[dev-dependencies] +ranged = "=2.0.0" +caret = "^1.0" +tagged = { git = "https://github.com/example/tagged", tag = "v2.0.0" } +forked = { git = "https://github.com/someone/forked", tag = "v1.0.0" } +mixed = "1.0.0" +published = { git = "https://github.com/example/published", tag = "v1.0.0", version = "2.0" } +"#; + let deps = parse(content).unwrap(); + for name in ["ranged", "caret", "tagged", "forked", "mixed", "published"] { + assert_eq!( + deps.iter().filter(|d| d.name == name).count(), + 2, + "{name} の宣言はまとめない: {deps:?}" + ); + } + } + + #[test] + fn test_parse_keeps_same_key_renamed_to_different_packages() { + // 依存キーが同じでも `package` で指す実パッケージが違う宣言はまとめない + let content = r#" +[dependencies] +http = { package = "http", version = "1.0.0" } + +[dev-dependencies] +http = { package = "http-legacy", version = "1.0.0" } +"#; + let deps = parse(content).unwrap(); + assert_eq!(deps.len(), 2, "{deps:?}"); + assert!(deps.iter().all(|d| d.manifest_name() == "http")); + } + + #[test] + fn test_parse_merged_declaration_is_production_when_any_declaration_is() { + // [dev-dependencies] が target 固有の [dependencies] より先に読まれても、 + // 本番の宣言が 1 つでもあれば本番の依存として扱う + let content = r#" +[dev-dependencies] +libc = "0.2.150" +cc = "1.0.83" + +[build-dependencies] +cc = "1.0.83" + +[target.'cfg(unix)'.dependencies] +libc = { version = "0.2.150", features = ["extra_traits"] } +"#; + let deps = parse(content).unwrap(); + assert_eq!(deps.len(), 2, "{deps:?}"); + assert!(!deps.iter().find(|d| d.name == "libc").unwrap().is_dev); + assert!(deps.iter().find(|d| d.name == "cc").unwrap().is_dev); + } } diff --git a/src/manifest/writer.rs b/src/manifest/writer.rs index c0f6a43..890a6f6 100644 --- a/src/manifest/writer.rs +++ b/src/manifest/writer.rs @@ -143,6 +143,9 @@ impl ManifestWriter { // 更新は順番に適用する let mut current_content = content.clone(); + // 曖昧として拒否した依存キー。版や取得元の食い違う宣言が別々の更新になっても、 + // 同じ依存のエラーは 1 行だけ出す + let mut refused_ambiguous: HashSet<&str> = HashSet::new(); for update in manifest_result.results.iter() { if let UpdateResult::Update { @@ -151,6 +154,17 @@ impl ManifestWriter { .. } = update { + // git 依存の branch/default/rev はマニフェストを書き換えない + // (Cargo.lock 側で commit hash が更新されるのを待つ)。何も書き換えないので + // 同名の宣言が他にあっても巻き添えは起きず、曖昧判定を当てない + if dependency + .git_source + .as_ref() + .is_some_and(|git| !matches!(git.reference, GitReference::Tag(_))) + { + continue; + } + let ambiguous_variable = dependency .variable_name .as_ref() @@ -158,38 +172,35 @@ impl ManifestWriter { if ambiguous_declarations.contains(dependency.manifest_name()) || ambiguous_variable { result.updates_failed += 1; - result.errors.push(format!( - "Refusing to update ambiguous dependency '{}' because it has multiple declarations or a shared version target", - dependency.manifest_name() - )); + if refused_ambiguous.insert(dependency.manifest_name()) { + result.errors.push(format!( + "Refusing to update ambiguous dependency '{}' because it has multiple declarations or a shared version target", + dependency.manifest_name() + )); + } continue; } - // git 依存の場合は参照種別で挙動が変わる。 - // - tag: マニフェストの tag 文字列を書き換える - // - branch/default/rev: マニフェストを書き換えない - // (Cargo.lock 側で commit hash が更新されるのを待つ) - if let Some(git) = &dependency.git_source { - if let GitReference::Tag(_) = &git.reference { - match parser.update_git_tag( - ¤t_content, - dependency.manifest_name(), - new_version, - ) { - Ok(updated_content) => { - if updated_content != current_content { - current_content = updated_content; - result.updates_applied += 1; - } - } - Err(e) => { - result.updates_failed += 1; - result.errors.push(format!( - "Failed to update git tag for {}: {}", - dependency.name, e - )); + // git 依存の tag はマニフェストの tag 文字列を書き換える + if dependency.git_source.is_some() { + match parser.update_git_tag( + ¤t_content, + dependency.manifest_name(), + new_version, + ) { + Ok(updated_content) => { + if updated_content != current_content { + current_content = updated_content; + result.updates_applied += 1; } } + Err(e) => { + result.updates_failed += 1; + result.errors.push(format!( + "Failed to update git tag for {}: {}", + dependency.name, e + )); + } } continue; } @@ -1073,6 +1084,164 @@ my-crate = { git = "https://github.com/example/my-crate.git", branch = "main" } assert_eq!(content, original_content); } + /// Cargo.toml を parse し、`(依存名, git 依存か, 更新先)` ごとに該当する依存を + /// 更新として書き込む。parse から書き込みまでを通すための補助で、 + /// 書き込みの結果と書き込み後のファイル内容を返す + fn apply_cargo_updates(content: &str, updates: &[(&str, bool, &str)]) -> (WriteResult, String) { + let temp_dir = TempDir::new().unwrap(); + let path = create_temp_cargo_toml(&temp_dir, content); + let parser = crate::manifest::CargoTomlParser; + let dependencies = parser.parse(content).unwrap(); + + let mut manifest_result = ManifestUpdateResult::new(&path, Language::Rust); + for (name, is_git, new_version) in updates { + let dependency = dependencies + .iter() + .find(|dependency| dependency.name == *name && dependency.is_git() == *is_git) + .unwrap_or_else(|| panic!("{name} が parse されない: {dependencies:?}")) + .clone(); + manifest_result.add_result(UpdateResult::update(dependency, *new_version)); + } + + let result = ManifestWriter::new(false) + .apply_updates(&manifest_result, &parser) + .unwrap(); + let written = fs::read_to_string(&path).unwrap(); + (result, written) + } + + #[test] + fn test_apply_updates_rust_same_git_tag_in_dependencies_and_dev_dependencies() { + // 回帰 (#20): 同じ git の tag 依存を [dependencies] と [dev-dependencies] に書いた + // 構成は曖昧として拒否せず、1 回の更新で両方の tag を書き換える + let content = r#"[dependencies] +serde_json = { git = "https://github.com/example/json", tag = "v1.0.100" } + +[dev-dependencies] +serde_json = { git = "https://github.com/example/json", tag = "v1.0.100", features = ["preserve_order"] } +"#; + let (result, written) = apply_cargo_updates(content, &[("serde_json", true, "v1.0.151")]); + + assert_eq!(result.updates_applied, 1); + assert_eq!(result.updates_failed, 0); + assert!(!result.has_errors(), "errors: {:?}", result.errors); + assert!(result.file_modified); + assert_eq!( + written.matches(r#"tag = "v1.0.151""#).count(), + 2, + "{written}" + ); + assert!(written.contains(r#"features = ["preserve_order"]"#)); + } + + #[test] + fn test_apply_updates_rust_same_registry_version_in_dependencies_and_dev_dependencies() { + // 回帰 (#20): crates.io の依存でも、同じ版の指定の宣言は 1 回の更新で全部書き換える + let content = r#"[dependencies] +anyhow = "1.0.0" + +[dev-dependencies] +anyhow = { version = "1.0.0", features = ["backtrace"] } +"#; + let (result, written) = apply_cargo_updates(content, &[("anyhow", false, "1.0.100")]); + + assert_eq!(result.updates_applied, 1); + assert_eq!(result.updates_failed, 0); + assert!(!result.has_errors(), "errors: {:?}", result.errors); + assert!(written.contains(r#"anyhow = "1.0.100""#), "{written}"); + assert!( + written.contains(r#"anyhow = { version = "1.0.100", features = ["backtrace"] }"#), + "{written}" + ); + } + + #[test] + fn test_apply_updates_rust_same_git_branch_in_dependencies_and_dev_dependencies() { + // 回帰 (#20): branch の git 依存は Cargo.toml を書き換えない更新なので、 + // 宣言が複数あってもエラーにしない + let content = r#"[dependencies] +serde_json = { git = "https://github.com/example/json", branch = "master" } + +[dev-dependencies] +serde_json = { git = "https://github.com/example/json", branch = "master", features = ["preserve_order"] } +"#; + let (result, written) = apply_cargo_updates(content, &[("serde_json", true, "def5678")]); + + assert_eq!(result.updates_applied, 0); + assert_eq!(result.updates_failed, 0); + assert!(!result.has_errors(), "errors: {:?}", result.errors); + assert!(!result.file_modified); + assert_eq!(written, content); + } + + #[test] + fn test_apply_updates_rust_unwritten_git_update_is_not_refused_beside_registry_declaration() { + // branch / rev / 既定ブランチの git 依存と同名の crates.io の宣言が並ぶとき、 + // Cargo.toml を書き換えない git の更新は通し、書き換える crates.io の更新だけを + // 曖昧として拒否する + for git_reference in [r#", branch = "main""#, r#", rev = "abc1234""#, ""] { + let content = format!( + r#"[dependencies] +tracked = {{ git = "https://github.com/example/tracked"{git_reference} }} + +[dev-dependencies] +tracked = "1.0.0" +"# + ); + + let (result, written) = apply_cargo_updates(&content, &[("tracked", true, "def5678")]); + assert_eq!(result.updates_failed, 0, "{git_reference}"); + assert!(!result.has_errors(), "{git_reference}: {:?}", result.errors); + assert_eq!(written, content); + + let (result, written) = apply_cargo_updates( + &content, + &[("tracked", true, "def5678"), ("tracked", false, "1.1.0")], + ); + assert_eq!(result.updates_applied, 0, "{git_reference}"); + assert_eq!(result.updates_failed, 1, "{git_reference}"); + assert_eq!( + result.errors.len(), + 1, + "{git_reference}: {:?}", + result.errors + ); + assert!(result.errors[0].contains("ambiguous dependency 'tracked'")); + assert_eq!(written, content); + } + } + + #[test] + fn test_apply_updates_rust_mismatched_declarations_report_one_error() { + // 版の指定が食い違う宣言は今のとおり拒否する。どちらの宣言も更新になっても、 + // 同じ依存のエラーは 1 行にまとめる + let content = r#"[dependencies] +shared = "^1.0.0" + +[dev-dependencies] +shared = "=2.0.0" +"#; + let temp_dir = TempDir::new().unwrap(); + let path = create_temp_cargo_toml(&temp_dir, content); + let parser = crate::manifest::CargoTomlParser; + let dependencies = parser.parse(content).unwrap(); + assert_eq!(dependencies.len(), 2, "{dependencies:?}"); + + let mut manifest_result = ManifestUpdateResult::new(&path, Language::Rust); + for dependency in dependencies { + manifest_result.add_result(UpdateResult::update(dependency, "3.0.0")); + } + let result = ManifestWriter::new(false) + .apply_updates(&manifest_result, &parser) + .unwrap(); + + assert_eq!(result.updates_applied, 0); + assert_eq!(result.updates_failed, 2); + assert_eq!(result.errors.len(), 1, "errors: {:?}", result.errors); + assert!(result.errors[0].contains("ambiguous dependency 'shared'")); + assert_eq!(fs::read_to_string(&path).unwrap(), content); + } + #[test] fn test_apply_all_updates_empty() { let writer = ManifestWriter::new(false);