IBM Bob Premium Package for i で RPG 専用のコードレビューを行う /review_rpg コマンドが用意されていたので、実際にライブラリーの RPG ソースに対してレビューを行ってみた内容を備忘録代わりに記載します。
サマリー
- /review_rpg は、RPG のソースメンバーまたはストリームファイルに対して、構文・コーディング標準・エラー処理・セキュリティー・モダナイゼーションの観点でコードレビューを行うコマンドです
- 実行すると、Bob は RPG レビュー用のスキル（
rpg-primer-basics、rpg-code-review）を読み込んでからレビューを行います - 今回は固定形式の ILE RPG 14 メンバーを対象にして、Critical 6 件、Major 9 件、Minor 8 件 の指摘と、優先順位付きのモダナイゼーションロードマップが返ってきました
-
ACTGRP(*NEW)の指摘のように、IBM i に特化した観点でのレビューが得られます。現時点では Bob Findings パネルとは連携していません
はじめに
/review_rpg とは
/review_rpg コマンドは、RPG のソースメンバーまたはストリームファイルに対して、専用のコードレビューを実行するよう Bob に指示するコマンドです。構文、言語のベストプラクティス、コーディング標準、エラー処理、セキュリティー、および固定形式（fixed-form）とフリー形式（free-form）の両方の RPG にわたるモダナイゼーションの機会を評価します。
| 項目 | 内容 |
|---|---|
| Command | /review_rpg |
| Argument | [source member or stream file path] |
| Available in | IBM i Developer |
| Requires | Active IBM i connection |
Bob は下記の手順で処理します。
- 対象ファイルが有効な RPG ソース（
.RPGLE、.SQLRPGLE、.RPG、.RPG36、.RPG38など）であることを検証します。CL、COBOL、DDS、純粋な SQL など RPG 以外のソースは、レビュールールが読み込まれる前に除外されます - RPG ソースの形式（固定形式、フリー形式、または混在）を判別し、適切なレビュースキルを読み込みます
- 下記の観点で RPG コードを評価します
| 観点 | 内容 |
|---|---|
| 構文とフォーマットルール | 仕様書の列の配置、コメントのフォーマット、サブプロシージャーの配置 |
| コーディング標準 | 命名規則、インデント、構造 |
| エラー処理と信頼性 | モニターブロック、状況の確認、ファイルエラーの処理 |
| セキュリティー | 権限の借用（adopted authority）、動的操作でのインジェクションのリスク |
| パフォーマンスとデータベース操作 | RLA と組み込み SQL、ブロッキング、カーソル管理 |
| モダナイゼーションの機会 | 固定形式の構造の変換、標識の排除、サブルーチンからサブプロシージャーへのリファクタリング |
結果は下記の構成で返されます。
| 項目 | 内容 |
|---|---|
| Detected Format | ソースの種類と形式の分類 |
| Summary | レビュー全体の評価の要約 |
| Critical Issues | 重大な構文、セキュリティー、または実行時の欠陥 |
| Major Issues | 構造的な欠陥、パフォーマンスのボトルネック、未処理のエラー |
| Minor Issues | スタイルの不一致や軽微なクリーンアップ |
| Modernization Roadmap | RPG コードをモダナイズするための具体的なステップ |
使用例は下記になります。引数なしで実行することも、ソースメンバーや IFS のストリームファイルを指定することもできます。
/review_rpg
/review_rpg MYLIB/QRPGLESRC/MYPGM.RPGLE
/review_rpg /home/user/src/order_processing.rpgle
/review_rpg /user/ibmi_local/FLIGHT400/QRPGLESRC/FRS400.rpgle
/review_rpg を実行する
レビューを依頼する
今回は IMAO1399 ライブラリーの RPG ソースを対象にレビューを行います。チャット欄で IMAO1399 を指定し、下記のように /review_RPG を入力して Enter を押下します。
実行すると、用意されている RPG レビュー用のスキル（rpg-primer-basics、rpg-code-review）を読み込んでから、IMAO1399/QRPGLESRC のメンバーを確認してレビューを進めていることがわかります。
レビュー結果を確認する
レビューが完了すると、下記のレビュー報告がチャット欄に出力されました。特に指定しない場合はチャット欄への出力になりますが、Bob に依頼すれば IFS へファイルとして出力することも可能です。
実際に出力されたレビュー報告（クリックで展開）
# IMAO1399 RPGソースコード レビュー報告
## 検出フォーマット
全14メンバーとも **固定形式（Fixed-Format）ILE RPG** です（H/F/D/C スペック、`/FREE` ブロックなし）。`/EXEC SQL ... /END-EXEC` による埋め込みSQL混在あり。
---
## 総合評価
動作するデモアプリとして一定の完成度がありますが、**プロダクション品質には Critical・Major 両面で課題**があります。特に「コミットメント制御の整合性」「ハードコードされた監査ユーザー」「SQL SQLCODE 未チェック」の3点は早急な対処が必要です。
---
## Critical Issues（本番使用前に必ず修正）
---
### C-1. `ORDERENTRY` H仕様: `ACTGRP(*NEW)` — パフォーマンスとトランザクション上の危険性
**対象**: `ORDERENTRY.RPGLE` (1行目), `PAYENTRY.RPGLE` (1行目)
```rpg
H DFTACTGRP(*NO) ACTGRP(*NEW) ← 問題
```
`ACTGRP(*NEW)` は呼び出しのたびに新しい活動化グループを作成します。
- 呼び出し元（`CNFSHPENT`）でコミットメント制御が開始されていても、`*NEW` は **別スコープ** で動作するため、既存のトランザクションに参加できません。
- 5250 対話型プログラムで `*NEW` を使うと、セッションごとにリソースが増え続けます。
**修正方法**:
```rpg
H DFTACTGRP(*NO) ACTGRP(*CALLER)
```
---
### C-2. `ORDERENTRY` `@SAVEORD`: 在庫引当が **コミットメント制御外**
**対象**: `ORDERENTRY.RPGLE` (603行目〜)
`@SAVEORD` では `INVENTORYP` の UPDATE / `SORDERHP` の WRITE / `SORDERLP` の WRITE を連続実行しますが、ファイル F仕様に **`COMMIT` キーワードがありません**。中断時にデータが不整合になります（在庫だけ減算されて受注レコードが存在しないなど）。
```rpg
* 現状（問題あり）
FSORDERHP O E DISK PREFIX(SO_)
FSORDERLP O E DISK PREFIX(SL_)
FINVENTORYPUF E K DISK
* 修正方法
FSORDERHP O E DISK PREFIX(SO_) COMMIT
FSORDERLP O E DISK PREFIX(SL_) COMMIT
FINVENTORYPUF E K DISK COMMIT
```
そして `@SAVEORD` の末尾に成功時 `COMMIT`、エラー時 `ROLBK` を追加する必要があります。
---
### C-3. `ORDERENTRY` `@SAVEORD` 668行目: 監査ユーザーのハードコード
**対象**: `ORDERENTRY.RPGLE` (668行目)
```rpg
C EVAL SO_CRTUSER = 'DSPFUSER' ← Critical
C EVAL SO_UPDUSER = 'DSPFUSER'
```
他のプログラム（`CREATESORD`、`CREATEINV`、`CONFIRMSHP`）はSQLの `USER` 特殊レジスタや PSDS で実行ユーザーを取得しているのに、`ORDERENTRY` だけが文字列リテラル `'DSPFUSER'` をハードコードしています。誰が操作したか追跡できない監査上の重大な欠陥です。
```rpg
* 修正方法 - @INIT または @SAVEORD 先頭に追加
C/EXEC SQL
C+ VALUES USER INTO :W#USER
C/END-EXEC
* そして
C EVAL SO_CRTUSER = W#USER
C EVAL SO_UPDUSER = W#USER
```
---
### C-4. `ORDERENTRY` `@SAVEORD`: SOID をタイムスタンプ採番（衝突リスク）
**対象**: `ORDERENTRY.RPGLE` (605行目)
```rpg
C EVAL W#SOID = (WDATE8 * 1000000) + WTIME6
```
`CREATESORD` はコメント「修正#R1」でSQLシーケンスに変更済み（`CREATESORD.RPGLE:129`）なのに、`ORDERENTRY` はまだタイムスタンプ採番のままです。同秒内に複数セッションが受注すると SOID が重複します。
```rpg
* 修正方法
C/EXEC SQL
C+ VALUES NEXT VALUE FOR IMAO1399/O2C_SOID_SEQ INTO :W#SOID
C/END-EXEC
```
---
### C-5. `PAYENTRY` `@SAVEPAY`: SQL文後の `SQLCODE` 未チェックが複数箇所
**対象**: `PAYENTRY.RPGLE` (488行目〜)
```rpg
* UPDCSR DECLARE/OPEN
C/EXEC SQL
C+ DECLARE UPDCSR ... ← SQLCODE チェックなし
C/END-EXEC
C/EXEC SQL
C+ OPEN UPDCSR ← SQLCODE チェックなし
C/END-EXEC
* UPDATE WHERE CURRENT OF
C/EXEC SQL
C+ UPDATE INVOICEHP ... WHERE CURRENT OF UPDCSR ← SQLCODE チェックなし（517行）
C/END-EXEC
* INSERT INTO PAYAPPLYP
C/EXEC SQL
C+ INSERT INTO PAYAPPLYP ... ← SQLCODE チェックなし（531行）
C/END-EXEC
```
これらは失敗しても `W#ERROR` がセットされず、誤ったデータが登録・コミットされます。各 `EXEC SQL` の直後に `IF SQLCODE < 0` チェックが必要です。
---
### C-6. `CREATEINV` / `CREATESORD`: H仕様に `DFTACTGRP(*NO)` / `ACTGRP` がない
**対象**: `CREATEINV.RPGLE` (1行目), `CREATESORD.RPGLE` (1行目)
```rpg
H DATEDIT(*YMD) DECEDIT('0.') COPYRIGHT('DEMOAPP')
* DFTACTGRP(*NO) も ACTGRP も指定なし
```
`DFTACTGRP(*NO)` がない場合、コンパイラはデフォルト活動化グループ（OPM互換）で動作する可能性があります。`COMMIT` / `ROLBK` の動作が意図と異なる恐れがあります。
```rpg
H DATEDIT(*YMD) DECEDIT('0.') COPYRIGHT('DEMOAPP')
H DFTACTGRP(*NO) ACTGRP(*CALLER)
```
---
## Major Issues（技術的負債・修正を強く推奨）
---
### M-1. 全プログラム: `BEGSR`/`ENDSR` サブルーチン → サブプロシージャ化
**対象**: 全14メンバー共通
`@INZ`、`@VALIDHDR`、`@PRCLINE` など全てのロジックが `BEGSR`/`ENDSR` で実装されています。ILEプログラムでは `DCL-PROC`/`END-PROC` サブプロシージャに変換することで、ローカルスコープ・パラメータ型チェック・再利用性が向上します。
> **例外**: `*PSSR`、`*INZSR`、`INFSR` リンクのエラーサブルーチンはシステム仕様上 `BEGSR` のままにすること。本コードベースにはこれらは存在しないため、全て移行候補です。
---
### M-2. 全プログラム: 結果標識（`*IN99`、`*IN31`〜`*IN43` 等）による制御
**対象**: `ORDERENTRY.RPGLE` (252行目), `CREATESORD.RPGLE` (209行目), `CONFIRMSHP.RPGLE` (188行目)
```rpg
C INVKEY CHAIN INVENTORYP 99
C IF *IN99 ← Major
```
`(E)` 拡張子 + `%FOUND()` / `%ERROR()` / `%STATUS()` への移行が推奨です。指標番号の衝突や意味の追跡が困難になります。
```rpg
* 修正例
C INVKEY CHAIN(E) INVENTORYP
C IF NOT %FOUND(INVENTORYP)
```
---
### M-3. `ORDERENTRY` / `PAYENTRY`: INFDS（ファイル情報データ構造）が未定義
F仕様にどのファイルも `INFDS()` キーワードがありません。ファイルエラー時の詳細ステータス（エラーコード・レコード名・操作コード）が取得できません。
---
### M-4. `CONFIRMSHP` `@GENSHPID`: SHPID をタイムスタンプ採番（衝突リスク）
**対象**: `CONFIRMSHP.RPGLE` (123行目)
```rpg
C EVAL W#SHPID = (WDATE8 * 1000000) + WTIME6
```
SOID と同様に、同秒内の出荷で衝突します。SQLシーケンスへの変更を推奨します。
---
### M-5. `CONFIRMSHP` `@VLDSOH`: `SORDERHP` を COMMIT ファイルで CHAIN（ロックなし）
**対象**: `CONFIRMSHP.RPGLE` (98行目)
```rpg
C W#SOID CHAIN SORDERHP ← COMMIT対象ファイルだがロックなし
```
その後 `@UPDSOH` で UPDATE していますが、検索時と更新時の間に他のプロセスが変更している可能性があります（Lost Update）。`CHAIN(E)` によるロック取得に変更してください。
---
### M-6. `ORDERENTRY` `@SAVEORD`: Phase2でのエラーハンドリング欠如
**対象**: `ORDERENTRY.RPGLE` (652行目)
Phase2（実際の在庫引当UPDATE）では UPDATE 失敗時の処理がありません。
```rpg
C INVKEY CHAIN(E) INVENTORYP
C IF %FOUND(INVENTORYP)
C EVAL QTYALC = QTYALC + L_ORDQTY(W#IDX)
C EVAL QTYAVL = QTYOH - QTYALC
C UPDATE INVREC ← UPDATE失敗時にW#ERRORをセットしていない
C ENDIF
```
---
### M-7. `ORDERENTRY` `@SAVEORD`: `SORDERLP` に `TAXRATE` フィールドが未設定
**対象**: `ORDERENTRY.RPGLE` (678行目)
`SOLREC` への WRITE で `SL_TAXRATE` がセットされていません。`CREATEINV` の `@PRCSHP` では `SORDERLP` から `TAXRATE` を取得しているため（`CREATEINV.RPGLE:174`）、`ORDERENTRY` から登録した受注に対して請求書を発行すると税率が 0 になります。
---
### M-8. `CNFSHPENT`: SFLのサイズ制限（最大9999行）とステータス未フィルタリング
**対象**: `CNFSHPENT.RPGLE` (69行目)
`@LOADSFL` では `SORDERHP` を全件ロードしており、`ALLOCATED`/`OPEN` でのフィルタリングがありません。受注が増えるとサブファイルがパフォーマンス問題を起こします。SQLカーソルによる WHERE フィルタリングが望ましいです。
---
### M-9. `PAYENTRY` `W#TIMESTR` の型不一致
**対象**: `PAYENTRY.RPGLE` (426行目)
```rpg
D WTIME6 S 6P 0 ← Packed Decimal
...
C EVAL W#TIMESTR = %CHAR(WTIME6)
```
`WTIME6` が `6P 0` (Packed) のため、`%CHAR()` では先頭ゼロが落ちます（例: `083045` → `'83045'`）。`CONFIRMSHP` で使われている `%EDITC()` による変換、または `TIME型` への変更が必要です。
---
## Minor Issues（スタイル・低影響の改善）
| # | 場所 | 内容 |
|---|------|------|
| m-1 | 全メンバー H仕様 | `OPTION(*SRCSTMT *NODEBUGIO)` が未指定 |
| m-2 | `CREATESORD`, `CREATEINV` | ヘッダコメントのライブラリ名が `DEMOAPP`（旧テンプレートの残存、実際は `IMAO1399`） |
| m-3 | `CREATEINV` (110行目) | `W#TAXRATE = 0.100` のハードコード（税率変更時に修正が必要） |
| m-4 | `ORDERENTRY` (272行目) | `REQDATE < ORDDATE` の文字列比較（8A型）は数値比較ではないため誤判定のリスクあり（Date型で比較推奨） |
| m-5 | `CONFIRMSHP` (55行目) | `W#USER` が `30A` だが、IBM i ユーザープロファイルは最大10文字。`10A` に揃えるべき |
| m-6 | `PAYENTRY` (25行目) | `WDATE8`/`WTIME6` が `8P 0`/`6P 0` なのに `ORDERENTRY` では `8S 0`/`6S 0`（プログラム間で型が統一されていない） |
| m-7 | 全サブルーチン | `EXSR` 呼び出しが全てグローバル変数依存（副作用が追いにくい） |
| m-8 | `PAYENTRY` (342行目) | SFL表示ロジックで `H#BALANCE >= W#AMOUNT` と比較しているが、外貨時は JPY 換算後の `W#BASEAMT` と比較すべき |
---
## 近代化ロードマップ（優先順位付き）
```
Phase 1 - Critical修正
1. ACTGRP(*NEW) -> (*CALLER) に変更
2. ORDERENTRY に COMMIT 制御を追加
3. 監査ユーザー 'DSPFUSER' ハードコードを修正
4. SOID 採番を SQLシーケンスに変更
Phase 2 - Major修正
5. SQLCODE 全チェックを追加
6. SHPID 採番を SQLシーケンスに変更
7. INFDS 定義を全ファイルに追加
8. TAXRATE 設定漏れを修正
Phase 3 - 近代化
**FREE 完全フリーフォーム移行
BEGSR -> DCL-PROC 変換
*IN99 指標 -> %FOUND / %ERROR へ置換
OPTION(*SRCSTMT) を全 H仕様に追加
```
---
## まとめ
| 分類 | 件数 | 影響 |
|------|------|------|
| Critical | 6件 | データ不整合・セキュリティ・本番障害リスク |
| Major | 9件 | 技術的負債・潜在バグ |
| Minor | 8件 | スタイル・可読性 |
**最優先で対応すべき3件**:
1. `ORDERENTRY.RPGLE` (668行目) — `'DSPFUSER'` ハードコード
2. `ORDERENTRY.RPGLE` (605行目) — SOID タイムスタンプ採番 -> SQLシーケンスへ
3. `ORDERENTRY.RPGLE` (F仕様) — `COMMIT` 制御の追加
レビュー報告は下記のように重要度別に整理されており、重要な項目から順にまとめられていることがわかります。
| 分類 | 件数 | 主な内容 |
|---|---|---|
| Critical | 6 件 |
ACTGRP(*NEW)、コミットメント制御の不足、監査ユーザーのハードコード、タイムスタンプによる採番、SQLCODE の未チェック |
| Major | 9 件 | サブルーチンのサブプロシージャー化、標識による制御、INFDS の未定義、Lost Update のリスクなど |
| Minor | 8 件 |
OPTION(*SRCSTMT *NODEBUGIO) の未指定、型の不統一、ヘッダーコメントの残存など |
最後に、Phase 1（Critical の修正）→ Phase 2（Major の修正）→ Phase 3（近代化）の優先順位付きロードマップと、最優先で対応すべき 3 件が示されています。
ただし、レビュー結果には正確ではない指摘も含まれていました。M-5 では「CHAIN(E) によるロック取得に変更してください」と指摘されていますが、(E) はエラーを %ERROR / %STATUS で受け取るための拡張子で、ロックの挙動は変わりません。この点を Bob に確認したところ、下記のように指摘の誤りを認めました。
| 命令 | 意味 |
|---|---|
CHAIN |
更新ファイル（UF）であれば、デフォルトでレコードをロックする |
CHAIN(E) |
エラーを %ERROR / %STATUS で受け取る。ロックの挙動は変わらない |
CHAIN(N) |
ロックを取得しない（参照のみ） |
ILE RPG Reference にも「If the file is specified as update, all records are locked if the N operation extender is not specified.」と記載されており、SORDERHP は UF で開かれているため、すでにレコードはロックされています。
レビュー結果の指摘をそのまま適用せず、内容を確認したうえで修正してください。
Bob Findings との連携について
IBM Bob には、レビュー結果を一覧で確認できる Bob Findings パネルがありますが、今回の /review_rpg の結果は Findings パネルでは確認できませんでした。
Findings パネルを利用するには Git との連携が前提になるため、/review でパネルを開くレビューとは別物として考えたほうがよいと思います。
感想
RPG 専用のコードレビュー機能は、オープン系の言語向けのものはありますが、ほかの製品では見たことがなかったので便利だと思います。
個人的には、ACTGRP(*NEW) は呼び出しのたびに新しい活動化グループを作成するのでパフォーマンスやトランザクションに支障が出るかもしれない、といった IBM i に特化したレビューを提供してくれる点が良いと感じました。
一方で、現時点では Bob Findings パネルとは連携していないため、結果はチャット欄で確認するか、Bob に依頼して IFS へ出力したファイルで確認する形になります。
※注意：本記事の内容は検証時点（IBM Bob Premium Package for i v2.0.0）のもので、画面や仕様は今後変更される可能性があります。