comprehensive-code-review

ACE Engine の包括的なコードレビュー

クイックスタート

AI を使って、コードレビュー分析を直接実行します。

"Review the code in frameworks/core/components_ng for memory management issues"

"Analyze this pull request focusing on security vulnerabilities"

"Check path/to/code.cpp for ACE Engine architecture compliance"

レビューワークフロー

フェーズ 1：コード分析

AI は、以下に基づいて関連するすべての観点からコードを分析します。

1. コードファイルの読み込み - 実装の詳細を理解します
2. 参照基準の適用 - references/ ディレクトリ内の基準を使用します
3. ACE Engine のパターンとの照合 - プロジェクト固有の規約に準拠しているか確認します

フェーズ 2：観点別レビュー

19 以上の観点からコードをレビューします。対象範囲の詳細は、DIMENSIONS.md を参照してください。

優先度の高い観点：

• セキュリティ（CRITICAL） - 入力検証、コマンドインジェクション、バッファオーバーフロー
• メモリ（CRITICAL） - スマートポインター、メモリーリーク、所有権のセマンティクス
• スレッド処理（HIGH） - データ競合、デッドロック、コールバックの安全性
• 安定性（HIGH） - エラー処理、境界条件

優先度が中程度の観点：

• パフォーマンス - アルゴリズムの計算量、最適化の機会
• SOLID 原則 - 単一責任、オープン・クローズドなど
• コードの不吉な臭い - 長いメソッド、大きなクラス、重複コード
• テスト容易性 - 依存性注入、疎結合

フェーズ 3：レポートの生成

AI は、以下を含む包括的なレビューレポートを生成します。

• 概要 - 重大度別の問題件数
• 重大な問題（🔴） - マージ前に必ず修正
• 優先度の高い問題（🟠） - マージ前に修正することを推奨
• 優先度が中程度の問題（🟡） - 早めに修正
• 優先度の低い問題（🟢） - 可能であれば対応
• 観点別の内訳 - カテゴリー別にまとめた問題
• ファイルごとの詳細 - 該当する行番号とコードスニペット
• 推奨事項 - 修正前後の例を含む、優先順位付きの対応項目
• レビューチェックリスト - マージ前の確認事項

観点のリファレンス

すべての観点のクイックリファレンス：

観点	確認対象	重大度レベル	リファレンス
安定性	エラー処理、境界条件	🔴🟠🟡🟢	STABILITY.md
パフォーマンス	アルゴリズム、最適化	🔴🟠🟡🟢	DIMENSIONS.md
スレッド処理	並行処理、同期	🔴🟠🟡	DIMENSIONS.md
セキュリティ	入力検証、脆弱性	🔴🟠🟡	SECURITY.md
メモリ	スマートポインター、メモリーリーク	🔴🟠🟡	MEMORY.md
モダン C++	C++11/14/17/20 の機能	🟠🟡	DIMENSIONS.md
Effective C++	RAII、Rule of Five	🟠🟡	DIMENSIONS.md
コードの不吉な臭い	22 種類の臭い	🟠🟡	CODE_SMELLS.md
SOLID	設計原則	🟠🟡	SOLID.md
デザインパターン	パターンの使用状況	🟡🟢	DIMENSIONS.md
堅牢性	フォールトトレランス	🟠🟡	DIMENSIONS.md
テスト容易性	依存性注入	🟠🟡	DIMENSIONS.md
保守性	コードの複雑さ	🟡🟢	DIMENSIONS.md
可観測性	ロギング、監視	🟡🟢	DIMENSIONS.md
API 設計	インターフェースの品質	🟠🟡	DIMENSIONS.md
技術的負債	TODO/FIXME の追跡	🟡🟢	DIMENSIONS.md
後方互換性	API の安定性	🟠🟡	DIMENSIONS.md
アーキテクチャ	ACE Engine への準拠	🔴🟠🟡	ACE_ENGINE_SPECIFIC.md

ACE Engine 固有のチェック

アーキテクチャの検証

4 層アーキテクチャ：

Frontend Bridge Layer
    ↓
Component Framework Layer (Pattern/Model/Property)
    ↓
Layout/Render Layer
    ↓
Platform Adapter Layer (OHOS/Preview)

確認事項：

• レイヤー境界への違反がないこと
• Pattern/Model/Property の分離が適切に行われていること
• フレームワークレイヤーからプラットフォームを直接呼び出していないこと

コンポーネント構造

components_ng/pattern/<component>/
├── *_pattern.h/cpp         # Business logic & lifecycle
├── *_model.h/cpp           # Data model interface
├── *_layout_property.h/cpp # Layout properties
├── *_paint_property.h/cpp  # Render properties
└── *_event_hub.h/cpp       # Event handling

RefPtr の使用

// ✅ Creation
auto node = AceType::MakeRefPtr<FrameNode>();

// ✅ Type-safe casting
auto pattern = AceType::DynamicCast<MenuPattern>(node->GetPattern());
if (!pattern) {
    LOGE("Failed to get pattern");
    return false;
}

// ✅ Breaking cycles with WeakPtr
class Child {
    WeakPtr<Parent> parent_;  // Use weak, not strong
};

// ✅ Safe callbacks
auto weak = AceType::WeakClaim(this);
PostTask([weak]() {
    auto pattern = AceType::DynamicCast<MenuPattern>(weak.Upgrade());
    if (pattern) pattern->Update();
});

命名規則

class MenuPattern {           // PascalCase
public:
    void OnModifyDone();      // PascalCase
    int GetWidth() const;     // Get prefix

private:
    int width_;               // snake_case_ with trailing underscore
    std::string component_id_;  // Abbreviations lowercase
};

constexpr int MAX_MENU_ITEMS = 100;  // UPPER_CASE

