HarmonyOS开发：Code Review实践

核心要点：Code Review不是走过场，它是团队代码质量的第一道防线——好的Review流程能把80%的低级错误拦在合并之前。

背景与动机

你有没有经历过这种场景？线上出了Bug，排查半天发现是三个月前合并的一段代码导致的。当时谁Review的？没人说得清。Review记录？没有。那段代码的问题其实很明显——变量没判空、资源没释放、异常没捕获——但凡有人认真看一眼都不至于上线。

更常见的情况是：Review变成了"点赞大会"。PR一提交，同事扫一眼，“看起来没问题”，Approved。代码质量？全凭开发者自觉。这种Review跟没有有什么区别？

Code Review的本质不是找茬，是知识共享和质量保障。一个人写的代码，永远有盲区。你觉得理所当然的逻辑，在别人眼里可能就是个坑。Review就是让第二双眼睛帮你看到盲区。

鸿蒙项目尤其需要严格的Review。为什么？ArkTS的异步模型、UI状态管理、生命周期回调——这些跟传统前端差别不小，踩坑概率高。一个不熟悉ArkUI状态管理的同事，很容易写出@State滥用、@Link传错方向的代码，Review的时候不指出来，后面改起来成本翻倍。

核心原理

Code Review的四个层次

Code Review不是简单地"看代码对不对"，它有层次之分：

graph TD
    A[Code Review四层模型] --> B[第一层：正确性]
    A --> C[第二层：可读性]
    A --> D[第三层：架构合规]
    A --> E[第四层：知识传递]
    
    B --> B1[逻辑是否正确]
    B --> B2[边界条件是否处理]
    B --> B3[异常是否捕获]
    
    C --> C1[命名是否清晰]
    C --> C2[函数是否过长]
    C --> C3[注释是否充分]
    
    D --> D1[是否违反分层]
    D --> D2[依赖是否合理]
    D --> D3[接口是否规范]
    
    E --> E1[团队成员是否学到新知识]
    E --> E2[隐含的业务规则是否显性化]
    E --> E3[最佳实践是否推广]
    
    classDef coreStyle fill:#FF6B6B,stroke:#C0392B,color:#fff,font-weight:bold
    classDef layerStyle fill:#4ECDC4,stroke:#1ABC9C,color:#fff
    classDef detailStyle fill:#FFE66D,stroke:#F39C12,color:#333
    
    class A coreStyle
    class B,C,D,E layerStyle
    class B1,B2,B3,C1,C2,C3,D1,D2,D3,E1,E2,E3 detailStyle

Review流程设计

一个有效的Review流程长什么样？

graph LR
    A[开发者提交PR] --> B[自动化检查]
    B --> B1{通过?}
    B1 -->|否| A
    B1 -->|是| C[指定Reviewer]
    C --> D[Reviewer审查]
    D --> D1{需要修改?}
    D1 -->|是| E[开发者修改]
    E --> D
    D1 -->|否| F[至少2人Approved]
    F --> G[合并到主分支]
    
    classDef startStyle fill:#A8E6CF,stroke:#2ECC71,color:#333
    classDef checkStyle fill:#FFD93D,stroke:#F1C40F,color:#333
    classDef reviewStyle fill:#6C5CE7,stroke:#8E44AD,color:#fff
    classDef endStyle fill:#FF6B6B,stroke:#E74C3C,color:#fff
    
    class A startStyle
    class B,B1 checkStyle
    class C,D,D1,E reviewStyle
    class F,G endStyle

关键规则：

• PR必须关联Issue：没有Issue的PR不予Review，防止无目的的改动
• 单次PR不超过400行：超过400行的PR，Reviewer大概率走马观花
• 至少2人Approved才能合并：一人可能看漏，两人交叉验证
• 24小时内完成Review：拖久了开发者上下文都忘了，改起来效率极低

代码实战

基础用法：Review Checklist模板

鸿蒙项目的Review不能靠"凭感觉"，得有Checklist。下面是一份实战中打磨出来的模板：

// Review Checklist - 鸿蒙项目专用
// 每次Review前对照检查，确保不遗漏关键项

interface ReviewChecklist {
  // === 正确性检查 ===
  correctness: {
    // 空值安全：ArkTS的null/undefined处理
    nullSafety: boolean;          // 是否处理了null/undefined？
    asyncError: boolean;          // 异步操作是否有错误处理？
    resourceRelease: boolean;     // 资源（文件、网络连接）是否释放？
    lifecycle: boolean;           // 组件生命周期是否正确使用？
  };

