AI 代码审查实战：2022年Java老项目挑出20个坑，老炮只认15个

老炮踩坑录 · A02 · AI 时代系列

· 基于「企业融合评估平台」真实源码

· 关键词：AI 代码审查 · 误报分析 · 人机对照 · 遗留系统

上一篇（A01），我把一个 1600 行的 Controller 喂给 AI 做重构建议，结论是"方案看起来专业，但一条都没敢用"。

有读者私信留言："如果把整个项目交给 AI 做代码审查呢？它总不能全错吧？"

这是个好问题。于是我做了这个实测：把整个项目的 Java 源码交给 AI ，让它做一次完整的代码审查，目标是找出 20 个"坑"——安全漏洞、性能隐患、设计缺陷等等都算。

然后我逐条人工复核。

结果：AI 报了 20 个，我认了 15 个，否了 5 个。命中率 75%。

看起来还不错？但仔细看那 5 个误报，你会发现 AI 犯的错误有一个共同规律——它总是"技术上正确，上下文里错误"。这个规律比命中率本身更值得探讨。

测试方法

1. 把 src/main/ 下全部源码交给 AI，提示词："对这个项目做一次代码审查，找出 20 个最严重的问题，按严重程度排序。安全问题 > 稳定性问题 > 性能问题 > 代码规范问题。"
2. AI 输出 20 条，我逐条对照源码做人工复核。
3. 判定标准：
   • 真坑：问题确实存在，且确实有危害，我同意改
   • 半对：问题存在，但 AI 的严重性判断标准或解决方案不对
   • 幻觉：AI 报的问题在实际代码中不成立

人机对照表

先看完整结果，再逐类展开：

#	AI 报的问题	涉及文件	老炮判定
1	fastjson 1.2.37 存在反序列化远程代码执行漏洞	pom.xml	✅ 真坑
2	Guava 20.0 是 2014 年版本，严重过时	pom.xml	✅ 真坑
3	AES 使用 ECB 模式，相同明文产生相同密文	AesEncodeUtil.java	✅ 真坑
4	AES 密钥硬编码在源码中	AesEncodeUtil.java	✅ 真坑
5	25+ 处 e.printStackTrace() 替代日志框架	全项目	✅ 真坑
6	Executors.newFixedThreadPool 使用无界队列，存在 OOM 风险	3 个文件	✅ 真坑
7	ThreadLocal 静态变量跨请求数据污染	AsyncService.java	✅ 真坑
8	@Transactional + try-catch 导致事务静默失效	UserController.java	✅ 真坑
9	10 处 @Transactional 缺少 rollbackFor	全项目	✅ 真坑
10	Guava Cache maximumSize=50 会踢出在线用户	AuthAspect.java	✅ 真坑
11	@Scheduled 定时任务裸 catch，失败无告警	SchedulerTask.java	✅ 真坑
12	synchronized 锁整个类，并发性能瓶颈	AsyncService.java	✅ 真坑
13	定时任务用 System.out.println 代替日志	SchedulerTask.java	✅ 真坑
14	多处 catch 后 return null，前端收到空响应	多个 Controller	✅ 真坑
15	SimpleDateFormat 线程不安全	多个 Service	⚠️ 半对
16	@Transactional 放在 Controller 层，违反分层原则	UserController.java	⚠️ 半对
17	JSONObject 贯穿全链路，应使用强类型 DTO	全项目	❌ 幻觉
18	定时任务中 count++ 存在线程安全问题	SchedulerTask.java	❌ 幻觉
19	Controller 大量 return null，违反 RESTful 规范	多个 Controller	❌ 幻觉
20	ObsClient 未在 finally 中关闭，资源泄漏	AsyncService.java	❌ 幻觉

命中率：15 真 + 1 半对 = 16 条有效 / 20 条总计 = 80%。 如果只算"完全正确"，是 15/20 = 75%。

看起来不错？但关键不在那 15 个对的——那 15 个对的，任何一个有经验的 Java 开发都能找出来。AI 的价值不在于"找到了已知的问题"，在于"在找到的问题里有多少是真的需要修改的"。

下面我们逐一展开分析。

AI 找对的 15 个真坑

这 15 个我不逐条分析了——每条都是实锤，直接上证据。

安全类（4 个）

#1 fastjson 1.2.37 反序列化漏洞

<!-- pom.xml -->
<artifactId>fastjson</artifactId>
<version>1.2.37</version>

fastjson 1.2.68 之前存在多个 autoType 绕过漏洞（CVE-2019-17571 等），攻击者可以通过构造恶意 JSON 实现远程代码执行。这是最高优先级的修复项，没有之一。

完整漏洞如下图：

#3 AES/ECB 模式

