PHP代码审查怎么做

wen PHP项目 2026-07-04 24

本文目录导读：

PHP代码审查怎么做

审查前的准备
审查的核心维度（按重要性排序）
审查流程（怎么做）
常见的PHP“坑”与检查清单
给审查者的建议
一份实用的PHP代码审查Checklist

进行有效的PHP代码审查,不仅仅是为了找出Bug，更重要的是提升代码质量、可维护性、安全性以及团队一致性。

以下是进行PHP代码审查的系统性步骤和关键检查点。

审查前的准备

明确目标：本次审查是为了修复Bug、优化性能、统一规范，还是安全检查？不同目标侧重点不同。
了解上下文：拿到代码后，先看PR/提交信息，了解要解决什么问题，不要上来就抠细节。
设置工具（可选但推荐）：
- 静态分析工具：PHPStan（推荐level 6+）、Psalm（严格模式），能在审查前自动揪出类型错误。
- 代码规范检查：PHP_CodeSniffer (phpcs) + PSR-12标准。
- Laravel专属：Larastan。

审查的核心维度（按重要性排序）

安全性（重中之重）

PHP代码中90%的高危漏洞来自这里。

SQL注入：是否使用预编译语句（PDO参数绑定）？直接拼接SQL字符串是红线。

// ❌ 不安全
$sql = "SELECT * FROM users WHERE id = " . $_GET['id'];
// ✅ 安全
$stmt = $pdo->prepare("SELECT * FROM users WHERE id = :id");
$stmt->execute(['id' => $_GET['id']]);

XSS（跨站脚本攻击）：所有输出到HTML的用户数据是否用了 htmlspecialchars() 或模板引擎的自动转义（如Blade的）？
文件上传：是否校验了文件类型（不只是扩展名，需检测MIME/幻数）？是否将上传目录限制在Web根目录外？
CSRF（跨站请求伪造）：所有执行“写”操作的POST请求是否有CSRF Token验证？
权限校验：用户能否越权访问他本不该访问的资源（例如普通用户改管理员密码）？

代码质量与可维护性

命名规范：变量、函数、类名是否遵循PSR规范（驼峰/下划线）？命名是否表达了其用途（$userAge 而不是 $data1）？
DRY原则：是否存在大量重复代码？应该抽离成函数或类。
函数/方法长度：一个方法是否超过100行？长方法往往违反了“单一职责原则”。

复杂度：if-else嵌套超过3层？建议使用提前返回或策略模式。

// ❌ 深层嵌套
if ($a) {
    if ($b) {
        // ...
    }
}
// ✅ 提前返回
if (!$a) {
    return;
}
if (!$b) {
    return;
}
// ...

魔术方法滥用：__get, __set, __call 是否被滥用？这会降低代码的可读性和静态分析效果。

类型系统（现代PHP核心）

类型声明：函数参数和返回值是否声明了类型（string、int、array、?User）？
严格模式：文件开头是否声明了 declare(strict_types=1);？强烈建议开启，避免隐式类型转换带来奇怪Bug。
PHPDoc：复杂的数组结构（如 array<User>）是否有规范的 @param 和 @return docblock？

性能与资源

数据库N+1查询：例如在循环内查询数据库。

// ❌ N+1 问题
$users = User::all();
foreach ($users as $user) {
    echo $user->profile->bio; // 每次循环都查一次数据库
}
// ✅ 预加载
$users = User::with('profile')->get();

大数组复制：是否不小心通过引用传递了大数组？
未释放的资源：文件句柄、数据库连接是否关闭了？（现代框架通常自动处理，但原生PHP要注意）

数据库交互

索引的使用：WHERE 或 ORDER BY 中的字段是否考虑过加数据库索引？
避免SELECT *：是否只查询了需要的列（SELECT id, name）？
事务：涉及多表更新或删除的操作，是否包裹在事务中（DB::transaction(...)）？

审查流程（怎么做）

通读业务逻辑：先理解代码意图，再看写法，如果看不懂，说明命名或架构需要优化。
运行代码（如果可能）：对于逻辑复杂的代码，可以在本地拉取分支运行，检查输出和边缘情况。
按优先级检查：速度 > 高（安全、错误逻辑） > 中（代码规范、性能） > 低（代码风格）。
发表评论：使用工具（如GitLab/GitHub PR Review）在对应行评论。
- 避免“你应该”：多用开放式问题，这里用 strpos 处理用户输入是否考虑过编码问题？”
- 提供示例：建议改成 preg_match() 时，给出具体正则表达式。
- 区分级别：标注是“必须修改（BUG/安全）”还是“建议优化（性能/可读性）”。

常见的PHP“坑”与检查清单

问题类型	检查点	示例
弱类型	vs	`empty($a)` vs `$a === ''`
未定义变量/数组键	是否用了 `isset()` 或	直接 `$arr['key']` 而非 `$arr['key'] ?? null`
时区问题	时间处理是否用 `Carbon` 或 `DateTime`？	直接 `date()` 可能忽略时区配置
错误抑制	是否出现符号？	`@file_get_contents()` 应替换为 `try-catch`
不安全的反序列化	`unserialize()` 是否接收了用户输入？	高危险，可能触发代码执行
Composer依赖	`composer.lock` 是否提交了？是否引入了过时版本？

给审查者的建议

不要当“语法警察”：格式问题交给 php-cs-fixer 自动修复，审查聚焦在逻辑和架构。
一次审查不要超过300-400行：超过1小时注意力会下降，建议分多次审查。
保持尊重：必要时开语音沟通，文字容易产生误解。

一份实用的PHP代码审查Checklist

[安全] 所有用户输入是否经过过滤或转义？
[安全] SQL语句是否使用了参数绑定？
[逻辑] 是否存在明显的边界条件错误（如除零）？
[质量] 方法是否过长？类是否过大？
[质量] 是否有死代码（注释掉的代码）？
[类型] 函数参数和返回值是否加了类型声明？
[性能] 循环内是否存在数据库查询（N+1）？
[错误处理] 所有可能抛出异常的地方是否被 try-catch 覆盖？
[依赖注入] 是否使用 new 关键字硬编码依赖？（建议用容器注入）
[测试] 新功能是否包含单元测试或功能测试？

开始实践时,建议先从 安全性 和 明显的逻辑错误 入手，然后再逐步关注代码结构和性能。