  // === ArkTS特性检查 ===
  arktsSpecific: {
    stateManagement: boolean;     // @State/@Link/@Prop使用是否正确？
    builderUsage: boolean;        // @Builder是否有不必要的重建？
    concurrentSafety: boolean;    // TaskPool/Worker是否有并发安全问题？
    typeSafety: boolean;          // 是否利用了ArkTS的严格类型？
  };

  // === 性能检查 ===
  performance: {
    avoidRebuild: boolean;        // 是否有不必要的UI重建？
    lazyLoad: boolean;            // 长列表是否使用LazyForEach？
    memoryLeak: boolean;          // 是否有潜在的内存泄漏？
  };

  // === 安全检查 ===
  security: {
    noHardcodedSecret: boolean;   // 是否有硬编码的密钥/密码？
    inputValidation: boolean;     // 用户输入是否做了校验？
    permissionCheck: boolean;     // 敏感操作是否检查了权限？
  };
}

// 使用示例：Review时逐项打勾
const checklist: ReviewChecklist = {
  correctness: {
    nullSafety: true,
    asyncError: false,      // ⚠️ 发现异步错误处理缺失
    resourceRelease: true,
    lifecycle: true,
  },
  arktsSpecific: {
    stateManagement: false, // ⚠️ @State使用不当
    builderUsage: true,
    concurrentSafety: true,
    typeSafety: true,
  },
  performance: {
    avoidRebuild: true,
    lazyLoad: false,        // ⚠️ 长列表用了ForEach
    memoryLeak: true,
  },
  security: {
    noHardcodedSecret: true,
    inputValidation: true,
    permissionCheck: true,
  },
};

进阶用法：常见代码问题模式与Review要点

鸿蒙开发中，有些问题模式反复出现。Review时重点盯这些：

// ==========================================
// 问题模式1：@State滥用导致性能问题
// ==========================================

// ❌ 错误写法：整个列表用@State，任何修改都触发全量重建
@Component
struct BadListExample {
  @State itemList: ItemData[] = [];  // 列表数据用@State

  build() {
    List() {
      ForEach(this.itemList, (item: ItemData) => {
        ListItem() {
          Text(item.name)
        }
      })
    }
  }
}

// ✅ 正确写法：列表用@State，但列表项用@Observed/@ObjectLink
@Observed
class ItemData {
  name: string = '';
  selected: boolean = false;  // 频繁变化的属性
}

@Component
struct GoodListExample {
  @State itemList: ItemData[] = [];

  build() {
    List() {
      LazyForEach(this.itemList, (item: ItemData) => {
        ListItem() {
          ItemView({ item: item })  // 通过@ObjectLink精准更新
        }
      })
    }
  }
}

@Component
struct ItemView {
  @ObjectLink item: ItemData;  // 只监听单个对象的属性变化

  build() {
    Row() {
      Text(this.item.name)
      Checkbox()
        .select(this.item.selected)
        .onChange((value: boolean) => {
          this.item.selected = value;  // 只重建当前ListItem
        })
    }
  }
}

// ==========================================
// 问题模式2：异步操作缺少错误处理
// ==========================================

// ❌ 错误写法：网络请求没有错误处理
async function fetchUserData(userId: string): Promise<UserData> {
  const response = await http.request(`https://api.example.com/users/${userId}`);
  return response.result as UserData;  // 如果网络错误呢？如果返回格式不对呢？
}

// ✅ 正确写法：完整的错误处理链
async function fetchUserDataSafe(userId: string): Promise<UserData> {
  try {
    // 参数校验
    if (!userId || userId.trim().length === 0) {
      throw new Error('用户ID不能为空');
    }

    const response = await http.request(
      `https://api.example.com/users/${userId}`,
      { method: http.RequestMethod.GET }
    );

    // 响应码校验
    if (response.responseCode !== 200) {
      throw new Error(`请求失败，状态码: ${response.responseCode}`);
    }

    // 数据格式校验
    const data = JSON.parse(response.result as string) as UserData;
    if (!data.id || !data.name) {
      throw new Error('返回数据格式异常');
    }

    return data;
  } catch (error) {
    // 记录错误日志
    hilog.error(0x0001, 'UserService', `获取用户数据失败: ${error}`);
    // 向上层抛出，让调用者决定如何处理
    throw new Error(`获取用户数据失败: ${error}`);
  }
}

// ==========================================
// 问题模式3：资源未释放导致泄漏
// ==========================================

// ❌ 错误写法：订阅了事件但从不取消
@Component
struct LeakExample {
  aboutToAppear() {
    // 订阅了，但aboutToDisappear里没取消
    emitter.on({ eventId: 1 }, () => {
      // 处理事件
    });
  }