// AesEncodeUtil.java
public static final String AES_TYPE = "AES/ECB/PKCS5Padding";

ECB 模式对相同的明文块总是产生相同的密文块，攻击者可以通过密文模式分析推断明文结构。前后端登录密码加密用的就是这个——AES/ECB/PKCS5Padding，密钥还写死在前端 JS 里。应改为 AES/CBC 或 AES/GCM（推荐）。

#4 AES 密钥硬编码

// AesEncodeUtil.java
private static final String AES_KEY = "t0b3pevklwcsd";

密钥直接写在源码里，任何能访问代码仓库的人都能拿到。应该通过配置中心或环境变量注入。

稳定性类（7 个）

#5 25+ 处 printStackTrace

全项目扫出来 25 处 e.printStackTrace()，分布在 ExcelUtil、FileDownloadUtil、AppInfoServiceImpl 等多个文件。异常进了 stdout 黑洞，生产环境搜日志根本搜不到。应该统一用 log.error("msg", e)。

#6 Executors.newFixedThreadPool 无界队列

// AsyncElsServiceImpl.java, HWFileServiceImpl.java 等 3 处
ExecutorService executorService = Executors.newFixedThreadPool(5);

newFixedThreadPool 内部用的是 LinkedBlockingQueue（无界），任务堆积时队列无限增长，最终 OOM。阿里 Java 开发手册第一条规约就禁止了这种写法。应改用 ThreadPoolExecutor 手动指定有界队列。

#7 ThreadLocal 静态变量跨请求污染

// AsyncService.java
public static final ThreadLocal<Map<String, Object>> threadLocal = new ThreadLocal<>();

static + ThreadLocal + 线程池复用 = 上一个请求的上下文数据泄漏到下一个请求。在 Tomcat 线程池环境下，用户 A 的登录数据可能出现在用户 B 的请求里。

#8 @Transactional + try-catch = 事务失效

// UserInfoController.java
@Transactional(rollbackFor = Exception.class)
public Object deleteReport(HttpServletRequest request) throws Exception {
    try {
        userInfoService.deleteObject(enterpriseId);
        diaService.delete(enterpriseId);
        // ...
    } catch (Exception e) {
        e.printStackTrace();
        return ResultInfo.setResultInfo(true, 500, "error", "删除失败", null);
    }
}

catch 把异常吞了，Spring 看不到异常，事务不会回滚。删了一半数据，数据库里就是脏数据。这是本次审查中最隐蔽的 bug——代码看起来有 @Transactional，看起来有 rollbackFor，但实际效果等于没加。

#9 10 处 @Transactional 缺少 rollbackFor

// EnterpriseRegistServiceImpl.java, LoginServiceImpl.java 等 10 处
@Transactional // 没有 rollbackFor = Exception.class

Spring 默认只对 RuntimeException 回滚。如果方法内部抛了受检异常（IOException 等），事务静默不回滚。应统一加 rollbackFor = Exception.class。

#10 Guava Cache maximumSize=50 踢人

// AuthAspect.java
private static final Cache<String, Object> CACHES = CacheBuilder.newBuilder()
    .maximumSize(50)
    .expireAfterWrite(30, TimeUnit.MINUTES)
    .build();

用本地缓存做 Session 恢复，最大只存 50 条。第 51 个用户登录时，最早的那条就被踢出去了——该用户下次请求会被判定为"未登录"，强制踢下线。并发稍高就会出事。

#11 @Scheduled 裸 catch，定时任务静默死亡

// SchedulerTask.java
@Scheduled(cron = "0 0/3 * * * ?")
private void processReportResult() {
    // ... 130 行业务逻辑 ...
    } catch (Exception ex) {
        log.error(ex.getMessage(), ex);
    }
}

每 3 分钟跑一次的同步任务，catch 了所有异常只打日志。如果某天外部诊断系统的接口地址变了，这个任务会持续失败，但没有任何告警——只有人去看日志才能发现。数据断了三个月，没人知道。

性能类（2 个）

#12 synchronized 锁整个类

// AsyncService.java
synchronized (HWFileServiceImpl.class) {
    params.put("enterpriseid", enterpriseid);
    applyFileService.modifyFileName(params);
}

锁的是 HWFileServiceImpl.class——所有线程、所有用户共用一把锁。任何时刻只有一个线程能执行这段代码。在高并发上传场景下，这是一个串行化性能瓶颈。

#13 System.out.println 代替日志

// SchedulerTask.java
System.out.println("start " + (count++));
System.out.println("end" + (count++));

定时任务用 System.out 输出，没有日志级别、没有时间戳、没有日志文件。生产环境排查问题时，stdout 的内容早就被日志轮转清掉了。

数据处理类（2 个）

#14 catch 后 return null

