reviewing-motoko

Motoko のレビュー

このスキルでは、Motoko のコードが適切に設計されているかを判断します。言語を教えるものではありません。構文、mo:core の API、機械的なルールについては writing-motoko を、安定状態を変更する前には migrating-motoko-actors を読み込んでください。

レビューでは、次の 4 つの問いをこの順に確認します。最初の 3 つは、それぞれ後続の問いを無効にする可能性があるためです。4 つ目は影響が最も小さいため、最後に確認します。

1. コードが依存するすべての不変条件は、型にエンコードされていますか？ 型システムで不正な状態を排除できるのに、それを実行時チェックやコメントに委ねている場合、スタイルの好みではなく欠陥です。
2. 状態は適切な永続性を指定して宣言されていますか？ 拡張移行では、安定状態と一時状態は別々の世界です。両者を曖昧にすると、状態の更新がいつの間にか止まります。
3. コードは適切なファイルにありますか？ types.mo / lib/ / mixins/ / main.mo は、何が何に依存してよいかを定める契約であり、単なるフォルダーの慣習ではありません。
4. Motoko らしいコードになっていますか？ 文脈に応じたドット記法、推論される暗黙の引数、呼び出し側ですでに指定されている注釈を省くこと、クラスを使わないことです。ほとんどは mops check --fix で対応できます。確認できない部分だけがレビュー担当者の仕事です。

手順

まず機械的なチェックを実行してください。コンパイラーがすでに報告する内容に、レビューの注意を費やさないでください。

mops check --fix

スタイルの自動修正（M0236 のドット記法、M0237 の冗長な暗黙の引数、M0223 の冗長なインスタンス化）は、[moc] args で -W が有効になっている場合にのみ実行されます。設定方法については writing-motoko の references/project-setup.md を参照してください。無効になっている場合、以下の規約に関する指摘はすべて手作業で対応してください。

次に、以下の順に読み進めます。ファイルの配置が不適切だと、その中の型すべてが疑わしくなるため、外側から確認します。

1. mops.toml — [canisters.*.migrations] チェーンがありますか？なければ、以下の指摘 B1 を最優先にしてください。
2. 構成 — ファイルの配置を確認し、次に各ファイルのインポートを確認します。
3. types.mo — すべての型宣言を確認します。指摘事項の大半はここにあります。
4. 状態 — main.mo のアクターフィールドと、それぞれの include が渡す内容を確認します。references/state-and-persistence.md を参照してください。
5. API 境界 — すべての public shared / public query 関数について、パラメーター型、戻り値の型、認可を確認します。
6. 本体 — 最後に確認します。対象は、以下の保守性に関する問題だけです。

深刻度

指摘には、次の 3 段階のいずれかを付けてください。大げさに評価しないでください。何もかもブロッカーと呼ぶレビュー担当者は、相手にされなくなります。

レベル	意味
ブロッカー	状態を破損させたり、気づかないうちに失わせたりする可能性がある、または呼び出し元が未認可の操作を実行できる
警告	型システムで強制できる不変条件を、実行時処理、コメント、または慣習に委ねている
注記	可読性、重複、または命名に関する問題です。そのファイルを開いている間に修正し、承認の条件にはしないでください

ルールではなく、具体的に何が起きるかを報告してください。「status : Text は "complete" を受け付けますが、3 か所の呼び出し側では "completed" と記述されています」は、「文字列よりバリアントを使うべきです」より適切です。

型によるエンコード

原則は次のとおりです。値の形が決まった集合に限られるなら、型もその集合の形を持つ必要があります。 変更を移行に反映する方法も含めた詳しい手順は、references/type-encoding.md を参照してください。

T1. 閉じた値の集合に Text を使用 — Warning、認可を左右する場合は Blocker

生成された Motoko で最もよく見られる不具合です。すべての生成側と利用側が慣例によって綴りに合意していても、コンパイラーは何もチェックしません。網羅性のチェックも、タイプミスのエラーも、新しい状態が追加されたときに呼び出し箇所の半分が更新されていないことを知らせるエラーもありません。

