AI代码审查20个告警为何只认15个？老项目实战复盘

上个月我把一个2022年落地的Java老项目完整跑了一遍AI代码审查工具最后弹出了20个告警。这个数字让我觉得“还挺解渴”但当我坐下来一个个复核真正能让我认下并排期去修的只有15个。剩下的5个不是代码没问题而是AI在缺少业务上下文时犯了一种非常典型的“看着有道理、实际不适用”的错误。这篇不聊AI替代人类这种大话就复盘那次审查20个坑是什么、为什么我只认15个、以及老项目到底该怎么借力AI。1. 20个告警是怎么冒出来的项目、工具和首轮清单1.1 被审查的“老项目”是什么来头这个项目是2022年春天组装的Spring Boot 2.3单体服务JDK 8Maven多模块结构代码量大概15万行。业务是交易中台下游的对账与报表系统核心逻辑集中在几个大Service里数据库用了MySQL和MyBatis缓存用的Redis批处理任务占了不小比重。选它做AI审查原因很直接它不是那种满是EJB、JSP的遗产代码但已经具备老项目的典型特征——多人维护过、风格不统一、核心Service类偏大、注释有但覆盖不足、外部接口文档缺失。这种项目拿给AI扫最能看出工具的真实水平既有明确的技术债也有大量需要业务知识才能判断的灰色地带。1.2 第一轮扫描我用了什么姿势我用的是内网接入的AI评审通道底层大概是“静态规则引擎 代码大模型”的组合规则引擎负责把模式匹配类的问题捞出来比如未关闭资源、硬编码密码、MyBatis${}拼接大模型再对这些告警做语义解释和归类顺便补一些它认为值得关注的设计问题。扫描之前我把规则集调到偏向“安全、并发、事务、资源、性能”这几类主动忽略了一堆纯代码风格项。全量扫描耗时大约十分钟最终输出的告警就是20个。每个告警都带文件路径、行号、问题描述、置信度和修改建议形式上非常完整。1.3 AI报出来的20个问题长这样下面这张表是我把AI给的问题归纳之后的完整清单基本情况是这样编号AI报出来的问题典型位置我复核后的结论01MyBatis使用${}拼接用户输入UserMapper.xml真实问题SQL注入02线程池用Executors创建未管理生命周期ReportService.java真实问题03Map.get结果未判空就调用方法ConfigCache.java真实问题04内部方法自调用导致Transactional不生效OrderService.java真实问题05文件读取流未关闭FileImportTask.java真实问题06数据库密码硬编码在配置文件application.yml真实问题07日志打印用户手机号明文UserController.java真实问题08列表循环内逐条查询数据库BatchSyncService.java真实问题N109全局SimpleDateFormat被多线程共享DateUtils.java真实问题10Integer用比较CartController.java真实问题11使用原生ObjectInputStream反序列化CacheLoader.java真实问题12catch块只printStackTrace或留空SmsClient.java真实问题13synchronized方法粒度过大InventoryService.java真实问题14HTTP客户端未设置超时时间CityWeatherClient.java真实问题15Redis写入未设置过期时间TokenStore.java真实问题16魔法数字status3建议换枚举OrderServiceImpl.java不认可风格建议当缺陷17process方法300行建议拆分MainService.java不认可拆了更痛18用System.out.println输出DataFixMain.java不认可一次性脚本可以豁免19使用Date而不是LocalDateTimeReportDTO.java不认可兼容成本被忽略了20循环里Thread.sleep拖慢性能FileWatcher.java不认可轮询逻辑本身需要sleep坦白讲这份清单质量已经很高覆盖了SQL注入、并发、事务、资源泄漏、性能、安全合规这些方向。但“质量高”和“全部可认”是两码事。真正的问题从第2章开始。2. 老炮的人工复核5个“坑”其实是伪命题2.1 魔法数字AI不知道3就是“已支付”AI在OrderServiceImpl里发现了一处if (order.getStatus() 3)建议定义枚举替换。单看代码这个建议完全正确魔法数字确实是坏味道。但问题在于项目里已经有OrderStatusEnum而且PAID.getCode()返回的正好是3。这个3在业务上下文里人人皆知强行改成枚举当然更好但它不是会让系统崩溃的坑更像一个需要顺手清理的风格问题。更要命的是如果按AI建议直接改成枚举遇到Integer类型还要小心包装类拆箱问题。老炮不认这一条不是反对用枚举而是反对把“代码整洁度”和“致命缺陷”混在一个优先级里。这种问题应该进待办清单但不值得拉一个修复排期。2.2 300行大方法拆开之后可能更痛AI对MainService.process()报了一个“方法过长建议拆分”的告警说这个方法有300多行。我专门翻了一遍它其实是一段交易状态流转脚本先校验参数再查上下游数据接着按业务分支计算最后统一落库并写对账日志。方法内部共享了十几个状态变量并且整个方法处于同一个事务边界内。如果机械地拆成validate()、calculate()、persist()这些状态变量要么变成方法参数传一圈要么被提成类的成员变量。后者会直接让Service变成有状态对象这在Spring默认单例下是并发安全隐患前者则会让调用链看起来更长可读性不一定提升。所以这条我也没认。老项目里真正该拆的是那种“什么都做、调用链模糊”的方法而不是“步骤明确、顺序固定”的事务脚本。2.3 System.out.println一次性脚本的豁免条款AI报了一个批处理类用了System.out.println建议换成slf4j。这个文件是DataFixMain名字就说明它是历史数据修复用的临时入口跑完就被丢弃不进入生产主链路。项目日志框架在这个场景下反而不友好了如果换成logback还要额外指定输出位置和格式否则修复日志混在生产日志里后续排查反而更乱。我的态度很明确新增代码、生产主代码里出现System.out.println直接打回但一次性脚本可以豁免。AI的问题在于它只看代码看不到这个类是不是会被长期保留也看不到它和主系统的边界。这种告警如果全盘照收团队会因为一个临时工具花掉不必要的半小时。2.4 Date替换成LocalDateTime升级建议不是bugAI对ReportDTO里的java.util.Date提出了替换建议认为应该用java.time.LocalDateTime。这个建议放在2025年看是正确的技术方向但放在这个项目里替换成本很高DTO要对应MySQL的datetime字段MyBatis有typeHandler前端序列化对Date和LocalDateTime的输出格式不一样接口文档里还写明了“yyyy-MM-dd HH:mm:ss”格式。一旦单独替换一个字段整个链路的回归测试都要跟着走一遍。与其在AI审查时顺手改不如把它登记成“JDK日期API迁移”专项债统一评估。这不是说Date是对的而是说“值得修的坑”和“需要规划的技术升级”是两种东西AI把它们混为一谈了。2.5 循环里的sleep轮询设计本身就是需要它AI报了一个FileWatcher里的循环代码大概长这样while (true) { File f findLatestFile(); if (f ! null) { processFile(f); } Thread.sleep(5000); }AI的结论是循环内sleep会导致线程空转、影响性能。但实际背景是上游系统没有回调机制文件生成时间不可控这个线程必须每隔5秒去探一次目录sleep是轮询节奏的一部分。真正应该补的是“最大等待时间”和“线程中断处理”而不是删掉sleep。比如可以增加一个超过30分钟仍未就绪时的告警或者在InterruptedException时清理资源退出循环。AI盯着sleep不放反而把有价值的改进点漏掉了这恰恰说明它只能做模式判断做不了业务场景推理。2.6 五个被否掉的共性它们不是缺陷是“建议被当成了缺陷”这五个被否掉的问题没有一个是语法错误也没有一个会在当前环境下引发线上事故。它们要么是风格建议要么是技术升级建议要么是脱离业务上下文后产生的误判。25%的误报率不算高但如果不加复核直接让AI按20条开改团队会被无意义的MR噪音淹没AI审查的可信度也会快速消耗掉。3. 真正值得动手修的15个坑按优先级排个序3.1 第一梯队会炸的——安全、事务和空指针第一梯队是那种“不修迟早出事”的问题我全部认了。最简单的例子是MyBatis里用${}拼接查询条件select idqueryUser resultTypeUser SELECT * FROM user WHERE name LIKE %${keyword}% /select${}是字符串直接替换用户输入一旦包含SQL片段整条语句就会被注入。修复方式就是改成#{keyword}利用预编译占位符让数据库把输入当参数处理。这个属于零成本修复优先级最高。同梯队的还有application.yml里硬编码数据库密码、用ObjectInputStream反序列化外部输入、日志里打印手机号明文、Map.get()结果直接调用方法以及catch块里只写e.printStackTrace()的空处理。后两者单独说一下。Map.get()不判空在Java老项目里太常见了String name userMap.get(userId).getName();只要key不存在这一行就能让接口直接500。修复也简单用Objects.nonNull判断或者改用getOrDefault配合默认值。catch块吞异常则是另一个极端把异常打印到控制台后业务逻辑继续往下走最后数据错了大家还找不到原因。至少要把异常记录到日志框架里并重新抛出包装异常或者显式降级。3.2 第二梯队会慢慢烂掉的——并发、资源与性能第二梯队是那种“当时不炸、负载上来或运行久了必炸”的问题。第一个是循环内逐条查数据库典型N1for (Order order : orders) { ListOrderItem items orderItemMapper.selectByOrderId(order.getId()); // ... }假设一次对账要处理500个订单这个循环就会发出500条查询。修复思路是先把订单ID收集起来用WHERE order_id IN (...)一次查出再在内存里按订单ID分组组装。查询次数从N1降到2整个批处理速度能提升一个量级。第二个是全局静态的SimpleDateFormat。老项目里常见写法是private static final SimpleDateFormat SDF new SimpleDateFormat(yyyy-MM-dd HH:mm:ss);SimpleDateFormat本身不是线程安全的线上高并发时会出现解析错乱甚至抛出NumberFormatException。要么用ThreadLocal包一层要么直接换成DateTimeFormatter它是不可变且线程安全的private static final DateTimeFormatter DTF DateTimeFormatter.ofPattern(yyyy-MM-dd HH:mm:ss);第三个是线程池直接Executors.newFixedThreadPool(10)。这种写法的问题在工作队列是无界的如果任务持续积压内存里会堆积大量等待任务。更麻烦的是很多老代码只创建线程池从不调用shutdown应用重启几次之后线程变成僵尸线程。建议改成ThreadPoolExecutor并显式指定队列大小、拒绝策略和线程名前缀。同梯队的还有IO流不关闭、Transactional自调用失效、synchronized方法粒度过大、HTTP调用不设超时。事务自调用我用一个简短例子说明Service public class OrderService { public void createOrder() { // ... this.updateStock(); // 走this调用事务注解失效 } Transactional public void updateStock() { // ... } }Spring事务是靠代理对象实现的this调用绕过了代理Transactional自然不生效。解决办法是把updateStock挪到另一个Bean里或者注入OrderService的代理对象再调用。这类问题AI能报出来确实帮了大忙因为人肉看代码经常一眼扫过。3.3 第三梯队会让人骂娘的——可维护性和隐蔽正确性第三梯队的问题不影响功能正确但会影响后续维护质量。最典型的是Integer用比较Integer a 128; Integer b 128; if (a b) { // 这里可能是false }Integer在-128到127之间有缓存超出范围就会走对象比较结果不可控。统一改成Objects.equals(a, b)或者把比较的字段改成基本类型int。这种问题在老项目里经常以“偶发的线上bug”形式出现排查成本远高于修改成本。另一个是Redis key写入时没有设置过期时间。比如一个token存储类redisTemplate.opsForValue().set(key, value);如果key永远不过期长期运行后Redis内存会被撑爆或者旧值一直生效导致逻辑错乱。现在写入时基本要求带上过期时间和时间单位至少也要有兜底清理策略。这份清单里的最后一条我认下不是因为AI的说法有多新而是它确实击中了我自己知道但一直没排期的地方。3.4 附15个真坑的核对清单为了方便维护我把15个问题重新分了个类你可以直接拿去做成跟进表格分类问题列表安全与合规MyBatis${}注入、密码硬编码、原生反序列化、日志明文手机号并发与正确性SimpleDateFormat线程不安全、Integer比较、线程池无界/未关闭、synchronized粒度过大事务与资源自调用事务失效、IO流未关闭、HTTP请求无超时稳定性空指针未判空、空catch吞异常、Redis key无TTL、循环查询N14. AI为什么能挑出这20个坑以及它为什么会误报4.1 规则扫描与大模型判断的分工AI审查能拿到20个结果本质上是两种能力叠加。规则引擎负责确定性问题比如${}、System.out.println、catch{}都是明确的模式命中就是命中不会含糊。大模型负责需要语义理解的场景比如“这个方法看起来太长”“这个sleep可能导致性能问题”它能给出一个人类语言解释但解释本身可能是基于片面的代码片段脑补出来的。所以AI审查结果的质量取决于底层规则集好不好以及大模型在解释时有没有足够的上下文。我在第一轮扫描时把规则集收敛到“安全、并发、事务、资源、性能”等于提前过滤了一批纯风格建议。但大模型那一层还是会带出风格类问题因为它在预训练时学到的“好代码标准”和团队实际的“项目约定”并不总是一致。4.2 AI漏了什么又为什么误报AI的误报集中在两个根源一是缺少业务上下文二是缺少调用链全景。第16到20号问题几乎都属于这两种情况。它不知道3是枚举值不知道300行方法是事务脚本不知道批处理Main是一次性的不知道Date的替换会牵动整条链路也不知道sleep是轮询节奏的组成部分。反过来AI也会漏真正关键的问题。比如某个对外接口没有做幂等保护、批量任务失败后没有断点续跑逻辑、库存扣减没有乐观锁这些很难通过单文件分析发现因为必须理解系统整体交互。所以AI审查最有价值的定位不是“结论”而是“线索”。4.3 把AI调教成更靠谱的初审员如果你也想在老项目上跑AI审查我建议先在提示词或规则说明里写清楚边界。我在第二轮扫描时给大模型加过一段约束大意是请忽略纯风格建议和枚举命名建议聚焦以下四类问题空指针与资源泄漏、并发与线程安全、事务与数据一致性、敏感信息与外部接口风险。对每个问题给出具体触发路径和文件行号如果你无法确定该问题在当前业务上下文中是否成立请标记为“需人工确认”不要直接列为严重问题。加了这段之后误报率明显下降因为大模型会倾向于把不确定的内容单独归类而不是和真实缺陷混在一起。对存量老项目还可以在扫描配置里排除临时目录、脚本目录和已经标记为技术债的模块避免每次全量扫描都重复报同一批噪音。5. 把AI审查接进老项目维护流程我的最终建议5.1 别让AI替代人审而是让AI帮你拉网对一个老项目来说最缺的不是“发现问题的能力”而是“快速过滤噪音的能力”。AI能在一小时内扫完人肉一周看不完的代码这已经值回票价。但它不熟悉业务上下文也不了解团队历史约定所以正确的用法是让AI先拉网把可疑点全部标记出来然后由真正熟悉业务的人做二审把“真坑”和“风格建议”分开最后再进入正常的缺陷管理流程。我在这次审查里真正花时间的不是扫描而是二审。每个告警我都会点开文件看一眼上下文再决定认不认。AI把20个问题摆到桌面上大大缩短了我寻找线索的时间但“哪些该修、哪些该缓、哪些该清理”这件事AI还做不了决定。5.2 建立“人工复核-标签沉淀”小闭环一次扫描不算完最好把结论沉淀回工具里。我给这次审查加了三个标签“真实缺陷”“误报-风格建议”“误报-上下文不匹配”。下次再扫描时同一位置的同类告警就可以根据历史标签自动归类人工只需要看新的和变化的告警。这样做的好处是AI会越用越顺手团队也会逐渐积累起一套针对自己项目的“审查规则偏好”。比如这个项目里一次性脚本目录可以永久排除status 3这类代码可以进入“低优先级技术债”而不是“严重缺陷”团队对新告警的信任度也会提高。5.3 三个实操技巧拿去就能用第一别一把梭全仓扫描先挑一个核心模块试跑。全量扫描的噪音量可能会吓退团队但单模块的20个告警是可以坐下来逐个聊完的。第二要求AI给出“触发路径”而不只是问题名称。一条真正有用的告警应该是“在这个接口、这个参数下会发生什么”而不是一句“存在SQL注入风险”。第三关键模块的审核至少两个人参与一个熟悉业务的老手负责定性一个刚接触代码的新人负责跑AI结果新人既能学知识老手也能借AI提效。我自己的体会是AI代码审查更像一个新来的实习生扫东西很快但判断力需要人带。20个候选里只认15个不是否定AI的价值而是我们终于有时间把剩下5个为什么不算坑解释清楚这本身就是一次很好的团队复盘。