diff --git a/AGENTS.md b/AGENTS.md index 10f3a21..0f6fa04 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -257,6 +257,7 @@ make ci # CI と同じ検査 (fmt-check + lint + test、書き換 - 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 を避けて拒否し、スキップ・固定された別宣言を暗黙に書き換えない。拒否のエラーは、同じ依存キーの宣言が別々の更新になっても依存キーごとに 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 扱いになる) +- マニフェストへ書き込めなかった更新 (曖昧として拒否・`update_version` / `update_git_tag` の失敗・読み込みや保存の失敗) は、更新として数えずに別に数える。writer は `WriteResult::failed_updates` に `results` 上の位置と理由を記録し、orchestrator の `write_phase` が `UpdateResult::Update` の `write_error` へ書き込む。summary は書き込み前後で意味を分け、`judged_updates()` / `has_judged_updates()` は judge が決めた更新 (writer が使う)、`updates()` / `update_count()` / `has_updates()` は書き込めなかった更新を除いたもの (出力・install・Rust の lock 監査が使う)、`failed_updates()` / `failed_count()` / `total_failed()` は書き込めなかった更新。件数は「judge が決めた更新 − 書き込めなかった更新」で数え、`updates_applied` は使わない (使うと Cargo.toml を書き換えない git の branch 依存や、書き換えても内容が同じ更新が 0 件になる)。以前は check の段階の件数をそのまま出していたので、1 バイトも書けていないのに「2 package(s) updated」と出ていた。出力は 3 種で数え方を揃える: テキストは見出し `— N updates, K failed, M skips`・行末の `✗ failed` (色なしは `[failed]`)・Summary の `K package(s) failed to update`・`-q` の `N updated, K failed`、JSON は `summary.failed` と各マニフェストの `failed` 配列 (要素に `error`。`updates` からは外す)、diff は hunk を出さず件数行に `, K failed`。全更新を書き込めなかったマニフェストは install の理由にせず (Rust の install は lock 全体を上げる `cargo update`)、Rust の lock 監査も書き込めなかった更新先を「表示した更新先」(差し戻し先の候補・食い違いの注記) に使わない。保存 (I/O) の失敗でも `apply_updates` は `Ok` を返し、保存前に個別の理由で失敗した更新はその理由を残す (`Err` は読み込み・解析に失敗してどの更新も試せなかったときだけ) - 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/cli-reference.ja.md b/docs/cli-reference.ja.md index 966d611..1f3819b 100644 --- a/docs/cli-reference.ja.md +++ b/docs/cli-reference.ja.md @@ -98,6 +98,7 @@ depup --cd ./projects/myapp -n - 変更の種類は `[major]` / `[minor]` / `[patch]` で示します。 - `✓ OSV` は、脆弱性チェックを通過したバージョンに付きます。更新行の下の `↳ OSV skipped:` 行には、脆弱性があるため候補から外したバージョンを表示します。ラベルは skipped ですが、依存のスキップではありません（依存そのものは更新されています）。 - スキップした依存の件数は、マニフェストの見出し（`— N updates, M skips`）と Summary に表示します。更新が 0 件のマニフェストでは、理由ごとの件数も表示します。 +- マニフェストへ書き込めなかった更新（書き込み先が曖昧なための拒否など）は、行末に `✗ failed` が付き、更新の件数に入りません。見出しは `— N updates, K failed, M skips` になり、Summary に `K package(s) failed to update` の行が加わります。理由は `Errors:` 欄に表示します。 - `--verbose` を付けると、スキップした依存を理由ごとに一覧し、Summary にも理由別・言語別の内訳を付けます。 - エラーは、OSV の通知も含めて `Errors:` 欄に表示します。 @@ -109,26 +110,45 @@ depup --json ```json { + "dry_run": false, + "summary": { + "updates": 1, + "failed": 1, + "skips": 1 + }, "manifests": [ { - "path": "package.json", - "language": "node", + "path": "./package.json", + "language": "Node.js", "updates": [ { - "type": "update", - "dependency": { - "name": "lodash", - "version_spec": "^4.17.20" - }, - "new_version": "4.17.21", - "released_at": "2024-12-15T10:30:00Z" + "name": "lodash", + "kind": "registry", + "from": "4.17.20", + "to": "4.17.21", + "dev": false + } + ], + "failed": [ + { + "name": "react", + "kind": "registry", + "from": "18.0.0", + "to": "19.2.0", + "dev": false, + "error": "Refusing to update ambiguous dependency 'react' because it has multiple declarations or a shared version target" } ] } + ], + "errors": [ + "Failed to write ./package.json: Refusing to update ambiguous dependency 'react' because it has multiple declarations or a shared version target" ] } ``` +`summary.updates` と各マニフェストの `updates` に入るのは、書き込めた更新（`--dry-run` では書き込める見込みの更新）だけです。書き込めなかった更新は `summary.failed` に数え、マニフェストの `failed` 配列に入れて、理由を `error` に入れます。git 依存は `"kind": "git"` になり、`source`（認証情報を伏せたリポジトリの URL）と `reference` が加わります。`--verbose` を付けると、各マニフェストに `skips` が、`summary` に `by_language` が加わります。 + ### diff 出力 ```bash @@ -136,13 +156,17 @@ depup --diff ``` ```diff ---- package.json -+++ package.json -@@ dependencies @@ +--- a/./package.json ++++ b/./package.json +@@ lodash @@ - "lodash": "^4.17.20" + "lodash": "^4.17.21" + +# 1 package(s) would be updated, 1 failed ``` +書き込めなかった更新の hunk は出しません。最後の行の件数はテキスト・JSON の出力と同じ数え方で、書き込めなかった更新があるときだけ `, K failed` を付けます。 + ### 終了コード | コード | 意味 | @@ -151,7 +175,7 @@ depup --diff | `1` | パッケージマネージャーの install が失敗した、`--cd` でディレクトリを移動できなかった、または depup 自体が動作を続けられなかった（HTTP クライアントの初期化や、結果の出力に失敗した場合など） | | `2` | 処理の一部が失敗した。マニフェストの読み込み・解析・書き込みの失敗（書き込み先が曖昧なための拒否を含む）、レジストリからの取得の失敗、Rust または Node の lock の age 監査で違反・未確認が残る場合（lock の読み込み失敗や時間切れを含む）が該当する。コマンドライン引数の誤りも `2` になる | -終了コード 2 になる失敗が起きても、処理は止まりません。取得に失敗した依存はスキップし、読み込みや解析に失敗したマニフェストは処理から外して、それ以外の書き込みや install は続けます。書き込みに失敗したファイルは元のまま残りますが、その依存は更新として表示され、`--install` も実行されます。`1` と `2` の両方に当てはまる場合は `1` になります。 +終了コード 2 になる失敗が起きても、処理は止まりません。取得に失敗した依存はスキップし、読み込みや解析に失敗したマニフェストは処理から外して、それ以外の書き込みや install は続けます。ファイルの保存に失敗した場合は、元の内容のまま残ります。マニフェストの一部の更新だけを書き込めなかった場合（書き込み先が曖昧なための拒否など）は、残りの更新を保存します。どちらの場合も、書き込めなかった更新は失敗として表示し、`--install` を実行する理由にもしません。`1` と `2` の両方に当てはまる場合は `1` になります。 取得の問題のうち、次の 3 つは終了コードを変えません。 diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 5c4ae16..d87f220 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -98,6 +98,7 @@ depup --cd ./projects/myapp -n - Change type: `[major]`, `[minor]`, `[patch]` - `✓ OSV` marks versions that passed the vulnerability check. A `↳ OSV skipped:` line below an update lists the vulnerable versions that were removed from the candidates; the dependency itself was still updated - Skipped dependencies are counted in each manifest heading (`— N updates, M skips`) and in the summary; a manifest with no updates also shows the count per reason +- An update that could not be written to the manifest (for example, a write refused as ambiguous) ends with `✗ failed` and is not counted as an update: the heading becomes `— N updates, K failed, M skips`, and the summary adds `K package(s) failed to update`. The reason is listed in the `Errors:` section - With `--verbose`, every skipped dependency is listed under its reason, and the summary is broken down by reason and language - Errors, including OSV notices, are listed in the `Errors:` section @@ -109,26 +110,45 @@ depup --json ```json { + "dry_run": false, + "summary": { + "updates": 1, + "failed": 1, + "skips": 1 + }, "manifests": [ { - "path": "package.json", - "language": "node", + "path": "./package.json", + "language": "Node.js", "updates": [ { - "type": "update", - "dependency": { - "name": "lodash", - "version_spec": "^4.17.20" - }, - "new_version": "4.17.21", - "released_at": "2024-12-15T10:30:00Z" + "name": "lodash", + "kind": "registry", + "from": "4.17.20", + "to": "4.17.21", + "dev": false + } + ], + "failed": [ + { + "name": "react", + "kind": "registry", + "from": "18.0.0", + "to": "19.2.0", + "dev": false, + "error": "Refusing to update ambiguous dependency 'react' because it has multiple declarations or a shared version target" } ] } + ], + "errors": [ + "Failed to write ./package.json: Refusing to update ambiguous dependency 'react' because it has multiple declarations or a shared version target" ] } ``` +`summary.updates` and each manifest's `updates` cover only the updates that were written (with `--dry-run`, the updates that would be written). Updates that could not be written are counted in `summary.failed` and listed in the manifest's `failed` array, with the reason in `error`. Git dependencies use `"kind": "git"` and add `source` (the repository URL with any credentials masked) and `reference`. With `--verbose`, each manifest also lists its `skips`, and the summary adds `by_language`. + ### Diff Output ```bash @@ -136,13 +156,17 @@ depup --diff ``` ```diff ---- package.json -+++ package.json -@@ dependencies @@ +--- a/./package.json ++++ b/./package.json +@@ lodash @@ - "lodash": "^4.17.20" + "lodash": "^4.17.21" + +# 1 package(s) would be updated, 1 failed ``` +Updates that could not be written get no hunk. The last line counts updates the same way as the text and JSON output, and adds `, K failed` only when some updates could not be written. + ### Exit Codes | Code | Meaning | @@ -151,7 +175,7 @@ depup --diff | `1` | A package manager install failed, `--cd` could not change the directory, or depup itself could not continue (for example, the HTTP client failed to initialize or the results could not be written). | | `2` | Part of the run failed: a manifest could not be read, parsed, or written (including a refused ambiguous write), a registry lookup failed, or a Rust or Node lockfile age audit left violations or unverified entries (including an unreadable lock or an exhausted time budget). Invalid command-line arguments also exit with `2`. | -A code-2 failure does not stop the run. A failed lookup skips that dependency, and a manifest that cannot be read or parsed is left out; everything else is still written and installed. A failed write leaves that file unchanged, but its dependencies are still listed as updated and `--install` still runs for it. When both `1` and `2` apply, `1` wins. +A code-2 failure does not stop the run. A failed lookup skips that dependency, and a manifest that cannot be read or parsed is left out; everything else is still written and installed. If a file cannot be saved, it keeps its original content. If only some updates in a manifest cannot be written (for example, one refused as ambiguous), the rest are still saved. Either way, the updates that could not be written are reported as failed instead of updated, and they do not make `--install` run. When both `1` and `2` apply, `1` wins. These lookup problems leave the exit code unchanged: diff --git a/docs/usage.ja.md b/docs/usage.ja.md index 4437bff..9b909bb 100644 --- a/docs/usage.ja.md +++ b/docs/usage.ja.md @@ -331,7 +331,7 @@ depup が書き換えるのは、解析した依存宣言だけです。依存 `--install` を指定すると、depup はマニフェストを書き換えたあと、プロジェクトごとにパッケージマネージャーを実行します。ロックファイルやインストール済みのパッケージを、新しいバージョンに合わせるためです。 -- install を実行するのは、1 件以上更新したマニフェストだけです。`--dry-run` のときは実行しません。 +- install を実行するのは、1 件以上更新したマニフェストだけです。`--dry-run` のときは実行しません。マニフェストへ書き込めなかった更新は数えません。 - Node は pnpm workspace の共有ルート、またはマニフェストのあるディレクトリで install を実行します。他言語は [`.depup`](configuration.ja.md#depup-設定ファイル) がなければ対象ディレクトリ（PATH 引数）を使い、あれば更新したマニフェストを含む対象のうち最も深いディレクトリを使います。 - install は 1 つずつ、ディレクトリのパス順に実行します。同じディレクトリでは、言語ごとに 1 回だけです。パッケージマネージャーの出力は画面に流さずに depup が受け取り、標準エラー出力は install が失敗したときだけ表示します。 @@ -506,4 +506,6 @@ Python プロジェクトで `--install` が `uv sync` を呼び出すとき、d 表の理由はテキスト出力の表記です。`--verbose` を付けた `--json` の出力では、同じ理由が `already_latest`、`pinned`、`change_level_limited: `、`excluded`、`not_in_only_list`、`no_suitable_version`、`parse_error: ...`、`fetch_failed: ...` と表記されます。 +行末に `✗ failed`（色なしでは `[failed]`）が付いた更新の行は、depup が見つけたもののマニフェストへ書き込めなかった更新です。書き込み先が曖昧で拒否した場合などがこれにあたります（[書き込みの共通原則](#書き込みの共通原則)を参照）。更新ではなく失敗として数え、理由は `Errors:` 欄に出し、終了コードは `2` になります。 + 依存が出力にまったく現れない場合、depup はその宣言を対象外にしています。ファイル・セクション・宣言の書き方が対応しているかを、[エコシステム別の詳細](ecosystems.ja.md#エコシステム別の詳細)で確認してください。`--node` のように言語を指定した場合、指定していない言語のマニフェストは解析しません（pnpm・Bun・mise の age 設定だけは読み取ります）。マニフェストは更新されたのに install が失敗した場合は、[パッケージマネージャーの実行](#パッケージマネージャーの実行--install)を参照してください。 diff --git a/docs/usage.md b/docs/usage.md index 6420ec1..585cd44 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -330,7 +330,7 @@ If the same dependency key is declared more than once in a manifest, or several With `--install`, depup runs each project's package manager after writing the manifests, so lock files and installed packages follow the new versions. -- An install runs only for manifests that received at least one update, and never with `--dry-run`. +- An install runs only for manifests that received at least one update, and never with `--dry-run`. An update that could not be written to the manifest does not count. - Node installs run at the shared pnpm workspace root or the manifest directory. Other languages use the target directory (PATH) without [`.depup`](configuration.md#depup-configuration-file), or the deepest listed directory containing the updated manifest when `.depup` is present. - Installs run one at a time, in directory path order, and each language runs at most once per directory. The package manager's output is captured instead of streamed; its stderr is printed only when the install fails. @@ -505,4 +505,6 @@ By default, the text output only counts skipped dependencies. Run with `--verbos These are the labels in the text output. In `--json` output with `--verbose`, the same reasons appear as `already_latest`, `pinned`, `change_level_limited: `, `excluded`, `not_in_only_list`, `no_suitable_version`, `parse_error: ...`, and `fetch_failed: ...`. +An update row that ends with `✗ failed` (`[failed]` without color) is an update depup found but could not write to the manifest, for example because the write was refused as ambiguous (see [Writing Rules](#writing-rules)). It is counted as failed instead of updated, its reason is listed in the `Errors:` section, and the exit code is `2`. + If a dependency does not appear in the output at all, depup did not process the declaration; check in [Ecosystem Details](ecosystems.md#ecosystem-details) that its file, section, and declaration form are supported. Manifests of languages left out by a language flag such as `--node` are not parsed (project age settings in pnpm, Bun, and mise files are still read). If the manifest was updated but the install failed, see [Running the Package Manager](#running-the-package-manager---install). diff --git a/src/domain/mod.rs b/src/domain/mod.rs index d049ed9..bfb2141 100644 --- a/src/domain/mod.rs +++ b/src/domain/mod.rs @@ -22,6 +22,6 @@ pub use change_level::ChangeLevel; pub use dependency::Dependency; pub use git_source::{GitReference, GitSource}; pub use language::Language; -pub use summary::{ManifestUpdateResult, UpdateSummary}; +pub use summary::{LanguageCounts, ManifestUpdateResult, UpdateSummary}; pub use update_result::{SkipReason, UpdateResult}; pub use version_spec::{VersionSpec, VersionSpecKind, range_lower_bound_version}; diff --git a/src/domain/summary.rs b/src/domain/summary.rs index 2145b04..57d3c27 100644 --- a/src/domain/summary.rs +++ b/src/domain/summary.rs @@ -4,7 +4,7 @@ use super::{Language, UpdateResult}; use serde::{Deserialize, Serialize}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; /// 単一マニフェストファイルの更新結果 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -38,9 +38,27 @@ impl ManifestUpdateResult { self.results.push(result); } - /// 更新件数を返す + /// judge が更新すると決めた結果を、書き込みの成否を問わずに返す。 + /// + /// 書き込みの段階 (と、それより前の段階) はこちらを使う。書き込んだ後の + /// 件数・出力・install は、失敗を除いた `updates` を使う + pub fn judged_updates(&self) -> impl Iterator { + self.results.iter().filter(|r| r.is_update()) + } + + /// judge が更新すると決めた結果があるかどうかを返す + pub fn has_judged_updates(&self) -> bool { + self.judged_updates().next().is_some() + } + + /// 更新件数を返す (書き込めなかった更新は数えない) pub fn update_count(&self) -> usize { - self.results.iter().filter(|r| r.is_update()).count() + self.updates().count() + } + + /// 書き込めなかった更新の件数を返す + pub fn failed_count(&self) -> usize { + self.failed_updates().count() } /// スキップ件数を返す @@ -48,9 +66,19 @@ impl ManifestUpdateResult { self.results.iter().filter(|r| r.is_skip()).count() } - /// 全更新を返す + /// 更新を返す (書き込めなかった更新は含めない)。 + /// + /// Cargo.toml を書き換えない更新 (git の branch 依存のように Cargo.lock だけが変わるもの) と、 + /// 書き換えても内容が同じだった更新は、書き込みに失敗していないので含める pub fn updates(&self) -> impl Iterator { - self.results.iter().filter(|r| r.is_update()) + self.results + .iter() + .filter(|r| r.is_update() && r.write_error().is_none()) + } + + /// 書き込めなかった更新を返す + pub fn failed_updates(&self) -> impl Iterator { + self.results.iter().filter(|r| r.write_error().is_some()) } /// 全スキップを返す @@ -58,10 +86,30 @@ impl ManifestUpdateResult { self.results.iter().filter(|r| r.is_skip()) } - /// 依存関係が更新されたかどうかを返す + /// 依存関係が更新されたかどうかを返す (書き込めなかった更新は数えない) pub fn has_updates(&self) -> bool { self.update_count() > 0 } + + /// `results` の `index` 番目の更新を、書き込めなかったものとして記録する + pub fn record_write_error(&mut self, index: usize, reason: impl Into) { + if let Some(result) = self.results.get_mut(index) { + result.record_write_error(reason); + } + } +} + +/// 言語ごとの件数 +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct LanguageCounts { + /// 言語 + pub language: Language, + /// 更新の数 (書き込めなかった更新は数えない) + pub updates: usize, + /// 書き込めなかった更新の数 + pub failed: usize, + /// スキップの数 + pub skips: usize, } /// 全更新操作の総合サマリ @@ -97,11 +145,16 @@ impl UpdateSummary { self.manifests.iter().filter(|m| m.modified).count() } - /// 更新された依存関係の合計数を返す + /// 更新された依存関係の合計数を返す (書き込めなかった更新は数えない) pub fn total_updates(&self) -> usize { self.manifests.iter().map(|m| m.update_count()).sum() } + /// 書き込めなかった更新の合計数を返す + pub fn total_failed(&self) -> usize { + self.manifests.iter().map(|m| m.failed_count()).sum() + } + /// スキップされた依存関係の合計数を返す pub fn total_skips(&self) -> usize { self.manifests.iter().map(|m| m.skip_count()).sum() @@ -124,8 +177,8 @@ impl UpdateSummary { .filter(move |m| m.language == language) } - /// 非空の言語ごとに (言語, 更新数, スキップ数) を `Language::all()` 順で返す - pub fn language_breakdown(&self) -> Vec<(Language, usize, usize)> { + /// 非空の言語ごとの件数を `Language::all()` 順で返す + pub fn language_breakdown(&self) -> Vec { Language::all() .iter() .filter_map(|language| { @@ -133,15 +186,26 @@ impl UpdateSummary { if manifests.is_empty() { None } else { - let updates: usize = manifests.iter().map(|m| m.update_count()).sum(); - let skips: usize = manifests.iter().map(|m| m.skip_count()).sum(); - Some((*language, updates, skips)) + Some(LanguageCounts { + language: *language, + updates: manifests.iter().map(|m| m.update_count()).sum(), + failed: manifests.iter().map(|m| m.failed_count()).sum(), + skips: manifests.iter().map(|m| m.skip_count()).sum(), + }) } }) .collect() } - /// 全マニフェストの全更新を返す + /// `path` のマニフェストの `results` の `index` 番目の更新を、書き込めなかったものとして + /// 記録する。マニフェストのパスは検出の段階で重複を除いてあるので、パスで 1 件に決まる + pub fn record_write_error(&mut self, path: &Path, index: usize, reason: impl Into) { + if let Some(manifest) = self.manifests.iter_mut().find(|m| m.path == path) { + manifest.record_write_error(index, reason); + } + } + + /// 全マニフェストの全更新を返す (書き込めなかった更新は含めない) pub fn all_updates(&self) -> impl Iterator { self.manifests.iter().flat_map(|m| m.updates()) } @@ -381,4 +445,64 @@ mod tests { let parsed: UpdateSummary = serde_json::from_str(&json).unwrap(); assert_eq!(parsed, summary); } + + #[test] + fn test_write_errors_are_counted_apart_from_updates() { + // 回帰 (#21): 書き込めなかった更新は更新として数えず、別に数える。 + // judge が決めた更新 (judged_updates) には残る + let mut summary = UpdateSummary::new(false); + let mut node = ManifestUpdateResult::new("/package.json", Language::Node); + node.add_result(sample_update("lodash")); + node.add_result(sample_update("express")); + node.add_result(sample_skip("react")); + summary.add_manifest(node); + let mut rust = ManifestUpdateResult::new("/Cargo.toml", Language::Rust); + rust.add_result(sample_update("serde")); + summary.add_manifest(rust); + + summary.record_write_error(Path::new("/package.json"), 1, "Failed to update express"); + summary.record_write_error(Path::new("/Cargo.toml"), 0, "Refusing to update serde"); + // Skip とマニフェストの外の位置には何もしない + summary.record_write_error(Path::new("/package.json"), 2, "ignored"); + summary.record_write_error(Path::new("/package.json"), 9, "ignored"); + + let node = &summary.manifests[0]; + assert_eq!(node.judged_updates().count(), 2); + assert_eq!(node.update_count(), 1); + assert_eq!(node.updates().next().unwrap().package_name(), "lodash"); + assert_eq!(node.failed_count(), 1); + assert_eq!( + node.failed_updates().next().unwrap().write_error(), + Some("Failed to update express") + ); + assert_eq!(node.skip_count(), 1); + assert!(node.has_updates()); + + // 全更新を書き込めなかったマニフェストは、更新が無いものとして扱う + let rust = &summary.manifests[1]; + assert!(rust.has_judged_updates()); + assert!(!rust.has_updates()); + assert_eq!(rust.failed_count(), 1); + + assert_eq!(summary.total_updates(), 1); + assert_eq!(summary.total_failed(), 2); + assert_eq!(summary.all_updates().count(), 1); + assert_eq!( + summary.language_breakdown(), + vec![ + LanguageCounts { + language: Language::Node, + updates: 1, + failed: 1, + skips: 1, + }, + LanguageCounts { + language: Language::Rust, + updates: 0, + failed: 1, + skips: 0, + }, + ] + ); + } } diff --git a/src/domain/update_result.rs b/src/domain/update_result.rs index 4410423..e38ab76 100644 --- a/src/domain/update_result.rs +++ b/src/domain/update_result.rs @@ -73,6 +73,10 @@ pub enum UpdateResult { /// age を免除した身元と、レジストリが確認した根拠。 #[serde(default, skip_serializing_if = "Option::is_none")] age_exemption: Option, + /// マニフェストへ書き込めなかったときの理由 (dry-run では書き込めない見込みの理由)。 + /// 書き込みの段階より前は常に `None` + #[serde(default, skip_serializing_if = "Option::is_none")] + write_error: Option, }, /// 依存関係の更新がスキップされた Skip { @@ -103,6 +107,7 @@ impl UpdateResult { osv_skipped: Vec::new(), osv_checked: false, age_exemption: None, + write_error: None, } } @@ -119,6 +124,7 @@ impl UpdateResult { osv_skipped: Vec::new(), osv_checked: false, age_exemption: None, + write_error: None, } } @@ -194,11 +200,29 @@ impl UpdateResult { Self::skip(dependency, SkipReason::FetchFailed(message.into())) } - /// Update結果かどうかを返す + /// Update結果かどうかを返す。 + /// + /// judge が更新すると決めたかどうかで、書き込みの成否は見ない。書き込めた更新だけを + /// 数えるときは `write_error` も見る (`ManifestUpdateResult::updates` を参照) pub fn is_update(&self) -> bool { matches!(self, UpdateResult::Update { .. }) } + /// マニフェストへ書き込めなかった更新の理由を返す + pub fn write_error(&self) -> Option<&str> { + match self { + UpdateResult::Update { write_error, .. } => write_error.as_deref(), + UpdateResult::Skip { .. } => None, + } + } + + /// マニフェストへ書き込めなかったことを記録する。`Skip` 結果に対しては no-op + pub fn record_write_error(&mut self, reason: impl Into) { + if let UpdateResult::Update { write_error, .. } = self { + *write_error = Some(reason.into()); + } + } + /// Skip結果かどうかを返す pub fn is_skip(&self) -> bool { matches!(self, UpdateResult::Skip { .. }) @@ -572,4 +596,34 @@ mod tests { let parsed: UpdateResult = serde_json::from_str(&json).unwrap(); assert_eq!(parsed, result); } + + #[test] + fn test_record_write_error() { + // 書き込めなかった理由は Update にだけ記録し、Skip では何もしない + let mut update = UpdateResult::update(sample_dependency(), "2.0.0"); + assert_eq!(update.write_error(), None); + update.record_write_error("Failed to update lodash"); + assert_eq!(update.write_error(), Some("Failed to update lodash")); + assert!(update.is_update()); + + let mut skip = UpdateResult::skip(sample_dependency(), SkipReason::Excluded); + skip.record_write_error("ignored"); + assert_eq!(skip.write_error(), None); + } + + #[test] + fn test_serde_update_result_write_error() { + // 書き込めなかった理由が無いときは出力せず、無いデータも読み込める + let result = UpdateResult::update(sample_dependency(), "2.0.0"); + let json = serde_json::to_string(&result).unwrap(); + assert!(!json.contains("write_error")); + let parsed: UpdateResult = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed.write_error(), None); + + let mut failed = result; + failed.record_write_error("Failed to update lodash"); + let json = serde_json::to_string(&failed).unwrap(); + let parsed: UpdateResult = serde_json::from_str(&json).unwrap(); + assert_eq!(parsed, failed); + } } diff --git a/src/install/tests.rs b/src/install/tests.rs index e3ab90c..943be44 100644 --- a/src/install/tests.rs +++ b/src/install/tests.rs @@ -221,6 +221,42 @@ fn audit_keeps_the_pre_install_baseline_and_displayed_target() { assert_eq!(reports[0].1[0].locked, "2.0.1"); } +/// `result_with_update` の更新を、マニフェストへ書き込めなかったことにした結果 +fn result_with_failed_update(path: PathBuf, language: Language) -> OrchestratorResult { + let mut result = result_with_update(path, language); + result.summary.manifests[0].record_write_error(0, "Failed to update pkg"); + result +} + +#[test] +fn build_install_map_skips_manifest_whose_updates_all_failed() { + // 回帰 (#21): 書き込めなかった更新しかないマニフェストは何も書き換えていないので、 + // install の理由にしない (Rust の install は lock 全体を上げる `cargo update`) + let root = PathBuf::from("/repo"); + let result = result_with_failed_update(root.join("Cargo.toml"), Language::Rust); + + assert!(build_install_map(&result, &None, &root).is_empty()); +} + +#[test] +fn audit_does_not_treat_an_unwritten_update_as_displayed() { + // 書き込めなかった更新先は、差し戻し先の候補にも「表示した更新先と lock の食い違い」の + // 注記にも使わない。lock の監査そのものは今までどおり計画する + let temp = tempfile::tempdir().unwrap(); + let mut args = CliArgs::parse_from(["depup", "--age", "2w"]); + args.path = temp.path().to_path_buf(); + let orchestrator = Orchestrator::new(args).unwrap(); + let result = result_with_failed_update(temp.path().join("Cargo.toml"), Language::Rust); + write_lock(temp.path(), "1.0.0"); + let plan = InstallPlan::new(&orchestrator, &result, &None, temp.path()); + write_lock(temp.path(), "2.0.1"); + let audit = plan.rust_audit_plan(&orchestrator, &result); + assert_eq!(audit.locks.len(), 1); + assert_eq!(audit.locks[0].baseline["pkg"], vec!["1.0.0"]); + assert!(!audit.locks[0].preferred.contains_key("pkg")); + assert!(audit.mismatch_reports().is_empty()); +} + #[test] fn a_lock_created_by_install_has_an_empty_baseline() { let temp = tempfile::tempdir().unwrap(); diff --git a/src/manifest/mod.rs b/src/manifest/mod.rs index dc61e26..1500e71 100644 --- a/src/manifest/mod.rs +++ b/src/manifest/mod.rs @@ -51,7 +51,7 @@ pub use package_swift::PackageSwiftParser; pub use pnpm_settings::{PnpmSettings, has_pnpm_workspace}; pub use pyproject_toml::PyprojectTomlParser; pub use tool_versions::TOOL_VERSIONS_FILENAME; -pub use writer::{ManifestWriter, WriteResult, read_manifest, write_manifest}; +pub use writer::{ManifestWriter, WriteFailure, WriteResult, read_manifest, write_manifest}; use crate::domain::{Dependency, Language}; use crate::error::ManifestError; diff --git a/src/manifest/writer.rs b/src/manifest/writer.rs index 890a6f6..b2030b6 100644 --- a/src/manifest/writer.rs +++ b/src/manifest/writer.rs @@ -6,7 +6,7 @@ //! - 更新時の書式保持 //! - 失敗時も継続できるエラーハンドリング -use crate::domain::{GitReference, Language, ManifestUpdateResult, UpdateResult}; +use crate::domain::{Dependency, GitReference, Language, ManifestUpdateResult, UpdateResult}; use crate::error::ManifestError; use crate::manifest::ManifestParser; use std::collections::{HashMap, HashSet}; @@ -26,23 +26,33 @@ pub struct ManifestWriter { pub struct WriteResult { /// 対象マニフェストのパス pub path: std::path::PathBuf, - /// 実際に反映された更新数 + /// 実際に反映された更新数 (書き換えても内容が同じだった更新と、 + /// マニフェストを書き換えない git の更新は数えない) pub updates_applied: usize, - /// 失敗した更新数 - pub updates_failed: usize, + /// 書き込めなかった更新 (`ManifestUpdateResult::results` 上の位置の順) + pub failed_updates: Vec, /// 実ファイルが変更されたかどうか pub file_modified: bool, /// 更新中に発生したエラー pub errors: Vec, } +/// 書き込めなかった更新 1 件 +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WriteFailure { + /// `ManifestUpdateResult::results` 上の位置 + pub index: usize, + /// 書き込めなかった理由 + pub reason: String, +} + impl WriteResult { /// 新しい `WriteResult` を作る fn new(path: impl Into) -> Self { Self { path: path.into(), updates_applied: 0, - updates_failed: 0, + failed_updates: Vec::new(), file_modified: false, errors: Vec::new(), } @@ -53,10 +63,57 @@ impl WriteResult { self.updates_applied > 0 } + /// 書き込めなかった更新の数 + pub fn updates_failed(&self) -> usize { + self.failed_updates.len() + } + /// エラーがあるかどうか pub fn has_errors(&self) -> bool { !self.errors.is_empty() } + + /// `index` の更新を書き込めなかったものとして記録する + fn record_failure(&mut self, index: usize, reason: impl Into) { + self.failed_updates.push(WriteFailure { + index, + reason: reason.into(), + }); + } + + /// マニフェスト全体の失敗 (読み込み・解析・保存の失敗) を記録する。 + /// + /// マニフェストを書き換える更新はどれも反映されていないので、まだ個別の理由が + /// 記録されていない更新を `reason` で書き込めなかったものにする。個別の理由 + /// (曖昧として拒否した、など) がある更新は、その理由を残す + fn record_manifest_failure(&mut self, manifest_result: &ManifestUpdateResult, reason: String) { + for (index, update) in manifest_result.results.iter().enumerate() { + if let UpdateResult::Update { dependency, .. } = update + && rewrites_manifest(dependency) + && !self + .failed_updates + .iter() + .any(|failure| failure.index == index) + { + self.record_failure(index, reason.clone()); + } + } + self.failed_updates.sort_by_key(|failure| failure.index); + self.updates_applied = 0; + self.file_modified = false; + self.errors.push(reason); + } +} + +/// 更新がマニフェストを書き換えるかどうかを返す。 +/// +/// git 依存の branch / rev / 既定ブランチはマニフェストを書き換えない +/// (Cargo.lock 側で commit hash が更新されるのを待つ) +fn rewrites_manifest(dependency: &Dependency) -> bool { + match &dependency.git_source { + Some(git) => matches!(git.reference, GitReference::Tag(_)), + None => true, + } } impl ManifestWriter { @@ -75,7 +132,11 @@ impl ManifestWriter { self.dry_run } - /// `ManifestUpdateResult` の更新をファイルへ適用する + /// `ManifestUpdateResult` の更新をファイルへ適用する。 + /// + /// 書き込めなかった更新は `WriteResult::failed_updates` に記録する。保存 (I/O) に失敗した + /// ときも `Ok` を返し、書き換えた更新をすべて書き込めなかったものとして記録する。 + /// `Err` を返すのは、マニフェストを読み込めず、どの更新も試せなかったときだけ pub fn apply_updates( &self, manifest_result: &ManifestUpdateResult, @@ -147,21 +208,16 @@ impl ManifestWriter { // 同じ依存のエラーは 1 行だけ出す let mut refused_ambiguous: HashSet<&str> = HashSet::new(); - for update in manifest_result.results.iter() { + for (index, update) in manifest_result.results.iter().enumerate() { if let UpdateResult::Update { dependency, new_version, .. } = update { - // git 依存の branch/default/rev はマニフェストを書き換えない - // (Cargo.lock 側で commit hash が更新されるのを待つ)。何も書き換えないので - // 同名の宣言が他にあっても巻き添えは起きず、曖昧判定を当てない - if dependency - .git_source - .as_ref() - .is_some_and(|git| !matches!(git.reference, GitReference::Tag(_))) - { + // マニフェストを書き換えない更新 (git 依存の branch/default/rev) は、 + // 同名の宣言が他にあっても巻き添えは起きないので、曖昧判定を当てない + if !rewrites_manifest(dependency) { continue; } @@ -171,13 +227,14 @@ impl ManifestWriter { .is_some_and(|name| ambiguous_variables.contains(name)); if ambiguous_declarations.contains(dependency.manifest_name()) || ambiguous_variable { - result.updates_failed += 1; + let reason = 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() - )); + result.errors.push(reason.clone()); } + result.record_failure(index, reason); continue; } @@ -195,11 +252,10 @@ impl ManifestWriter { } } Err(e) => { - result.updates_failed += 1; - result.errors.push(format!( - "Failed to update git tag for {}: {}", - dependency.name, e - )); + let reason = + format!("Failed to update git tag for {}: {}", dependency.name, e); + result.errors.push(reason.clone()); + result.record_failure(index, reason); } } continue; @@ -217,10 +273,9 @@ impl ManifestWriter { } } Err(e) => { - result.updates_failed += 1; - result - .errors - .push(format!("Failed to update {}: {}", dependency.name, e)); + let reason = format!("Failed to update {}: {}", dependency.name, e); + result.errors.push(reason.clone()); + result.record_failure(index, reason); } } } @@ -228,10 +283,17 @@ impl ManifestWriter { // dry-run でなく、実際に変更がある場合のみ書き戻す if result.updates_applied > 0 && !self.dry_run { - write_atomically(path, ¤t_content).map_err(|e| ManifestError::WriteError { - path: path.clone(), - source: e, - })?; + if let Err(source) = write_atomically(path, ¤t_content) { + let error = ManifestError::WriteError { + path: path.clone(), + source, + }; + result.record_manifest_failure( + manifest_result, + format!("Failed to process manifest: {}", error), + ); + return Ok(result); + } result.file_modified = true; } @@ -246,23 +308,20 @@ impl ManifestWriter { ) -> Vec { manifests .iter() - .filter_map(|manifest| { - // 更新対象があるマニフェストだけ処理する - if !manifest.has_updates() { - return None; - } - + // 更新対象があるマニフェストだけ処理する + .filter(|manifest| manifest.has_judged_updates()) + .map(|manifest| { let parser = get_parser(manifest.language); - match self.apply_updates(manifest, parser.as_ref()) { - Ok(result) => Some(result), - Err(e) => { + self.apply_updates(manifest, parser.as_ref()) + .unwrap_or_else(|e| { + // 読み込み・解析に失敗したので、どの更新も試せていない let mut result = WriteResult::new(&manifest.path); + result.record_manifest_failure( + manifest, + format!("Failed to process manifest: {}", e), + ); result - .errors - .push(format!("Failed to process manifest: {}", e)); - Some(result) - } - } + }) }) .collect() } @@ -509,7 +568,7 @@ mod tests { let result = WriteResult::new("/path/to/file"); assert_eq!(result.path, std::path::PathBuf::from("/path/to/file")); assert_eq!(result.updates_applied, 0); - assert_eq!(result.updates_failed, 0); + assert_eq!(result.updates_failed(), 0); assert!(!result.file_modified); assert!(result.errors.is_empty()); } @@ -673,7 +732,7 @@ tokio_v1 = { package = "tokio", version = "1.0", features = ["rt"] } .unwrap(); assert_eq!(result.updates_applied, 0); - assert_eq!(result.updates_failed, 1); + assert_eq!(result.updates_failed(), 1); assert!(!result.file_modified); assert!(result.errors[0].contains("ambiguous dependency")); assert_eq!(fs::read_to_string(path).unwrap(), original_content); @@ -707,7 +766,7 @@ beta = { module = "com.example:beta", version.ref = "shared" } .unwrap(); assert_eq!(result.updates_applied, 0); - assert_eq!(result.updates_failed, 1); + assert_eq!(result.updates_failed(), 1); assert!(!result.file_modified); assert_eq!(fs::read_to_string(&path).unwrap(), original_content); } @@ -734,7 +793,7 @@ android-library = { id = "com.android.library", version.ref = "agp" } .apply_updates(&manifest_result, &parser) .unwrap(); assert_eq!(result.updates_applied, 1); - assert_eq!(result.updates_failed, 0); + assert_eq!(result.updates_failed(), 0); assert!(result.file_modified); let updated = fs::read_to_string(path).unwrap(); assert!(updated.contains("agp = \"9.4.1\"")); @@ -774,7 +833,7 @@ android-library = { id = "com.android.library", version.ref = "agp" } .unwrap(); assert_eq!(result.updates_applied, 1); - assert_eq!(result.updates_failed, 0); + assert_eq!(result.updates_failed(), 0); assert!(!result.has_errors(), "errors: {:?}", result.errors); assert!(result.file_modified); @@ -820,7 +879,7 @@ dependencies { .unwrap(); assert_eq!(result.updates_applied, 0); - assert_eq!(result.updates_failed, 1); + assert_eq!(result.updates_failed(), 1); assert!(!result.file_modified); assert!(result.errors[0].contains("ambiguous dependency")); assert_eq!(fs::read_to_string(&path).unwrap(), original_content); @@ -851,7 +910,7 @@ dependencies { .unwrap(); assert_eq!(result.updates_applied, 0); - assert_eq!(result.updates_failed, 1); + assert_eq!(result.updates_failed(), 1); assert!(!result.file_modified); assert!(result.errors[0].contains("ambiguous dependency")); assert_eq!(fs::read_to_string(&path).unwrap(), original_content); @@ -882,7 +941,7 @@ dependencies { let result = writer.apply_updates(&manifest_result, &parser).unwrap(); assert_eq!(result.updates_applied, 1); - assert_eq!(result.updates_failed, 1); + assert_eq!(result.updates_failed(), 1); assert!(result.has_errors()); assert!(result.file_modified); // 成功分は書き戻される } @@ -941,7 +1000,7 @@ dependencies { let result = writer.apply_updates(&manifest_result, &NoOpParser).unwrap(); assert_eq!(result.updates_applied, 0); - assert_eq!(result.updates_failed, 0); + assert_eq!(result.updates_failed(), 0); assert!(!result.file_modified); assert!(!result.has_errors()); @@ -998,6 +1057,9 @@ dependencies { let mut manifest_result = ManifestUpdateResult::new(&path, Language::Node); let dep = sample_dependency("lodash", "4.17.21", Language::Node); manifest_result.add_result(UpdateResult::update(dep, "4.18.0")); + // 保存の前に個別の理由で失敗する更新 + let missing = sample_dependency("nonexistent", "1.0.0", Language::Node); + manifest_result.add_result(UpdateResult::update(missing, "2.0.0")); let writer = ManifestWriter::new(false); let parser = crate::manifest::PackageJsonParser; @@ -1008,11 +1070,36 @@ dependencies { perms.set_mode(0o644); fs::set_permissions(&path, perms).unwrap(); - assert!(result.is_err()); - match result.unwrap_err() { - ManifestError::WriteError { .. } => {} - e => panic!("Expected WriteError, got: {:?}", e), - } + // 保存に失敗しても Err にせず、書き換えた更新を書き込めなかったものとして記録する。 + // 保存の前に個別の理由で失敗した更新は、その理由を残す + let result = result.unwrap(); + assert_eq!(result.updates_applied, 0); + assert!(!result.file_modified); + assert_eq!(result.updates_failed(), 2); + assert_eq!(result.failed_updates[0].index, 0); + assert!( + result.failed_updates[0] + .reason + .contains("failed to write manifest file"), + "{:?}", + result.failed_updates + ); + assert_eq!(result.failed_updates[1].index, 1); + assert!( + result.failed_updates[1] + .reason + .starts_with("Failed to update nonexistent"), + "{:?}", + result.failed_updates + ); + assert_eq!(result.errors.len(), 2, "{:?}", result.errors); + assert!( + result + .errors + .iter() + .any(|error| error.starts_with("Failed to process manifest")) + ); + assert_eq!(fs::read_to_string(&path).unwrap(), original_content); } #[test] @@ -1123,7 +1210,7 @@ serde_json = { git = "https://github.com/example/json", tag = "v1.0.100", featur 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_eq!(result.updates_failed(), 0); assert!(!result.has_errors(), "errors: {:?}", result.errors); assert!(result.file_modified); assert_eq!( @@ -1146,7 +1233,7 @@ 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_eq!(result.updates_failed(), 0); assert!(!result.has_errors(), "errors: {:?}", result.errors); assert!(written.contains(r#"anyhow = "1.0.100""#), "{written}"); assert!( @@ -1168,7 +1255,7 @@ serde_json = { git = "https://github.com/example/json", branch = "master", featu let (result, written) = apply_cargo_updates(content, &[("serde_json", true, "def5678")]); assert_eq!(result.updates_applied, 0); - assert_eq!(result.updates_failed, 0); + assert_eq!(result.updates_failed(), 0); assert!(!result.has_errors(), "errors: {:?}", result.errors); assert!(!result.file_modified); assert_eq!(written, content); @@ -1190,7 +1277,7 @@ tracked = "1.0.0" ); let (result, written) = apply_cargo_updates(&content, &[("tracked", true, "def5678")]); - assert_eq!(result.updates_failed, 0, "{git_reference}"); + assert_eq!(result.updates_failed(), 0, "{git_reference}"); assert!(!result.has_errors(), "{git_reference}: {:?}", result.errors); assert_eq!(written, content); @@ -1199,7 +1286,7 @@ tracked = "1.0.0" &[("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.updates_failed(), 1, "{git_reference}"); assert_eq!( result.errors.len(), 1, @@ -1236,7 +1323,7 @@ shared = "=2.0.0" .unwrap(); assert_eq!(result.updates_applied, 0); - assert_eq!(result.updates_failed, 2); + 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); @@ -1281,5 +1368,132 @@ shared = "=2.0.0" assert_eq!(results.len(), 1); assert!(results[0].has_errors()); + // 読み込めないマニフェストの更新は、どれも書き込めなかったものとして記録する + assert_eq!(results[0].updates_failed(), 1); + assert_eq!(results[0].failed_updates[0].index, 0); + assert!( + results[0].failed_updates[0] + .reason + .starts_with("Failed to process manifest") + ); + } + + #[test] + fn test_apply_updates_records_failed_update_positions() { + // 回帰 (#21): 書き込めなかった更新は、`results` 上の位置と理由で記録する。 + // 出力はこれを使って、書き込めた更新と書き込めなかった更新を分けて数える + let temp_dir = TempDir::new().unwrap(); + let path = create_temp_package_json( + &temp_dir, + r#"{ + "dependencies": { + "lodash": "^4.17.21" + } +}"#, + ); + + let mut manifest_result = ManifestUpdateResult::new(&path, Language::Node); + let express = sample_dependency("express", "4.18.0", Language::Node); + manifest_result.add_result(UpdateResult::skip_already_latest(express)); + let lodash = sample_dependency("lodash", "4.17.21", Language::Node); + manifest_result.add_result(UpdateResult::update(lodash, "4.18.0")); + let missing = sample_dependency("nonexistent", "1.0.0", Language::Node); + manifest_result.add_result(UpdateResult::update(missing, "2.0.0")); + + let result = ManifestWriter::new(false) + .apply_updates(&manifest_result, &crate::manifest::PackageJsonParser) + .unwrap(); + + assert_eq!(result.updates_applied, 1); + assert_eq!(result.updates_failed(), 1); + assert_eq!(result.failed_updates[0].index, 2); + assert!( + result.failed_updates[0] + .reason + .starts_with("Failed to update nonexistent"), + "{:?}", + result.failed_updates + ); + assert_eq!(result.errors, vec![result.failed_updates[0].reason.clone()]); + } + + #[test] + fn test_apply_updates_dry_run_records_refused_updates() { + // dry-run でも、書き込めない見込みの更新を記録する (ファイルは変えない) + let temp_dir = TempDir::new().unwrap(); + let original_content = r#"{ + "dependencies": { + "shared": "^1.0.0" + }, + "devDependencies": { + "shared": "~1.0.0" + } +}"#; + let path = create_temp_package_json(&temp_dir, original_content); + let parser = crate::manifest::PackageJsonParser; + let mut manifest_result = ManifestUpdateResult::new(&path, Language::Node); + for dependency in parser.parse(original_content).unwrap() { + manifest_result.add_result(UpdateResult::update(dependency, "1.5.0")); + } + + let result = ManifestWriter::dry_run() + .apply_updates(&manifest_result, &parser) + .unwrap(); + + assert_eq!(result.updates_failed(), 2); + assert_eq!( + result + .failed_updates + .iter() + .map(|failure| failure.index) + .collect::>(), + vec![0, 1] + ); + assert_eq!(result.errors.len(), 1, "{:?}", result.errors); + assert_eq!(fs::read_to_string(&path).unwrap(), original_content); + } + + #[test] + fn test_apply_all_updates_unreadable_manifest_keeps_unwritten_git_updates() { + // マニフェストを読み込めなくても、マニフェストを書き換えない git の更新 + // (Cargo.lock 側で上がる branch の依存) は書き込めなかったものにしない + let mut manifest_result = + ManifestUpdateResult::new("/nonexistent/path/Cargo.toml", Language::Rust); + let branch = Dependency::new( + "tracked", + VersionSpec::new(VersionSpecKind::Exact, "main", "main"), + false, + Language::Rust, + ) + .with_git_source(GitSource::new( + "https://github.com/example/tracked", + GitReference::Branch("main".to_string()), + )); + manifest_result.add_result(UpdateResult::update(branch, "def5678")); + let tagged = Dependency::new( + "tagged", + VersionSpec::new(VersionSpecKind::Exact, "v1.0.0", "v1.0.0"), + false, + Language::Rust, + ) + .with_git_source(GitSource::new( + "https://github.com/example/tagged", + GitReference::Tag("v1.0.0".to_string()), + )); + manifest_result.add_result(UpdateResult::update(tagged, "v1.1.0")); + + let results = ManifestWriter::new(false).apply_all_updates(&[manifest_result], |_| { + Box::new(crate::manifest::CargoTomlParser) + }); + + assert_eq!(results.len(), 1); + assert_eq!( + results[0] + .failed_updates + .iter() + .map(|failure| failure.index) + .collect::>(), + vec![1] + ); } } diff --git a/src/orchestrator.rs b/src/orchestrator.rs index 10edf15..66fc0de 100644 --- a/src/orchestrator.rs +++ b/src/orchestrator.rs @@ -496,7 +496,7 @@ impl Orchestrator { .await; self.sync_tauri_if_needed(manifests, progress, &mut summary, &mut errors) .await; - let write_results = self.write_phase(&summary, progress, &mut errors); + let write_results = self.write_phase(&mut summary, progress, &mut errors); OrchestratorResult { summary, @@ -675,9 +675,12 @@ impl Orchestrator { } /// 書き込み phase: 更新を適用 (`dry_run` ならプレビューのみ) し、書き込みエラーを集約する。 + /// + /// 書き込めなかった更新は `summary` に記録する。以後の出力・install・lock の監査は、 + /// 書き込めた (dry-run では書き込める見込みの) 更新だけを更新として扱う fn write_phase( &self, - summary: &UpdateSummary, + summary: &mut UpdateSummary, progress: &mut Progress, errors: &mut Vec, ) -> Vec { @@ -689,6 +692,9 @@ impl Orchestrator { progress.finish_and_clear(); for result in &write_results { + for failure in &result.failed_updates { + summary.record_write_error(&result.path, failure.index, failure.reason.clone()); + } for error in &result.errors { errors.push(OrchestratorError::WriteError { path: result.path.display().to_string(), @@ -1888,6 +1894,58 @@ mod tests { assert_eq!(app_result.updates().count(), 0); } + #[tokio::test] + async fn write_failures_are_recorded_in_summary() { + // 回帰 (#21): 書き込めなかった更新 (ここでは曖昧として拒否した更新) は summary に + // 記録し、更新の件数に入れない。dry-run でも書き込める見込みかどうかで数える + let root = TempDir::new().unwrap(); + fs::write( + root.path().join("package.json"), + r#"{"dependencies":{"shared":"^1.0.0","library":"^1.0.0"},"devDependencies":{"shared":"~1.0.0"}}"#, + ) + .unwrap(); + let orchestrator = Orchestrator::new(make_args_with_path( + root.path(), + &["--no-age", "--no-osv", "--dry-run", "--quiet"], + )) + .unwrap(); + let now = chrono::Utc::now(); + { + let mut cache = orchestrator.versions.cache.lock().await; + for name in ["shared", "library"] { + cache.insert( + (Language::Node, name.into()), + vec![ + VersionInfo::new("1.0.0", now - chrono::Duration::days(60)), + VersionInfo::new("1.5.0", now - chrono::Duration::days(30)), + ], + ); + } + } + + let result = orchestrator.run().await; + + let manifest = &result.summary.manifests[0]; + assert_eq!(manifest.judged_updates().count(), 3); + let updated: Vec<_> = manifest.updates().map(|u| u.package_name()).collect(); + assert_eq!(updated, vec!["library"]); + let failed: Vec<_> = manifest + .failed_updates() + .map(|u| (u.package_name(), u.dependency().is_dev)) + .collect(); + assert_eq!(failed, vec![("shared", false), ("shared", true)]); + assert!(manifest.failed_updates().all(|u| { + u.write_error() + .unwrap() + .contains("ambiguous dependency 'shared'") + })); + assert_eq!(result.summary.total_updates(), 1); + assert_eq!(result.summary.total_failed(), 2); + // エラーの行は依存ごとに 1 行 + assert_eq!(result.errors.len(), 1, "{:?}", result.errors); + assert_eq!(result.write_results[0].updates_failed(), 2); + } + #[tokio::test] async fn tauri_sync_is_scoped_to_each_project_age_policy() { let root = TempDir::new().unwrap(); diff --git a/src/output/diff.rs b/src/output/diff.rs index d259ea4..eb0ad37 100644 --- a/src/output/diff.rs +++ b/src/output/diff.rs @@ -30,7 +30,8 @@ impl DiffFormatter { /// /// 実際にマニフェストへ書き込まれる更新だけを対象にする。git 依存の /// branch / default / rev はマニフェストを書き換えない (Cargo.lock 側の - /// 更新のみ) ため、偽の差分として表示しない。表示対象がなければ何も書かない。 + /// 更新のみ) ため、偽の差分として表示しない。書き込めなかった更新も + /// (`updates` が含めないので) 表示しない。表示対象がなければ何も書かない。 fn write_manifest_diff( &self, manifest: &ManifestUpdateResult, @@ -91,6 +92,14 @@ impl DiffFormatter { } } +/// 書き込めなかった更新があるときだけ、件数の後ろに付ける `, N failed` +fn failed_suffix(summary: &UpdateSummary) -> String { + match summary.total_failed() { + 0 => String::new(), + failed => format!(", {} failed", failed), + } +} + impl OutputFormatter for DiffFormatter { fn format(&self, result: &OrchestratorResult, writer: &mut dyn Write) -> std::io::Result<()> { let prefix = self.dry_run_prefix(); @@ -101,12 +110,15 @@ impl OutputFormatter for DiffFormatter { } } - // 最後にサマリを書く + // 最後にサマリを書く。件数はテキスト・JSON の出力と同じく、書き込めなかった更新を + // 含めずに別に数える (hunk の無い更新、たとえば Cargo.lock だけが変わる git 依存も数える) let updates = result.summary.total_updates(); writeln!( writer, - "{}# {} package(s) would be updated", - prefix, updates + "{}# {} package(s) would be updated{}", + prefix, + updates, + failed_suffix(&result.summary) )?; Ok(()) @@ -123,8 +135,11 @@ impl OutputFormatter for DiffFormatter { writeln!( writer, - "{}# {} package(s) updated, {} skipped", - prefix, updates, skips + "{}# {} package(s) updated{}, {} skipped", + prefix, + updates, + failed_suffix(summary), + skips )?; Ok(()) @@ -434,6 +449,39 @@ mod tests { assert!(output_str.contains("1 skipped")); } + #[test] + fn test_format_diff_excludes_write_failures() { + // 回帰 (#21): 書き込めなかった更新は hunk に出さず、件数にも入れずに別に数える + let mut result = create_test_result(); + let manifest = &mut result.summary.manifests[0]; + manifest.add_result(UpdateResult::update( + sample_dependency("react", "18.0.0"), + "19.0.0", + )); + manifest.record_write_error(1, "Failed to update react"); + + let formatter = DiffFormatter::new(false); + let mut output = Vec::new(); + formatter.format(&result, &mut output).unwrap(); + let text = String::from_utf8(output).unwrap(); + assert!(text.contains("@@ lodash @@"), "{text}"); + assert!(!text.contains("react"), "{text}"); + assert!( + text.contains("# 1 package(s) would be updated, 1 failed"), + "{text}" + ); + + let mut output = Vec::new(); + formatter + .format_summary(&result.summary, &mut output) + .unwrap(); + let text = String::from_utf8(output).unwrap(); + assert!( + text.contains("# 1 package(s) updated, 1 failed, 0 skipped"), + "{text}" + ); + } + #[test] fn test_format_manifest_no_updates() { // 更新なしのマニフェストは出力されない diff --git a/src/output/json.rs b/src/output/json.rs index 11b9a51..1d95b8a 100644 --- a/src/output/json.rs +++ b/src/output/json.rs @@ -42,8 +42,10 @@ struct JsonOutput { /// サマリ統計の JSON 表現 #[derive(Serialize)] struct JsonSummary { - /// 更新の総数 + /// 更新の総数 (書き込めなかった更新は数えない) updates: usize, + /// 書き込めなかった更新の総数 + failed: usize, /// スキップの総数 skips: usize, /// 言語別の内訳 @@ -56,8 +58,10 @@ struct JsonSummary { struct JsonLanguageSummary { /// 言語名 language: String, - /// 更新数 + /// 更新数 (書き込めなかった更新は数えない) updates: usize, + /// 書き込めなかった更新の数 + failed: usize, /// スキップ数 skips: usize, } @@ -69,8 +73,10 @@ struct JsonManifest { path: String, /// マニフェストの言語 language: String, - /// 更新のリスト + /// 更新のリスト (書き込めなかった更新は含めない) updates: Vec, + /// 書き込めなかった更新のリスト。各要素の `error` に理由が入る + failed: Vec, /// スキップのリスト (verbose モードのみ) #[serde(skip_serializing_if = "Vec::is_empty")] skips: Vec, @@ -97,6 +103,9 @@ struct JsonUpdate { /// git 依存の場合の参照情報 (type/value) #[serde(skip_serializing_if = "Option::is_none")] reference: Option, + /// マニフェストへ書き込めなかった理由 (`failed` の要素だけ) + #[serde(skip_serializing_if = "Option::is_none")] + error: Option, } /// git 参照の JSON 表現 (branch/tag/rev/default_branch) @@ -161,55 +170,64 @@ impl JsonFormatter { } } + /// 更新 1 件を JSON 表現に変換する (`Skip` 結果は `None`) + fn update_to_json(result: &UpdateResult) -> Option { + let UpdateResult::Update { + dependency, + new_version, + age_exemption, + write_error, + .. + } = result + else { + return None; + }; + if let Some(git) = &dependency.git_source { + let from = match &git.reference { + GitReference::Tag(t) => t.clone(), + _ => git.current_commit.clone().unwrap_or_default(), + }; + Some(JsonUpdate { + age_exemption: age_exemption.clone(), + name: dependency.name.clone(), + kind: "git", + from, + to: new_version.clone(), + dev: dependency.is_dev, + // URL に埋め込まれたアクセストークンを伏せる。 + // fetch 成功時の Update は GitRemoteError を経由しないため、 + // エラー側の伏せ字が効かず、ここが唯一の未伏せ出力経路だった + // (CI の JSON 成果物・ログにトークンが残る)。 + source: Some(crate::registry::redact_url(&git.url)), + reference: Some(JsonGitReference::from_reference(&git.reference)), + error: write_error.clone(), + }) + } else { + Some(JsonUpdate { + age_exemption: age_exemption.clone(), + name: dependency.name.clone(), + kind: "registry", + // `to` はレジストリが返す生の値なので、`from` も同じ書式 + // (Go なら `v` 付き) に揃える + from: dependency.version_spec.display_version(), + to: new_version.clone(), + dev: dependency.is_dev, + source: None, + reference: None, + error: write_error.clone(), + }) + } + } + /// マニフェスト結果を JSON 表現に変換 fn manifest_to_json(&self, manifest: &ManifestUpdateResult) -> JsonManifest { let updates: Vec = manifest .updates() - .filter_map(|result| { - if let UpdateResult::Update { - dependency, - new_version, - age_exemption, - .. - } = result - { - if let Some(git) = &dependency.git_source { - let from = match &git.reference { - GitReference::Tag(t) => t.clone(), - _ => git.current_commit.clone().unwrap_or_default(), - }; - Some(JsonUpdate { - age_exemption: age_exemption.clone(), - name: dependency.name.clone(), - kind: "git", - from, - to: new_version.clone(), - dev: dependency.is_dev, - // URL に埋め込まれたアクセストークンを伏せる。 - // fetch 成功時の Update は GitRemoteError を経由しないため、 - // エラー側の伏せ字が効かず、ここが唯一の未伏せ出力経路だった - // (CI の JSON 成果物・ログにトークンが残る)。 - source: Some(crate::registry::redact_url(&git.url)), - reference: Some(JsonGitReference::from_reference(&git.reference)), - }) - } else { - Some(JsonUpdate { - age_exemption: age_exemption.clone(), - name: dependency.name.clone(), - kind: "registry", - // `to` はレジストリが返す生の値なので、`from` も同じ書式 - // (Go なら `v` 付き) に揃える - from: dependency.version_spec.display_version(), - to: new_version.clone(), - dev: dependency.is_dev, - source: None, - reference: None, - }) - } - } else { - None - } - }) + .filter_map(Self::update_to_json) + .collect(); + let failed: Vec = manifest + .failed_updates() + .filter_map(Self::update_to_json) .collect(); let skips: Vec = if self.verbosity == Verbosity::Verbose { @@ -241,6 +259,7 @@ impl JsonFormatter { path: manifest.path.display().to_string(), language: manifest.language.display_name().to_string(), updates, + failed, skips, } } @@ -249,6 +268,7 @@ impl JsonFormatter { impl OutputFormatter for JsonFormatter { fn format(&self, result: &OrchestratorResult, writer: &mut dyn Write) -> std::io::Result<()> { let updates = result.summary.total_updates(); + let failed = result.summary.total_failed(); let skips = result.summary.total_skips(); let by_language: Vec = if self.verbosity == Verbosity::Verbose { @@ -256,10 +276,11 @@ impl OutputFormatter for JsonFormatter { .summary .language_breakdown() .into_iter() - .map(|(language, updates, skips)| JsonLanguageSummary { - language: language.display_name().to_string(), - updates, - skips, + .map(|counts| JsonLanguageSummary { + language: counts.language.display_name().to_string(), + updates: counts.updates, + failed: counts.failed, + skips: counts.skips, }) .collect() } else { @@ -270,6 +291,7 @@ impl OutputFormatter for JsonFormatter { dry_run: result.summary.dry_run, summary: JsonSummary { updates, + failed, skips, by_language, }, @@ -294,12 +316,10 @@ impl OutputFormatter for JsonFormatter { summary: &UpdateSummary, writer: &mut dyn Write, ) -> std::io::Result<()> { - let updates = summary.total_updates(); - let skips = summary.total_skips(); - let output = JsonSummary { - updates, - skips, + updates: summary.total_updates(), + failed: summary.total_failed(), + skips: summary.total_skips(), by_language: Vec::new(), }; @@ -618,9 +638,78 @@ mod tests { let parsed: serde_json::Value = serde_json::from_str(&output_str).unwrap(); assert_eq!(parsed["summary"]["updates"], 0); + assert_eq!(parsed["summary"]["failed"], 0); assert_eq!(parsed["summary"]["skips"], 0); assert!(parsed["manifests"].as_array().unwrap().is_empty()); // エラーがない場合はerrorsフィールドが省略される assert!(parsed.get("errors").is_none() || parsed["errors"].as_array().unwrap().is_empty()); } + + #[test] + fn test_format_json_separates_write_failures() { + // 回帰 (#21): 書き込めなかった更新は `updates` と `summary.updates` に入れず、 + // `failed` に理由付きで入れる。`summary` の件数は各配列の長さの和と一致する + let mut result = create_test_result(); + let manifest = &mut result.summary.manifests[0]; + manifest.add_result(UpdateResult::update( + sample_dependency("react", "18.0.0"), + "19.0.0", + )); + manifest.record_write_error(2, "Failed to update react: package not found"); + + for verbosity in [Verbosity::Normal, Verbosity::Verbose] { + let mut output = Vec::new(); + JsonFormatter::new(verbosity) + .format(&result, &mut output) + .unwrap(); + let parsed: serde_json::Value = serde_json::from_slice(&output).unwrap(); + + assert_eq!(parsed["summary"]["updates"], 1); + assert_eq!(parsed["summary"]["failed"], 1); + assert_eq!(parsed["summary"]["skips"], 1); + let manifest = &parsed["manifests"][0]; + let updates = manifest["updates"].as_array().unwrap(); + assert_eq!(updates.len(), 1); + assert_eq!(updates[0]["name"], "lodash"); + assert!(updates[0].get("error").is_none()); + let failed = manifest["failed"].as_array().unwrap(); + assert_eq!(failed.len(), 1); + assert_eq!(failed[0]["name"], "react"); + assert_eq!(failed[0]["from"], "18.0.0"); + assert_eq!(failed[0]["to"], "19.0.0"); + assert_eq!( + failed[0]["error"], + "Failed to update react: package not found" + ); + if verbosity == Verbosity::Verbose { + assert_eq!(parsed["summary"]["by_language"][0]["updates"], 1); + assert_eq!(parsed["summary"]["by_language"][0]["failed"], 1); + } + } + + let mut output = Vec::new(); + JsonFormatter::new(Verbosity::Normal) + .format_summary(&result.summary, &mut output) + .unwrap(); + let parsed: serde_json::Value = serde_json::from_slice(&output).unwrap(); + assert_eq!(parsed["updates"], 1); + assert_eq!(parsed["failed"], 1); + } + + #[test] + fn test_format_json_always_lists_failed() { + // 書き込めなかった更新が無くても `failed` は空の配列で出す + let mut output = Vec::new(); + JsonFormatter::new(Verbosity::Normal) + .format(&create_test_result(), &mut output) + .unwrap(); + let parsed: serde_json::Value = serde_json::from_slice(&output).unwrap(); + assert_eq!(parsed["summary"]["failed"], 0); + assert!( + parsed["manifests"][0]["failed"] + .as_array() + .unwrap() + .is_empty() + ); + } } diff --git a/src/output/text.rs b/src/output/text.rs index d72f703..840545a 100644 --- a/src/output/text.rs +++ b/src/output/text.rs @@ -173,6 +173,17 @@ impl TextFormatter { .unwrap_or(0) } + /// 書き込めなかった更新の行の末尾に付ける印 + fn write_failed_marker(&self, write_failed: bool) -> String { + if !write_failed { + String::new() + } else if self.color { + format!(" {}", "✗ failed".red()) + } else { + " [failed]".to_string() + } + } + /// 単一の更新行をフォーマット #[allow(clippy::too_many_arguments)] fn format_update_line( @@ -185,11 +196,13 @@ impl TextFormatter { variable_name: Option<&str>, osv_skipped: &[String], osv_checked: bool, + write_failed: bool, max_name_len: usize, writer: &mut dyn Write, ) -> std::io::Result<()> { let change_type = VersionChangeType::from_versions(old_version, new_version); let dev_marker = if is_dev { " 🔧" } else { "" }; + let failed_marker = self.write_failed_marker(write_failed); // リリース日をフォーマット let date_display = released_at @@ -231,7 +244,7 @@ impl TextFormatter { writeln!( writer, - " {} {} {} {} [{}]{}{}{}{}", + " {} {} {} {} [{}]{}{}{}{}{}", name_display, old_version.dimmed(), arrow, @@ -240,12 +253,13 @@ impl TextFormatter { date_colored, var_colored, osv_colored, - dev_display + dev_display, + failed_marker )?; } else { writeln!( writer, - " {:width$} {} -> {} [{}]{}{}{}{}", + " {:width$} {} -> {} [{}]{}{}{}{}{}", name, old_version, new_version, @@ -254,6 +268,7 @@ impl TextFormatter { var_display, osv_marker_plain, dev_marker, + failed_marker, width = max_name_len )?; } @@ -273,15 +288,18 @@ impl TextFormatter { /// git 依存の更新行をフォーマットする /// 例: ` tree-sitter-xojo branch=main f41817b3 → 045c52a6 [git]` + #[allow(clippy::too_many_arguments)] fn format_git_update_line( &self, name: &str, git: &GitSource, new_version: &str, is_dev: bool, + write_failed: bool, max_name_len: usize, writer: &mut dyn Write, ) -> std::io::Result<()> { + let failed_marker = self.write_failed_marker(write_failed); let ref_label = git.reference.display_name(); let (old_display, new_display) = match &git.reference { GitReference::Tag(current) => (current.clone(), new_version.to_string()), @@ -298,24 +316,26 @@ impl TextFormatter { let name_display = format!("{:width$}", name, width = max_name_len); writeln!( writer, - " {} {} {} {} {} {}{}", + " {} {} {} {} {} {}{}{}", name_display, ref_label.cyan(), old_display.dimmed(), "→".dimmed(), new_display.bright_white().bold(), "[git]".dimmed(), - dev_marker.dimmed() + dev_marker.dimmed(), + failed_marker ) } else { writeln!( writer, - " {:width$} {} {} -> {} [git]{}", + " {:width$} {} {} -> {} [git]{}{}", name, ref_label, old_display, new_display, dev_marker, + failed_marker, width = max_name_len ) } @@ -337,8 +357,10 @@ impl TextFormatter { osv_skipped, osv_checked, age_exemption, + write_error, } = result { + let write_failed = write_error.is_some(); // git 依存は専用フォーマットで表示 if let Some(git) = &dependency.git_source { self.format_git_update_line( @@ -346,6 +368,7 @@ impl TextFormatter { git, new_version, is_dev, + write_failed, max_name_len, writer, )?; @@ -369,6 +392,7 @@ impl TextFormatter { dependency.variable_name.as_deref(), osv_skipped, *osv_checked, + write_failed, max_name_len, writer, )?; @@ -392,8 +416,8 @@ impl TextFormatter { ) -> std::io::Result<()> { let prefix = self.dry_run_prefix(); - // 更新とスキップを収集 - let updates: Vec<_> = manifest.updates().collect(); + // 更新とスキップを収集。行には書き込めなかった更新も印を付けて並べる + let updates: Vec<_> = manifest.judged_updates().collect(); let skips: Vec<_> = manifest.skips().collect(); // 更新もスキップもない完全に空のマニフェストを除外 @@ -401,8 +425,9 @@ impl TextFormatter { return Ok(()); } - // 更新数とスキップ数をカウント - let update_count = updates.len(); + // 更新数とスキップ数をカウント。更新数には書き込めなかった更新を含めず、別に数える + let update_count = manifest.update_count(); + let failed_count = manifest.failed_count(); let skip_count = skips.len(); // 更新なしでスキップありの場合、スキップサマリを表示 (非 verbose モードでも) @@ -439,12 +464,21 @@ impl TextFormatter { } }); - // カウント付きマニフェストヘッダを書き出す + // カウント付きマニフェストヘッダを書き出す。書き込めなかった更新があるときだけ + // その件数も出す let path_display = manifest.path.display().to_string(); let lang_display = format!("({})", manifest.language); + let failed_display = if failed_count > 0 { + format!( + ", {} failed", + self.apply_color(&failed_count.to_string(), Color::Red) + ) + } else { + String::new() + }; writeln!( writer, - "{}{} {} — {} {}, {} {}", + "{}{} {} — {} {}{}, {} {}", prefix, self.maybe_bold(&path_display), self.maybe_dimmed(&lang_display), @@ -454,6 +488,7 @@ impl TextFormatter { } else { "updates" }, + failed_display, self.maybe_dimmed(&skip_count.to_string()), if skip_count == 1 { "skip" } else { "skips" } )?; @@ -703,17 +738,28 @@ impl OutputFormatter for TextFormatter { writer: &mut dyn Write, ) -> std::io::Result<()> { let prefix = self.dry_run_prefix(); + // 更新数には書き込めなかった更新を含めず、別に数える let updates = summary.total_updates(); + let failed = summary.total_failed(); let skips = summary.total_skips(); if self.verbosity == Verbosity::Quiet { // 最小限の出力 - if updates > 0 { + if updates > 0 || failed > 0 { + let failed_display = if failed > 0 { + format!( + ", {} failed", + self.apply_color(&failed.to_string(), Color::Red) + ) + } else { + String::new() + }; writeln!( writer, - "{}{} updated", + "{}{} updated{}", prefix, - self.apply_color(&updates.to_string(), Color::Green) + self.apply_color(&updates.to_string(), Color::Green), + failed_display )?; } else { writeln!(writer, "{}{}", prefix, self.maybe_dimmed("No updates"))?; @@ -756,6 +802,15 @@ impl OutputFormatter for TextFormatter { writeln!(writer, " {}", self.maybe_dimmed("No packages updated"))?; } + // 書き込めなかった更新 (理由は Errors 欄に出る) + if failed > 0 { + writeln!( + writer, + " {} package(s) failed to update", + self.apply_color(&failed.to_string(), Color::Red) + )?; + } + // スキップサマリ if skips > 0 { write!( @@ -780,18 +835,27 @@ impl OutputFormatter for TextFormatter { if self.verbosity == Verbosity::Verbose { writeln!(writer)?; writeln!(writer, "{}:", self.maybe_dimmed("By language"))?; - for (language, lang_updates, lang_skips) in summary.language_breakdown() { + for counts in summary.language_breakdown() { let lang_name = if self.color { - language.to_string().cyan().to_string() + counts.language.to_string().cyan().to_string() } else { - language.to_string() + counts.language.to_string() + }; + let failed_display = if counts.failed > 0 { + format!( + ", {} failed", + self.apply_color(&counts.failed.to_string(), Color::Red) + ) + } else { + String::new() }; writeln!( writer, - " {}: {} updated, {} skipped", + " {}: {} updated{}, {} skipped", lang_name, - self.apply_color(&lang_updates.to_string(), Color::Green), - self.maybe_dimmed(&lang_skips.to_string()) + self.apply_color(&counts.updates.to_string(), Color::Green), + failed_display, + self.maybe_dimmed(&counts.skips.to_string()) )?; } } @@ -1029,6 +1093,114 @@ mod tests { assert!(output_str.contains("No updates")); } + /// 書き込めなかった更新を含む結果。`failed` に挙げた名前の更新を書き込めなかったことにする + fn create_result_with_write_failures(failed: &[&str]) -> OrchestratorResult { + let mut summary = UpdateSummary::new(false); + let mut manifest = ManifestUpdateResult::new(PathBuf::from("package.json"), Language::Node); + // 本番依存 - マイナー更新 + manifest.add_result(UpdateResult::update( + sample_dependency("lodash", "4.17.21", false), + "4.18.0", + )); + // 本番依存 - メジャー更新 + manifest.add_result(UpdateResult::update( + sample_dependency("react", "18.0.0", false), + "19.0.0", + )); + // 開発依存 - パッチ更新 + manifest.add_result(UpdateResult::update( + sample_dependency("typescript", "5.0.0", true), + "5.0.1", + )); + manifest.add_result(UpdateResult::skip( + sample_dependency("express", "4.18.0", false), + SkipReason::AlreadyLatest, + )); + for index in 0..manifest.results.len() { + if failed.contains(&manifest.results[index].package_name()) { + let reason = format!( + "Failed to update {}", + manifest.results[index].package_name() + ); + manifest.record_write_error(index, reason); + } + } + summary.add_manifest(manifest); + + OrchestratorResult { + summary, + write_results: Vec::new(), + errors: Vec::new(), + } + } + + fn format_text(result: &OrchestratorResult, verbosity: Verbosity) -> String { + let formatter = TextFormatter::with_color(verbosity, false, false); + let mut output = Vec::new(); + formatter.format(result, &mut output).unwrap(); + String::from_utf8(output).unwrap() + } + + #[test] + fn test_format_counts_write_failures_apart_from_updates() { + // 回帰 (#21): 書き込めなかった更新は見出しと Summary の更新数に入れず、 + // 別に数える。一覧の行には印を付けて残す + let result = create_result_with_write_failures(&["react", "typescript"]); + + let output = format_text(&result, Verbosity::Normal); + assert!(output.contains("— 1 update, 2 failed, 1 skip"), "{output}"); + let line = |name: &str| { + output + .lines() + .find(|line| line.trim_start().starts_with(name)) + .unwrap_or_else(|| panic!("{name} の行が無い: {output}")) + .to_string() + }; + assert!(!line("lodash").contains("[failed]")); + assert!(line("react").ends_with("[major] [failed]"), "{output}"); + assert!(line("typescript").ends_with("🔧 [failed]"), "{output}"); + assert!( + output.contains("1 package(s) updated (1 minor)"), + "{output}" + ); + assert!(output.contains("2 package(s) failed to update"), "{output}"); + + let quiet = format_text(&result, Verbosity::Quiet); + assert_eq!(quiet.trim(), "1 updated, 2 failed"); + + let verbose = format_text(&result, Verbosity::Verbose); + assert!( + verbose.contains(": 1 updated, 2 failed, 1 skipped"), + "{verbose}" + ); + } + + #[test] + fn test_format_shows_manifest_whose_updates_all_failed() { + // 全更新を書き込めなかったマニフェストも、スキップの要約にせず更新の行を出す + let result = create_result_with_write_failures(&["lodash", "react", "typescript"]); + + let output = format_text(&result, Verbosity::Normal); + assert!(output.contains("— 0 updates, 3 failed, 1 skip"), "{output}"); + assert_eq!(output.matches("[failed]").count(), 3, "{output}"); + assert!(output.contains("No packages updated"), "{output}"); + assert!(output.contains("3 package(s) failed to update"), "{output}"); + + let quiet = format_text(&result, Verbosity::Quiet); + assert_eq!(quiet.trim(), "0 updated, 3 failed"); + } + + #[test] + fn test_format_without_write_failures_keeps_counts_unchanged() { + // 書き込めなかった更新が無いときは、件数の表示を変えない + let result = create_result_with_write_failures(&[]); + + let output = format_text(&result, Verbosity::Normal); + assert!(output.contains("— 3 updates, 1 skip"), "{output}"); + assert!(!output.contains("failed"), "{output}"); + assert_eq!(format_text(&result, Verbosity::Quiet).trim(), "3 updated"); + } + #[test] fn test_count_by_change_type() { let formatter = TextFormatter::new(Verbosity::Normal, false);