| name | ios-code-review |
| description | Guides comprehensive iOS code review following a structured five-phase methodology. Use when reviewing iOS Pull Requests, Merge Requests, or peer code that involves Swift, Objective-C, SwiftUI, or UIKit. |
iOS 代码审查
核心原则
先理解为什么改,再评判改得对不对,最后关注会不会影响其他地方。
代码审查不是找茬,而是团队的质量防线。好的 Review 需要做到三件事:
- 理解意图 — 这个 PR 要解决什么问题?方向对不对?
- 审查质量 — 实现是否正确、安全、可维护?
- 评估影响 — 改动会不会波及其他功能?
审查者的职责边界:
- 把关正确性和安全性 → 必须严格
- 把关架构一致性 → 必须坚持
- 风格偏好和命名 → 建议但不阻塞
- 更优雅的实现方式 → 分享但不强求
绝对不能放过的问题:
- 可能导致崩溃的代码(强制解包、数组越界、线程不安全)
- 内存泄漏(循环引用、未释放资源)
- 逻辑错误(条件分支遗漏、状态不一致)
- 对现有功能的破坏(公共代码修改未评估影响)
- 安全问题(敏感信息暴露、不安全的数据处理)
审查流程(五阶段)
复杂度速判
在开始审查前,先根据 PR 改动量和类型判断复杂度等级,选择对应的审查深度:
| 等级 | 判断标准 | 审查策略 |
|---|
| S(小型) | ≤50 行,单一职责(文案/配置/样式微调) | 跳过阶段二,阶段三仅做安全性快扫,直接进入阶段五 |
| M(中型) | 51–300 行,功能完整但范围可控 | 完整五阶段流程 |
| L(大型) | >300 行,跨模块/架构变更/新功能 | 完整五阶段 + 要求拆分(>500 行必须拆分),重点关注影响范围 |
注意: 即使是 S 级 PR,涉及以下内容时自动升级为 M+:安全相关改动、公共 API 变更、并发/线程代码、数据持久化。
PR 规模审查策略
| PR 规模 | 文件数 | 审查方式 |
|---|
| 精准型 | 1–3 个文件 | 逐行精读,关注每个变更的上下文 |
| 功能型 | 4–10 个文件 | 先理解架构意图,按 Model → Service → ViewModel → View 顺序审查 |
| 大型重构 | >10 个文件 | 先要求 PR 描述提供改动总览,按模块分批审查,使用 checklist 跟踪 |
| 自动生成 | 大量但模式单一 | 抽查 2–3 个代表文件,确认生成规则正确 |
阶段一:阅读 PR 概览
提取 PR 标题/描述/关联 Issue,查看改动统计和文件清单。判断 PR 是否需要拆分(>500 行、混合类型改动)、描述是否充分(缺少"为什么"、Bug 修复无根因、UI 变化无截图)。如果 PR 描述不清晰,先请作者补充再开始 Review。
门控输出: PR 概要(类型 + 规模 + 复杂度等级)已确认,可进入下一阶段。若需拆分或补充描述,阻塞后续阶段。
阶段二:理解改动意图与上下文
根据 PR 类型(新功能/Bug 修复/重构)理解背景。不要只看 diff,打开完整文件理解修改点的上下文,全局搜索被修改函数的调用方和继承关系。脱离上下文的 diff 审查是无效的。
门控输出: 改动意图已理解,能用一句话概括"这个 PR 在做什么"。若无法理解意图,标记 💡 Question 向作者确认。
阶段三:逐层代码审查
从多个维度系统化审查,根据改动内容选择对应的审查引用文件。首先审查正确性(逻辑完整性、边界值、数据类型),然后根据涉及的技术栈深入审查安全性和规范性。→ 参见下方 Topic Router
门控输出: 所有审查维度已覆盖,发现的问题已按分级标签分类。无 🔴 遗漏。
阶段四:评估影响范围
检查修改代码的依赖方:公共模型/组件/Protocol/Service 的改动是否影响调用方。评估间接影响:状态传播、导航流程、数据持久化、编译配置。影响范围不清楚时,宁可多问,不要放过。
门控输出: 影响范围已评估,受影响的模块/调用方已列出。不确定的影响已标记为 💡 Question。
阶段五:给出反馈
使用反馈分级标签,按文件组织发现,提供清晰可操作的建议。→ 参见下方 输出格式规范 和 references/review-techniques.md
门控输出: Review 总结已输出,结论已给出(Approve / Request Changes / Comment)。
Topic Router
| 审查话题 | 引用文件 |
|---|
| Swift 可选值 / 错误处理 / 类型安全 / Swift 6 并发 | references/swift-safety.md |
| Objective-C 空安全 / Block / KVC-KVO / 混编 | references/objc-safety.md |
| 循环引用 / 资源释放 / 线程安全 / 并发 | references/memory-thread.md |
| SwiftUI 属性包装器 / 性能 / 状态管理 | references/swiftui-review.md |
| UIKit Auto Layout / Cell 复用 / VC 生命周期 / 混编 | references/uikit-review.md |
| 废弃 API 识别 / 现代替代方案 | references/modern-api.md |
| 提问法 / 沟通模板 / 阅读顺序 / 效率技巧 | references/review-techniques.md |
| 生命周期 / API 兼容性 / 架构一致性 | references/platform-checklist.md |
| 无障碍 / 权限 / 隐私 / Privacy Manifest | references/accessibility-privacy.md |
| 测试覆盖 / 测试质量 / Swift Testing | references/testing-review.md |
输出格式规范
反馈分级标签
| 标签 | 含义 | 是否阻塞合并 |
|---|
| 🔴 Must Fix | 崩溃/数据丢失/安全问题/逻辑错误/内存泄漏 | 是 |
| 🟡 Should Fix | 代码质量/架构约定/缺少错误处理/缺少测试 | 否(强烈建议) |
| 🟢 Nit | 命名偏好/风格微调/更优雅的写法 | 否 |
| 💡 Question | 不理解意图/不确定设计决策 | 视回答而定 |
| 📝 Info | 知识分享/未来风险提醒 | 否 |
单条反馈格式
[等级标签] 问题概述
**文件:** `FileName.swift:42`
**违规规则:** [对应的检查项]
**问题:** [具体描述]
**原因:** [为什么这是问题]
// Before
[问题代码]
// After
[建议修改]
整体 Review 总结
## Review 总结
**改动概述:** [一句话概括]
**总体评价:** [先肯定做得好的地方]
**优先级摘要:**
- 🔴 Must Fix: N 个
- 🟡 Should Fix: N 个
- 🟢 Nit: N 个
**必须修复:**
1. [问题 + 文件位置]
**建议改进:**
1. [建议 + 原因]
**影响范围确认:**
[需要作者额外验证的地方]
**结论:** [Approve / Request Changes / Comment]
红旗 — 审查流程违规
| 违规行为 | 为什么是错误的 | 正确做法 |
|---|
| 只看 diff 不看上下文 | 脱离上下文无法判断正确性 | 打开完整文件理解改动位置 |
| 不理解需求就开始 Review | 无法判断实现是否正确 | 先读 PR 描述和关联 Issue |
| 只关注代码风格不关注逻辑 | 风格问题不会导致线上故障 | 优先审查正确性和安全性 |
| 放过 force unwrap 因为"现在不会 nil" | 未来数据变化时会崩溃 | 要求安全处理 |
| 不评估公共代码修改的影响 | 改一处坏多处 | 全局搜索确认影响范围 |
| 大量 nit 淹没关键问题 | 作者分不清轻重缓急 | 使用分级标签,关键问题优先 |
| Approve 但心里有疑问没问 | 上线后出问题更难排查 | 有疑问就问,不确定就不 Approve |
| 只批评不给方案 | 不可操作,浪费作者时间 | 每个问题附带建议方案 |
| Review 超过 1000 行不要求拆分 | 大 PR 审查质量必然下降 | 先要求拆分再 Review |
成功指标
- Review 发现的问题在上线前被修复,而非上线后被用户发现
- 作者从 Review 中学到了新东西(不仅是修复了问题)
- Review 评论清晰到作者不需要追问就能修复
- 团队代码质量在持续提升,同类问题不反复出现
- Review 回合数通常 ≤ 2 轮(说明反馈足够清晰和全面)
与其他 Skill 的协作
- ios-feature-impl:需求实现的代码提交后,使用本 skill 进行审查
- ios-issue-fix:Bug 修复的代码提交后,使用本 skill 进行审查