  build() {
    Text('Hello')
  }
}

// ✅ 正确写法：生命周期配对
@Component
struct NoLeakExample {
  private eventCallback = () => {
    // 处理事件
  };

  aboutToAppear() {
    emitter.on({ eventId: 1 }, this.eventCallback);
  }

  aboutToDisappear() {
    // 组件销毁时取消订阅，防止内存泄漏
    emitter.off({ eventId: 1 }, this.eventCallback);
  }

  build() {
    Text('Hello')
  }
}

完整示例：自动化Review辅助工具

手动Review效率有限，我们可以写个脚本自动检测一些常见问题：

// review-helper.ets
// 自动化Review辅助工具：扫描代码中的常见问题模式

interface ReviewIssue {
  file: string;           // 文件路径
  line: number;           // 行号
  severity: 'error' | 'warning' | 'info';  // 严重程度
  rule: string;           // 规则ID
  message: string;        // 问题描述
  suggestion: string;     // 修复建议
}

class CodeReviewHelper {
  private issues: ReviewIssue[] = [];

  // 检查@State是否用于大对象
  checkStateUsage(content: string, filePath: string): void {
    const lines = content.split('\n');
    lines.forEach((line, index) => {
      // 检测 @State 装饰的数组或大对象
      const stateArrayMatch = line.match(/@State\s+\w+:\s+\w+\[\]/);
      if (stateArrayMatch) {
        this.issues.push({
          file: filePath,
          line: index + 1,
          severity: 'warning',
          rule: 'ARK-STATE-001',
          message: '@State装饰了数组类型，可能导致不必要的全量重建',
          suggestion: '考虑使用@Observed/@ObjectLink进行细粒度更新',
        });
      }

      // 检测 @State 装饰的class类型
      const stateClassMatch = line.match(/@State\s+\w+:\s+[A-Z]\w+/);
      if (stateClassMatch) {
        this.issues.push({
          file: filePath,
          line: index + 1,
          severity: 'info',
          rule: 'ARK-STATE-002',
          message: '@State装饰了自定义类型，确认是否需要细粒度更新',
          suggestion: '如果属性频繁变化，考虑@Observed/@ObjectLink',
        });
      }
    });
  }

  // 检查ForEach是否应该替换为LazyForEach
  checkForEachUsage(content: string, filePath: string): void {
    const lines = content.split('\n');
    let inListComponent = false;

    lines.forEach((line, index) => {
      if (line.includes('@Component')) {
        inListComponent = true;
      }
      if (inListComponent && line.includes('ForEach(')) {
        this.issues.push({
          file: filePath,
          line: index + 1,
          severity: 'warning',
          rule: 'ARK-PERF-001',
          message: '在组件中使用了ForEach，大数据量时性能较差',
          suggestion: '数据量超过50条时建议使用LazyForEach',
        });
      }
    });
  }