public type Task = { id : Nat; var status : Text };           // WRONG
public type TaskStatus = { #open; #claimed : Principal; #done : { at : Time.Time } };

バリアントは共有できるため、フロントエンドにも判別共用体として届きます。コード、コメント、ドキュメントのいずれかに許可される値が列挙されている場合は、Text（または Nat、Bool）に フラグを立ててください。== "..." による比較、case _ を含む Text の switch、Text で保持されるロール名などが該当します。Text は、本当に値が開かれている集合や、外部システムが所有する文字列に使ってください。

T2. 相関するフィールド — Warning

一緒にしか有効にならない2つのフィールドは、1つのバリアントにまとめてください。Bool とオプショナルなペイロードの組み合わせでは、表現可能な組み合わせが4つあり、そのうち意味があるのは2つだけです。

// WRONG — { hasDeadline = true; deadline = null } compiles and means nothing
{ hasDeadline : Bool; deadline : ?Time.Time }

// RIGHT
{ deadline : { #none; #due : Time.Time } }   // or simply `deadline : ?Time.Time`

?T ですでに表現できている場合は、Bool を削除してください。?T を言い換えるだけのバリアントを導入しないでください。

T3. 要素数の型が不適切 — Note、呼び出し側が防御的にループする場合は Warning

実際の要素数に合った型を使ってください。常に0個か1個の [T] では、すべての呼び出し側でループを書くか if size == 1 とする必要がありますが、?T なら反復処理が不要であることが伝わります。

T4. エラーに Text を使う — Warning

Result<Ok, Text> では、呼び出し側が失敗の種類を判別するたびに文字列を照合しなければなりません。各ケースに必要なデータを持つバリアントを Err に使ってください。?T を返す場合や、代わりにトラップする場合も含む完全なルールについては、writing-motoko の Result セクションを参照してください。

T5. var フィールドが API に漏れている — Blocker（コンパイルできない場合）または Warning（コンパイルできるが公開範囲が広すぎる場合）

var フィールドを持つレコードは共有できないため、公開関数から返せません。プロジェクトに内部型とビュー型の分離（内部型の Task、共有型の TaskView）がすでにある場合は、次の点を確認してください。

• 公開シグネチャに内部型が含まれていないこと。
• ビュー型が内部レコードの機械的なフィールド単位のコピーではなく、内部型より限定されていること。内部の Principal リスト、監査証跡、または他のユーザーのデータを再公開するビューは、コンパイルできてしまう情報漏えいです。

T6. case _ がバリアントを握りつぶす — Warning

網羅的に列挙することで「バリアントタグの追加は、必要なすべての箇所でコンパイルエラーになる」はずが、「バリアントタグを追加しても、デフォルトの分岐が黙って実行される」ことになってしまうのが、包括的な分岐です。各分岐が本当に同じ処理を行い、集合が増えることを想定している場合（たとえば、多数のタグを1つのラベルに対応付ける場合）に限り、case _ を使ってください。新しいタグに新しい動作が必要となる箇所では、タグを列挙してください。

T7. 安全性として扱われている透過的な型エイリアス — Note

type UserId = Principal はドキュメント上のエイリアスです。UserId が期待される場所では、どの Principal でも型チェックに通ります。これは問題なく、通常は適切なトレードオフです。ただし、強制される区別として扱わないでください。異なる2つの ID 型が呼び出し箇所で本当に混同されている場合、実際にそれを防ぐのは1フィールドのレコード（{ userId : Principal }）かバリアントタグ（#user(Principal)）です。ただし、Candid の形状を変える必要があります。

状態と永続化

拡張マイグレーション（EM）では、アクターの安定フィールドは 型のみ で宣言され、初期値はすべてマイグレーションチェーンから取得されます。この分離が重要です。これにより、安定状態を一時的な値から初期化できなくなります。詳細は references/state-and-persistence.md を参照してください。

B1. 拡張マイグレーションがない — Blocker

mops.toml に [canisters.<name>.migrations] チェーンがない場合、拡張マイグレーション導入前の永続化が使われます。この方式では安定フィールドにインライン初期化子を記述するため、安定フィールドを一時的な値から初期化できます。

transient let defaultQuota = 100;
let quota = defaultQuota;   // stable; the initializer runs ONCE, on first install

defaultQuota を 250 に変更してデプロイしても、quota は警告なしにいつまでも 100 のままです。変換方法は migrating-motoko-actors を参照してください。以下の内容はすべて、拡張マイグレーションを前提としています。

S1. 安定フィールドのインライン初期化子 — ブロッカー

EM では M0250 / M0014 に該当するため、コンパイルできません。ただし、EM 以前の例からコピーしたコードや、未完了のリファクタリングでは頻繁に見られます。修正のために transient を追加してはいけません。初期値を保留中のマイグレーションの NewActor に移してください。

S2. transient の誤用 — どちらの場合も 警告

すべてのアクターとミックスインのフィールドについて、次の点を確認してください。アップグレード時に失われても、アプリは正しく動作しますか？

• transient の付け忘れ：キャッシュ、派生インデックス、レート制限カウンターなど、再構築可能なものが永続化されています。その結果、永久にマイグレーションし続ける必要が生じ、キャッシュをクリアするはずだったアップグレード後も古いキャッシュが残ります。
• 誤った transient の指定：永続化すべきドメインデータが、アップグレードのたびに初期化子の値へ暗黙にリセットされます。デプロイ後もユーザーが保持されていると期待するデータに transient が付いていないか確認してください。

S3. ミックスインと共有する var アクターフィールド — ブロッカー

var パラメーターは値渡しのため、ミックスインが変更するのはコピーであり、アクターのフィールドは変化しません。レコードでラップして、そのレコードを渡してください。また、include Api({ var n = state.n }) のように include 箇所で新しいレコードリテラルを指定している場合も指摘してください。ミックスインごとに個別のコピーが作られます。writing-motoko を参照してください。

S4. mixin ブロック内の安定状態 — ブロッカー

mixin のトップレベルにあるすべての裸の let／var は暗黙に安定状態となり、実行時に IC0503 でトラップします。状態はパラメーターとして受け取り、定数はモジュールに置き、インラインで許可されるのは transient のみです。

S5. マイグレーションの衛生管理 — ブロッカー

• mo:core/... 以外をインポートするマイグレーションファイル。チェーンは繰り返し再生されるため、プロジェクトのインポートがあると、その型が変更された時点で誤りになります。（caffeineai-lints が検出します。）
• ビルド内に保留中のマイグレーションが複数ある、または適用済みのマイグレーションを編集している。
• main.mo の安定フィールドを、チェーン内のどのマイグレーションも提供していない（M0254 / M0267）。

恒等マイグレーション本体は 警告 です。変更は安定互換だったため、ファイルを削除してください。完全な表。

構成

レイヤー間の依存関係は一方向のみです。main.mo → mixins/ → lib/ → types.mo。

ファイル	含むもの	含めてはいけないもの
types.mo	型宣言、その型に関する小規模な純粋ヘルパー	状態、エンドポイント、ビジネスルール
lib/*.mo	ステートレスなモジュールとしてのドメインロジック、パラメーターとして渡す状態	パブリックエンドポイント、アクターフィールド
mixins/*.mo	パブリックエンドポイント：認可、委譲、ビューへの変換	ドメインロジック、安定状態
main.mo	状態宣言と includes	あらゆるパブリックメソッド、あらゆるロジック
migrations/	凍結されたチェーン、ファイルごとに1つのモジュール	プロジェクトのインポート

• A1 — ブロッカー／警告： main.mo にパブリックメソッドがある。ミックスインに移してください。
• A2 — 警告： モノリシックなファイル。型、状態、ロジック、エンドポイントが1つの .mo にまとめられている。
• A3 — 警告： ミックスイン本体にドメインロジックがある。エンドポイントの本体が認可 → 委譲 → 変換を超える場合、中央部分は lib/ に置いてください。
• A4 — 警告： 依存関係の方向が誤っている。lib/ がミックスインをインポートしている、または types.mo がそのどちらかをインポートしている。
• A5 — 注意： 機能単位ではないミックスイン。ミックスインは動詞（mixins/Getters.mo）ではなく、機能（mixins/Bookings.mo）ごとに分割します。
• A6 — 警告： 状態の注入範囲が広すぎる。使用しない状態まで受け取るミックスインは、変更によって影響を受ける範囲を広げます。使用する部分だけを渡してください。

保守性

この大半は、最後の本体パスで検出されます。例外は認可を阻む問題である M1 です。これは、以下の本体に関する問題を確認する段階ではなく、API 境界の確認（ステップ 5）で検出します。

• M1 — ブロッカー： caller をチェックしない public shared 関数です。すべての状態変更と、他のユーザーのデータの読み取りには、バックエンドでの認可が必要です。
• M2 — 警告： 演算時に非負であることを証明できない Nat の減算です。トラップします。
• M3 — 警告： 呼び出し元が回復できる場面でのトラップです。トラップするとメッセージがロールバックされ、不透明な拒否としてクライアントに届きます。呼び出し元が修正できる失敗は、Result のエラーバリアントに含めてください。
• M4 — 注記： 内部表現からビューへのマッピングが重複しています。lib/ または types.mo に、型ごとに 1 つの toView を用意してください。
• M5 — 注記： マジック値です。上限、クォータ、期間に関する名前のないリテラルを指します。モジュール内で名前を付けてください。
• M6 — 注記： 型で表現できる不変条件をコメントで主張しています。「空であってはならない」や「…のときだけ設定する」というコメントは、型で表現できる問題を別の形で示したものです。Motoko で不変条件を表現できない場合は、コメントを残したうえで、すべての使用箇所で再チェックするのではなく、境界で値を 1 回検証してください。

規約

4 つの観点の中で最も影響が小さく、-W フラグが有効な場合は mops check --fix で大半を修正できるため、上記の確認を終えてから、ツールで対応できない箇所に限って指摘してください。ルールは writing-motoko に記載されています。確認する点は以下のとおりです。

• C1 — 注記： 関数が self を受け取る場合のモジュール関数呼び出しです（List.add(list, x)、Principal.toText(p)）。M0236 により自動修正されます。
• C2 — 注記： 明示的に渡された暗黙値（map.add(Text.compare, k, v)）、またはすでに解決済みの型インスタンス化推論です。M0237 / M0223 により自動修正されます。
• C3 — 警告： 呼び出し引数として渡されるインライン func の型注釈です（xs.filter(func(x : Nat) : Bool { x > 1 })）。これを検出する診断はないため、レビュアーが確認する必要があります。呼び出し側ですでに両方の型が決まっており、注釈を付けると重複して、型が食い違う可能性が生じます。xs.filter(func x = x > 1) と記述してください。M0098 が求める場合は、ラムダではなく 呼び出し をインスタンス化してください（map<In, Out>）。例外として、非同期コールバックの : async () は動作上必要です。
• C4 — ブロッカー： class または関数を保持するオブジェクトをアクターの状態として使用しています。関数は安定ではないため、M0131（declared stable but has non-stable type）で失敗します。状態以外でも、ここではクラスは慣用的な書き方ではありません。self を受け取るモジュールとドット記法を使えば、同じ呼び出し方を維持しながら安定性を保てます。

リファクタリング

Motoko での設計修正は無料ではありません。安定アクターフィールドから到達できる型を変更するには移行が必要で、その変更の連鎖は一方向です。型の変更を提案する前に、その型が安定状態かどうかを確認してください。

1. まず mops check --fix を実行し、機械的な差分をなくします。
2. 安定状態の型変更をまとめます — ビルドごとに保留できる移行は 1 件だけなので、まとめて反映してください。
3. 移行を記述します（migrating-motoko-actors）。Text からバリアントへの変更では、#unknown タグを追加するのではなく、認識できない文字列の扱いを明示的に決めてください。
4. 変更する前にコードを移動します — 先に移動し、その後に編集することで、差分ごとにレビューしやすくなります。
5. mops check --fix をエラーがなくなるまで実行し、その後 mops build を 1 回実行します。

報告

重大度ごとにまとめ、ブロッカーを最初に記載してください。各指摘には、file:line、レベル、許容される具体的な失敗、変更内容を記載します。コンパイラーがすでに報告している指摘は省略してください。

Blocker  src/backend/mixins/Tasks.mo:34  (M1)
  `claimTask` never reads `caller` — any principal can claim any task.

Warning  src/backend/types.mo:12  (T1)
  `Task.status : Text` is a closed set, so the comparison at Tasks.mo:51
  spells it "complete" and is never true. Stable state — needs a migration.

問題がなければ、その旨を明確に伝えてください。報告を埋めるために指摘を作り出さないでください。

追加の参考資料

• references/type-encoding.md — 不変条件ごとのレシピと、それぞれに必要な移行
• references/state-and-persistence.md — EM が重要な理由、安定状態と一時状態に関する検討事項、移行時の作法
• writing-motoko — 言語の仕組み、mo:core API、Result、ドット記法
• mops ツール：mops.toml の設定、および mops check／mops build の詳細を確認するには、mops-cli を読み込みます
• migrating-motoko-actors — 型の変更で必要になる移行の記述方法
• troubleshooting-motoko-migrations — ソースと一致しない互換性診断