// EnterpriseRegistController.java 的多个导出方法
try {
    // ... 导出逻辑 ...
    return null;
} catch (Exception e) {
    log.error(e.getMessage(), e);
    return null;
}

成功返回 null，失败也返回 null。前端拿到 null，不知道是 "没有数据" 还是 "出错了"。应该至少返回一个带错误码的 Result 对象。

AI 找错的 5 个——这才是最有价值的部分

#15 SimpleDateFormat "线程不安全"——判对了对象，搞错了位置

AI 说：

SimpleDateFormat 是线程不安全的，项目中多处使用存在并发风险。

实际情况：

// ReportPbServiceImpl.java 等多处
SimpleDateFormat sdf = new SimpleDateFormat("yyyyMMddHHmmss");

项目里每个方法都是 new 出来的局部变量，根本不存在共享，也就不存在线程安全问题。

AI 的规则库告诉它 "SimpleDateFormat = 线程不安全"，但它没有去看这个变量是 static 共享的还是局部创建的。

真正的浪费不是线程安全，而是每个方法都 new 一个 formatter——完全可以提取成 ThreadLocal 或 DateTimeFormatter（线程安全）复用。但 AI 没看到这个优化点。

误报原因：AI 按关键词匹配规则，不看变量的作用域。

#16 @Transactional 放在 Controller 层——原则有错，但能工作

AI 说：

@Transactional 不应该出现在 Controller 层，违反分层原则。应该下沉到 Service。