重要度レベル

🔴 重大 - マージ前に必ず修正してください

• セキュリティの脆弱性（コマンドインジェクション、バッファーオーバーフロー）
• 長時間実行プロセスでのメモリーリーク
• 解放後使用、二重解放
• データ競合
• クラッシュ／未定義動作
• アーキテクチャ違反

🟠 高 - マージ前の修正を推奨します

• パフォーマンスの低下
• API 違反
• SOLID 原則への違反
• 重大なコードの不吉な臭い（巨大なクラス、ショットガン手術）
• RefPtr の不適切な使用
• 安全でないコールバックキャプチャ

🟡 中 - 早めに修正してください

• 軽微なパフォーマンスの問題
• コードの不吉な臭い（長いメソッド、重複コード）
• スタイル違反
• 重要でない処理経路でのエラーハンドリングの不足

🟢 低 - 対応すると望ましい項目

• コメントの不足
• 軽微なスタイル上の問題
• 最適化の機会
• コードドキュメントの改善

よくある問題のパターン

パターン	分野	重要度	検出方法
RefPtr の代わりに生ポインターを使用	メモリー	高	RefPtr を使わない new T()
コマンドインジェクション	セキュリティ	重大	system(user_input)
バッファーオーバーフロー	セキュリティ	重大	strcpy()、sprintf()
安全でない this のキャプチャ	スレッド処理	高	PostTask 内の [this]
長いメソッド（50 行超）	コードの不吉な臭い	中	関数の長さ
巨大なクラス（500 行超）	コードの不吉な臭い	高	クラスのサイズ
null チェックの不足	安定性	高	検証せずにポインターを使用
DynamicCast の代わりに static_cast を使用	メモリー	中	static_cast<T*>

使用例

プルリクエストのレビュー

"Review the changes in this PR with focus on memory management and threading safety"

AI は次の処理を行います：

• 変更されたすべてのファイルを分析します
• メモリー管理（RefPtr、WeakPtr、生ポインター）を確認します
• スレッドセーフ性（コールバックキャプチャ、PostTask の使用方法）を検証します
• 行番号と重要度を添えて問題を報告します

セキュリティ監査

"Perform a security audit of path/to/code, focusing on input validation and potential vulnerabilities"

AI は次の項目を確認します：

• コマンドインジェクションのリスク（system() の呼び出し）
• バッファーオーバーフローの脆弱性（strcpy、sprintf）
• 検証されていないユーザー入力
• 安全でない型キャスト

メモリーリークの検出

"Analyze frameworks/core/components for potential memory leaks and improper RefPtr usage"

AI は次の項目を特定します：

• スマートポインターを使わない生の new/delete
• WeakPtr を使用しない循環参照
• DynamicCast 後の null チェックの不足
• 不適切な所有権の扱い

アーキテクチャ準拠の確認

"Verify that components_ng/menu follows the four-layer architecture and Pattern/Model/Property separation"

AI は次の項目を検証します：

• 4 層アーキテクチャへの準拠
• Pattern/Model/Property の分離
• レイヤー境界への違反がないこと
• コンポーネントのライフサイクルメソッド（OnModifyDone など）

統合例

コミット前フック

コミット前フックには、従来の静的解析ツールの使用を検討してください：

#!/bin/bash
# .git/hooks/pre-commit

CHANGED_FILES=$(git diff --cached --name-only --diff-filter=ACM | grep -E '\.(cpp|cc|cxx|h|hpp|ts|tsx)$')

if [ -n "$CHANGED_FILES" ]; then
    echo "Running basic static analysis..."
    # Use clang-tidy, cppcheck, eslint, etc.
    for file in $CHANGED_FILES; do
        if [[ $file == *.cpp || $file == *.h ]]; then
            clang-tidy $file --checks='*' || true
        elif [[ $file == *.ts ]]; then
            eslint $file || true
        fi
    done
fi

手動レビューのワークフロー

AI を活用して包括的なレビューを行う場合：

1. レビュー対象のコード（PR、特定のファイル、ディレクトリ）を特定します
2. 重点項目を指定してレビューを依頼します
3. AI が参照基準を使ってコードを分析します
4. 重要度が付与された問題を含む結果を確認します
5. 重大度と優先度の高い問題に対処します
6. 検証のため、再度レビューを依頼します

参考資料

すべての分野に関する詳細なドキュメントは references/ にあります：

• DIMENSIONS.md - 19 以上のすべての分野のクイックリファレンス
• STABILITY.md - エラーハンドリング、境界条件
• MEMORY.md - スマートポインター、リーク、所有権
• SECURITY.md - 入力検証、脆弱性
• CODE_SMELLS.md - 22 種類のコードの不吉な臭い
• SOLID.md - 5 つの設計原則
• ACE_ENGINE_SPECIFIC.md - プロジェクト固有のルール

アセット

• report_template.md - AI 生成レビュー向けのカスタマイズ可能なレポートテンプレート

レビューチェックリスト

コードをマージする前に：

コード品質：

• 重大な問題がない
• 高優先度の問題に対処済み、または記録済み
• コンパイラの警告がない
• 静的解析で問題がない

アーキテクチャ：

• 4 層アーキテクチャを遵守している
• パターン／モデル／プロパティを適切に分離している
• レイヤー境界に違反していない

メモリ：

• メモリリークがない
• RefPtr を正しく使用している
• 循環参照の解消に WeakPtr を使用している
• コールバックで WeakClaim を使用している

セキュリティ：

• すべての入力を検証している
• コマンドインジェクションがない
• バッファーオーバーフローがない
• 機密データを保護している

テスト：

• ユニットテストを追加している
• エッジケースを網羅している
• エラーのシナリオをテストしている