  // 检查硬编码密钥
  checkHardcodedSecrets(content: string, filePath: string): void {
    const lines = content.split('\n');
    const secretPatterns = [
      { pattern: /password\s*[:=]\s*['"].+['"]/, name: '密码' },
      { pattern: /api[_-]?key\s*[:=]\s*['"].+['"]/, name: 'API密钥' },
      { pattern: /secret\s*[:=]\s*['"].+['"]/, name: '密钥' },
      { pattern: /token\s*[:=]\s*['"].+['"]/, name: '令牌' },
    ];

    lines.forEach((line, index) => {
      for (const { pattern, name } of secretPatterns) {
        if (pattern.test(line.toLowerCase())) {
          this.issues.push({
            file: filePath,
            line: index + 1,
            severity: 'error',
            rule: 'SEC-001',
            message: `检测到硬编码的${name}`,
            suggestion: `将${name}移至环境变量或加密配置中`,
          });
        }
      }
    });
  }

  // 检查资源释放配对
  checkResourceRelease(content: string, filePath: string): void {
    const subscribePatterns = [
      { on: 'emitter.on', off: 'emitter.off' },
      { on: 'eventHub.on', off: 'eventHub.off' },
    ];

    for (const { on, off } of subscribePatterns) {
      const hasSubscribe = content.includes(on);
      const hasUnsubscribe = content.includes(off);
      if (hasSubscribe && !hasUnsubscribe) {
        this.issues.push({
          file: filePath,
          line: 0,
          severity: 'warning',
          rule: 'LEAK-001',
          message: `使用了${on}但未找到对应的${off}`,
          suggestion: `在aboutToDisappear中调用${off}释放订阅`,
        });
      }
    }
  }

  // 执行全部检查
  runAllChecks(content: string, filePath: string): ReviewIssue[] {
    this.issues = [];
    this.checkStateUsage(content, filePath);
    this.checkForEachUsage(content, filePath);
    this.checkHardcodedSecrets(content, filePath);
    this.checkResourceRelease(content, filePath);
    return this.issues;
  }

  // 生成Review报告
  generateReport(issues: ReviewIssue[]): string {
    const errorCount = issues.filter(i => i.severity === 'error').length;
    const warningCount = issues.filter(i => i.severity === 'warning').length;
    const infoCount = issues.filter(i => i.severity === 'info').length;

    let report = `=== Code Review 自动检查报告 ===\n`;
    report += `错误: ${errorCount} | 警告: ${warningCount} | 提示: ${infoCount}\n\n`;

    issues.forEach(issue => {
      const icon = issue.severity === 'error' ? '❌' : 
                   issue.severity === 'warning' ? '⚠️' : 'ℹ️';
      report += `${icon} [${issue.rule}] ${issue.file}:${issue.line}\n`;
      report += `   ${issue.message}\n`;
      report += `   建议: ${issue.suggestion}\n\n`;
    });

    return report;
  }
}

// 使用示例
const reviewer = new CodeReviewHelper();
const sampleCode = `
@Component
struct MyPage {
  @State userList: UserData[] = [];
  @State config: AppConfig = new AppConfig();
  password: string = 'admin123';

  aboutToAppear() {
    emitter.on({ eventId: 1 }, () => {});
  }

  build() {
    List() {
      ForEach(this.userList, (item: UserData) => {
        ListItem() { Text(item.name) }
      })
    }
  }
}
`;

const issues = reviewer.runAllChecks(sampleCode, 'MyPage.ets');
console.log(reviewer.generateReport(issues));

踩坑与注意事项

坑1：Review变成了"挑格式"大会

很多团队的Review沦为了格式讨论：这个缩进不对、这个命名不好、这个分号多了……格式问题应该交给Lint工具自动处理，Review的时间应该花在逻辑和架构上。

解决方案：在CI中加入Lint检查，格式问题不通过不允许提交PR。Review时只关注Lint无法检测的问题。

坑2：PR太大，没人愿意认真看

一个PR改了2000行代码，你让Reviewer怎么看？扫一眼就Approved了，因为认真看完得花半天。

解决方案：

• 单次PR控制在400行以内（不含自动生成代码）
• 大功能拆成多个小PR，逐步提交
• PR描述必须写清楚改了什么、为什么改、怎么测试的

坑3：Review意见没人跟进

Reviewer提了一堆意见，开发者看了一眼，“好的好的”，然后合并了——问题一个没改。

解决方案：

• Review意见必须逐条回复：已修改/不修改（附理由）/后续处理（附Issue号）
• "Request Changes"后必须等开发者重新提交，不能直接Override合并
• 合并前检查所有Conversation是否已Resolved

坑4：Review标准不统一

同样的代码，A说没问题，B说要改。开发者一脸懵：到底听谁的？

解决方案：

• 团队统一Review Checklist
• 争议问题由Tech Owner最终裁定
• 定期Review Review本身——回顾Review意见的合理性和一致性

坑5：鸿蒙特有的Review盲区

传统前端Review经验不能完全套用到鸿蒙项目。以下是鸿蒙特有的Review盲区：

HarmonyOS 6适配说明

HarmonyOS 6对开发工具链做了不少升级，直接影响Code Review的效率：

1. DevEco Studio 6内置Code Review助手：基于AI的代码审查建议，能在提交PR前自动检测常见问题。注意，AI建议不能替代人工Review，但可以作为预检工具减少低级问题。

2. ArkTS严格模式增强：HarmonyOS 6的ArkTS编译器对类型检查更严格，一些之前能跑的代码可能报错。Review时关注编译器的新警告。

3. 模块化Review支持：HSP模块的Review可以独立进行，不再需要全量编译。大型项目可以按模块并行Review，提升效率。

4. Review与测试联动：HarmonyOS 6的测试框架支持在Review阶段自动运行增量测试，只有测试通过的PR才能进入Review流程。

总结

Code Review不是形式主义，它是团队代码质量的保险绳。好的Review流程能让问题在合并前暴露，而不是上线后炸雷。

关键记住这几条：

• Review有层次：从正确性到知识传递，四层递进
• Checklist驱动：不靠感觉，靠规则
• PR要小：400行以内，大了没人认真看
• 鸿蒙有盲区：状态管理、生命周期、并发安全是重点
• 工具辅助：Lint处理格式，人工关注逻辑