// UserInfoController.java
@Transactional(rollbackFor = Exception.class)
public Object deleteReport(HttpServletRequest request) throws Exception {

实际情况： Spring 的 @Transactional 是基于 AOP 代理实现的，放在 Controller 层技术上完全能工作。只要方法不是被同一个类内部调用（自调用），事务就能正常生效。

这条建议 "原则上正确"，但严重性被 AI 高估了——它不是 bug，只是代码组织问题。在遗留系统里，为了 "分层纯洁性" 去改一个能正常运行的事务边界，风险大于收益。

误报原因：AI 把"代码规范问题"和"功能缺陷" 混为一谈，严重性判断标准有问题。

#17 JSONObject 贯穿全链路——技术上对，上下文里错

AI 说：

全链路使用 JSONObject 作为方法参数，丢失类型安全。应使用强类型 DTO。

实际情况： 这条我在上一篇文章（A01）里详细分析过。JSONObject 不是某一个接口的偷懒，而是贯穿 Controller → Service → Mapper → XML 的架构决策。只改 Controller 层反而增加转换成本；要改就得改 80 个文件。

在一个没有测试的项目里，这种"全链路改造"就是赌博。 AI 不知道这个项目有多少文件、有没有测试、改动一个参数类型会波及多少层。

误报原因：AI 只看代码形式，不看改造成本和项目约束。

#18 count++ "线程安全问题"——不存在

AI 说：

SchedulerTask 中的 count 字段使用 count++，在多线程环境下存在竞态条件。

// SchedulerTask.java
private int count = 0;

@Scheduled(cron = "0 0 0 * * ?")
private void process() {
    System.out.println("start " + (count++));
}

实际情况： Spring 的 @Scheduled 默认使用 ScheduledThreadPoolExecutor，核心线程数是 1。也就是说，同一时刻只有一个定时任务在执行，根本不存在并发。count++ 在这个场景下是安全的。

AI 看到 "共享可变状态 + 非原子操作" 就报线程安全，但它不知道 Spring 定时任务的默认调度模型是怎么的。

误报原因：AI 不了解框架的默认行为，把理论风险当成了实际 bug。

#19 Controller return null "违反 RESTful 规范"——它已经通过 response 写完了

AI 说：

多个导出方法返回 null，违反 RESTful 规范，应返回合适的 HTTP 状态码。

@AuthBack
@PostMapping("back/export")
public Result backExport(HttpServletResponse response, @RequestBody JSONObject params) {
    try {
        // ... 通过 response.getOutputStream() 写入 Excel 二进制流 ...
        elecExcelExportUtil.exportByTpl(response, 1, "企业清单");
        return null;  // ← AI 说这里不该返回 null
    } catch (Exception e) {
        log.error(e.getMessage(), e);
        return null;
    }
}

实际情况： 这个方法通过 HttpServletResponse 直接写入了 Excel 二进制流。到 return null 时，HTTP 响应已经提交（状态码 200 + Content-Type + 文件内容）都写完了。Spring 检测到 response 已提交，不会再尝试序列化返回值。return null 在这里不会造成任何问题。

这是文件下载场景的常见写法，不算优雅，但也不是 bug。

误报原因：AI 不理解 HTTP 响应的生命周期，只看方法签名的返回值。

5 个误报背后的共同规律

把 5 个误报放在一起看：

误报	AI 的推理	实际情况	共同规律
SimpleDateFormat 线程不安全	看到类名就触发规则	局部变量，不共享	不看作用域
@Transactional 在 Controller	违反分层原则	能正常工作	不看运行结果
JSONObject 全链路	应该用 DTO	改动量 = 重写半个项目	不看改造成本
count++ 线程安全	共享可变状态	单线程调度，无并发	不看框架默认行为
return null 违反规范	返回值是 null	response 已提交	不看 HTTP 生命周期

5 个误报，100% 是同一个原因：AI 只看到了代码的静态"形式"，没有看到代码的"运行时上下文"。

它知道 SimpleDateFormat 在教科书上是线程不安全的，但不知道这个变量是方法内的局部变量。它知道 @Transactional 应该放在 Service 层，但不知道放在 Controller 层也能正常工作。它知道 JSONObject 不如 DTO 安全，但不知道改一个参数类型要波及 80 个文件。

这就是 AI 代码审查的天花板：它是一个非常博学的"规则匹配器"，但不是一个"理解运行时上下文的工程师"。

那 75% 的命中率够用吗？

回到最初的问题：AI 报了 20 个坑，命中率 75%，这个结果好不好？

我的判断：作为"辅助工具"够用，作为"独立审查者"不够。

够用的部分：AI 确实帮我快速定位了 fastjson 漏洞、Executors 无界队列、ThreadLocal 污染这些 "扫描类" 问题。这类问题的特征是模式固定、判断标准明确——要么有 CVE 编号，要么违反明确的规约。AI 做这种 "规则扫描" 比人快得多。

不够的部分：那 5 个误报，每一个都需要我花时间去验证。如果我对项目不够熟，很可能就信了——然后花时间去修一个不存在的问题，或者改一个能正常工作的代码。

更危险的是"漏报"——AI 没发现的问题。 这次测试里，AI 就没有发现：

• 登录接口 type 参数被 JS 硬编码覆盖（上篇 F02 的坑 4 讲过）
• 后台登录没设置 SESSION_COMPANY_TYPE 导致鉴权失败（坑 6）
• 雪花算法 workerId 硬编码导致多实例 ID 重复（上篇 F14 的坑）

这些问题不是 "代码模式" 问题，而是业务逻辑 + 跨文件调用链的问题。AI 的审查粒度到不了这一层，也是它的局限性。

怎么用 AI 做代码审查——我的实操建议

经过这次实测，我总结了一个 "人机协作" 的审查流程：

第一步：让 AI 做"规则扫描"（命中率最高）

把代码交给 AI，让它找：安全漏洞（CVE）、过时的依赖版本、明确的规约违反（阿里规约、SonarQube 规则）。这类问题 AI 的准确率接近 100%。

第二步：AI 的"设计建议"全部打问号

当 AI 说"应该用 XXX 模式"、"应该重构为 XXX"时，先问自己三个问题（和 A01 的"三问法则"一样）：

1. 有测试能证明改完之后行为不变吗？
2. 我理解这段代码背后的业务决策吗？
3. 改错了代价是什么？

第三步：人工补"漏报"

AI 扫不出来的问题是：跨文件的业务逻辑错误、前后端数据结构不一致、配置和代码的隐含约定。这些只有理解业务的人才能发现。

---

写在最后

回到标题的数字：20 个坑，认 15 个。

75% 的命中率，说明 AI 做代码审查确实有用——至少比 "不审查" 强得多。但那 25% 的误报和漏报提醒我们：AI 的审查报告不能直接当"施工图纸"用，它更像是一张"待验证的线索清单"。

每一条线索都需要一个懂业务、懂上下文、懂改造成本的人来判断：这是真坑，还是虚惊一场。

AI 能发现"代码里写了什么"，但只有人能判断"代码意味着什么"。

这也是为什么标题叫"老炮只认 15 个"——不是老炮比 AI 聪明，而是老炮知道哪些 "看起来像坑" 的东西，其实不是坑。

---

好了，今天的内容都讲完了。

这个系列的下一篇，打算聊聊 AI 生码率进 KPI 这件事。作为一个写了 18 年代码还在一线的人，我想说说：当 AI 写代码的速度成了考核指标，老炮的护城河到底在哪？

下期预告：《AI 生码率进 KPI 了，18 年老炮的三个保命技能》

AI审代码，我认了15个坑，剩下5个是幻觉。但比“AI能不能审代码”更现实的问题是：AI生码率已经进了KPI，末位淘汰不是段子。

下期讲三个老炮的保命技能：拆需求、验代码、兜底线。

我是老炮，18年Java老兵，仍在一线。关注「Java老炮踩坑录」，少踩坑。