老炮踩坑录 · A02 · AI 时代系列
· 基于「企业融合评估平台」真实源码
· 关键词:AI 代码审查 · 误报分析 · 人机对照 · 遗留系统
👋 欢迎阅读
🏠个人主页:知守观
📘我的专栏:老炮踩坑录
💻当前内容:AI代码审查实战
文章目录
- 测试方法
- 人机对照表
- AI 找对的 15 个真坑
- 安全类(4 个)
- 稳定性类(7 个)
- 性能类(2 个)
- 数据处理类(2 个)
- AI 找错的 5 个——这才是最有价值的部分
- #15 SimpleDateFormat "线程不安全"——判对了对象,搞错了位置
- #16 @Transactional 放在 Controller 层——原则有错,但能工作
- #17 JSONObject 贯穿全链路——技术上对,上下文里错
- #18 count++ "线程安全问题"——不存在
- #19 Controller return null "违反 RESTful 规范"——它已经通过 response 写完了
- 5 个误报背后的共同规律
- 那 75% 的命中率够用吗?
- 怎么用 AI 做代码审查——我的实操建议
- 写在最后
上一篇,我把一个 1600 行的 Controller 喂给 AI 做重构建议,结论是"方案看起来专业,但一条都没敢用"。
有读者私信留言:“如果把整个项目交给 AI 做代码审查呢?它总不能全错吧?”
这是个好问题。于是我做了这个实测:把整个项目的 Java 源码交给 AI ,让它做一次完整的代码审查,目标是找出 20 个"坑"——安全漏洞、性能隐患、设计缺陷等等都算。
然后我逐条人工复核。
结果:AI 报了 20 个,我认了 15 个,否了 5 个。命中率 75%。
看起来还不错?但仔细看那 5 个误报,你会发现 AI 犯的错误有一个共同规律——它总是"技术上正确,上下文里错误"。这个规律比命中率本身更值得探讨。
测试方法
- 把
src/main/下全部源码交给 AI,提示词:“对这个项目做一次代码审查,找出 20 个最严重的问题,按严重程度排序。安全问题 > 稳定性问题 > 性能问题 > 代码规范问题。” - AI 输出 20 条,我逐条对照源码做人工复核。
- 判定标准:
- 真坑:问题确实存在,且确实有危害,我同意改
- 半对:问题存在,但 AI 的严重性判断标准或解决方案不对
- 幻觉:AI 报的问题在实际代码中不成立
人机对照表
先看完整结果,再逐类展开:
命中率:15 真 + 1 半对 = 16 条有效 / 20 条总计 = 80%。如果只算"完全正确",是 15/20 = 75%。
看起来不错?但关键不在那 15 个对的——那 15 个对的,任何一个有经验的 Java 开发都能找出来。AI 的价值不在于"找到了已知的问题",在于"在找到的问题里有多少是真的需要修改的"。
下面我们逐一展开分析。
自查打卡:搜一下你的项目,grep -r “printStackTrace” src/ | wc -l,数字打在评论区。我看看谁的项目最“健康”。
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.javapublicstaticfinalStringAES_TYPE="AES/ECB/PKCS5Padding";ECB 模式对相同的明文块总是产生相同的密文块,攻击者可以通过密文模式分析推断明文结构。前后端登录密码加密用的就是这个——AES/ECB/PKCS5Padding,密钥还写死在前端 JS 里。应改为 AES/CBC 或 AES/GCM(推荐)。
#4 AES 密钥硬编码
// AesEncodeUtil.javaprivatestaticfinalStringAES_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 处ExecutorServiceexecutorService=Executors.newFixedThreadPool(5);newFixedThreadPool内部用的是LinkedBlockingQueue(无界),任务堆积时队列无限增长,最终 OOM。阿里 Java 开发手册第一条规约就禁止了这种写法。应改用ThreadPoolExecutor手动指定有界队列。
#7 ThreadLocal 静态变量跨请求污染
// AsyncService.javapublicstaticfinalThreadLocal<Map<String,Object>>threadLocal=newThreadLocal<>();static+ThreadLocal+ 线程池复用 = 上一个请求的上下文数据泄漏到下一个请求。在 Tomcat 线程池环境下,用户 A 的登录数据可能出现在用户 B 的请求里。
#8 @Transactional + try-catch = 事务失效
// UserInfoController.java@Transactional(rollbackFor=Exception.class)publicObjectdeleteReport(HttpServletRequestrequest)throwsException{try{userInfoService.deleteObject(enterpriseId);diaService.delete(enterpriseId);// ...}catch(Exceptione){e.printStackTrace();returnResultInfo.setResultInfo(true,500,"error","删除失败",null);}}catch 把异常吞了,Spring 看不到异常,事务不会回滚。删了一半数据,数据库里就是脏数据。这是本次审查中最隐蔽的 bug——代码看起来有@Transactional,看起来有rollbackFor,但实际效果等于没加。
#9 10 处 @Transactional 缺少 rollbackFor
// EnterpriseRegistServiceImpl.java, LoginServiceImpl.java 等 10 处@Transactional// 没有 rollbackFor = Exception.classSpring 默认只对RuntimeException回滚。如果方法内部抛了受检异常(IOException等),事务静默不回滚。应统一加rollbackFor = Exception.class。
#10 Guava Cache maximumSize=50 踢人
// AuthAspect.javaprivatestaticfinalCache<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 * * * ?")privatevoidprocessReportResult(){// ... 130 行业务逻辑 ...}catch(Exceptionex){log.error(ex.getMessage(),ex);}}每 3 分钟跑一次的同步任务,catch 了所有异常只打日志。如果某天外部诊断系统的接口地址变了,这个任务会持续失败,但没有任何告警——只有人去看日志才能发现。数据断了三个月,没人知道。
性能类(2 个)
#12 synchronized 锁整个类
// AsyncService.javasynchronized(HWFileServiceImpl.class){params.put("enterpriseid",enterpriseid);applyFileService.modifyFileName(params);}锁的是HWFileServiceImpl.class——所有线程、所有用户共用一把锁。任何时刻只有一个线程能执行这段代码。在高并发上传场景下,这是一个串行化性能瓶颈。
#13 System.out.println 代替日志
// SchedulerTask.javaSystem.out.println("start "+(count++));System.out.println("end"+(count++));定时任务用System.out输出,没有日志级别、没有时间戳、没有日志文件。生产环境排查问题时,stdout 的内容早就被日志轮转清掉了。
数据处理类(2 个)
#14 catch 后 return null
// EnterpriseRegistController.java 的多个导出方法try{// ... 导出逻辑 ...returnnull;}catch(Exceptione){log.error(e.getMessage(),e);returnnull;}成功返回 null,失败也返回 null。前端拿到 null,不知道是 “没有数据” 还是 “出错了”。应该至少返回一个带错误码的 Result 对象。
AI 找错的 5 个——这才是最有价值的部分
#15 SimpleDateFormat “线程不安全”——判对了对象,搞错了位置
AI 说:
SimpleDateFormat是线程不安全的,项目中多处使用存在并发风险。
实际情况:
// ReportPbServiceImpl.java 等多处SimpleDateFormatsdf=newSimpleDateFormat("yyyyMMddHHmmss");项目里每个方法都是 new 出来的局部变量,根本不存在共享,也就不存在线程安全问题。
AI 的规则库告诉它 “SimpleDateFormat = 线程不安全”,但它没有去看这个变量是 static 共享的还是局部创建的。
真正的浪费不是线程安全,而是每个方法都 new 一个 formatter——完全可以提取成ThreadLocal或DateTimeFormatter(线程安全)复用。但 AI 没看到这个优化点。
误报原因:AI 按关键词匹配规则,不看变量的作用域。
#16 @Transactional 放在 Controller 层——原则有错,但能工作
AI 说:
@Transactional不应该出现在 Controller 层,违反分层原则。应该下沉到 Service。
// UserInfoController.java@Transactional(rollbackFor=Exception.class)publicObjectdeleteReport(HttpServletRequestrequest)throwsException{实际情况: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.javaprivateintcount=0;@Scheduled(cron="0 0 0 * * ?")privatevoidprocess(){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")publicResultbackExport(HttpServletResponseresponse,@RequestBodyJSONObjectparams){try{// ... 通过 response.getOutputStream() 写入 Excel 二进制流 ...elecExcelExportUtil.exportByTpl(response,1,"企业清单");returnnull;// ← AI 说这里不该返回 null}catch(Exceptione){log.error(e.getMessage(),e);returnnull;}}实际情况:这个方法通过HttpServletResponse直接写入了 Excel 二进制流。到return null时,HTTP 响应已经提交(状态码 200 + Content-Type + 文件内容)都写完了。Spring 检测到 response 已提交,不会再尝试序列化返回值。return null在这里不会造成任何问题。
这是文件下载场景的常见写法,不算优雅,但也不是 bug。
误报原因:AI 不理解 HTTP 响应的生命周期,只看方法签名的返回值。
5 个误报背后的共同规律
把 5 个误报放在一起看:
5 个误报,100% 是同一个原因:AI 只看到了代码的静态"形式",没有看到代码的"运行时上下文"。
它知道SimpleDateFormat在教科书上是线程不安全的,但不知道这个变量是方法内的局部变量。它知道@Transactional应该放在 Service 层,但不知道放在 Controller 层也能正常工作。它知道 JSONObject 不如 DTO 安全,但不知道改一个参数类型要波及 80 个文件。
这就是 AI 代码审查的天花板:它是一个非常博学的"规则匹配器",但不是一个"理解运行时上下文的工程师"。
有个细节我没完全确认:AI审查的效果,在不同规模的项目上差异到底有多大?我测的是一个200多个文件的老项目,如果是几千行的小项目,误报率会不会低一些?有踩过坑的兄弟评论区指点一下。
那 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 的"三问法则"一样):
- 有测试能证明改完之后行为不变吗?
- 我理解这段代码背后的业务决策吗?
- 改错了代价是什么?
第三步:人工补"漏报"
AI 扫不出来的问题是:跨文件的业务逻辑错误、前后端数据结构不一致、配置和代码的隐含约定。这些只有理解业务的人才能发现。
写在最后
回到标题的数字:20 个坑,认 15 个。
75% 的命中率,说明 AI 做代码审查确实有用——至少比 “不审查” 强得多。但那 25% 的误报和漏报提醒我们:AI 的审查报告不能直接当"施工图纸"用,它更像是一张"待验证的线索清单"。
每一条线索都需要一个懂业务、懂上下文、懂改造成本的人来判断:这是真坑,还是虚惊一场。
AI 能发现"代码里写了什么",但只有人能判断"代码意味着什么"。
这也是为什么标题叫"老炮只认 15 个"——不是老炮比 AI 聪明,而是老炮知道哪些 “看起来像坑” 的东西,其实不是坑。
好了,今天的内容都讲完了。
这个系列的下一篇,打算聊聊 AI 生码率进 KPI 这件事。作为一个写了 18 年代码还在一线的人,我想说说:当 AI 写代码的速度成了考核指标,老炮的护城河到底在哪?
最后问一句:你用AI审过代码吗?它报的问题,你是照单全收,还是逐条复核?评论区聊聊,我看看有多少人敢把AI审查报告当施工图纸用。
下期预告:《AI 生码率进 KPI 了,18 年老炮的三个保命技能》
AI审代码,我认了15个坑,剩下5个是幻觉。但比“AI能不能审代码”更现实的问题是:AI生码率已经进了KPI,末位淘汰不是段子。
下期讲三个老炮的保命技能:拆需求、验代码、兜底线。
我是老炮,18年Java老兵,仍在一线。关注「Java老炮踩坑录」,少踩坑。