forked from datawhale/whale-town-end
578 lines
16 KiB
Markdown
578 lines
16 KiB
Markdown
# 步骤3：代码质量检查
|
||
|
||
## ⚠️ 执行前必读规范
|
||
|
||
**🔥 重要：在执行本步骤之前，AI必须先完整阅读同级目录下的 `README.md` 文件！**
|
||
|
||
该README文件包含：
|
||
- 🎯 执行前准备和用户信息收集要求
|
||
- 🔄 强制执行原则和分步执行流程
|
||
- 🔥 修改后立即重新执行当前步骤的强制规则
|
||
- 📝 文件修改记录规范和版本号递增规则
|
||
- 🧪 测试文件调试规范和测试指令使用规范
|
||
- 🚨 全局约束和游戏服务器特殊要求
|
||
|
||
**不阅读README直接执行步骤将导致执行不规范，违反项目要求！**
|
||
|
||
---
|
||
|
||
## 🎯 检查目标
|
||
清理和优化代码质量，消除未使用代码、规范常量定义、处理TODO项。
|
||
|
||
## 🧹 未使用代码清理
|
||
|
||
### 清理未使用的导入
|
||
```typescript
|
||
// ❌ 错误：导入未使用的模块
|
||
import { Injectable, NotFoundException, BadRequestException } from '@nestjs/common';
|
||
import { User, Admin } from './user.entity';
|
||
import * as crypto from 'crypto'; // 未使用
|
||
import { RedisService } from '../redis/redis.service'; // 未使用
|
||
|
||
// ✅ 正确：只导入使用的模块
|
||
import { Injectable, NotFoundException } from '@nestjs/common';
|
||
import { User } from './user.entity';
|
||
```
|
||
|
||
### 清理未使用的变量
|
||
```typescript
|
||
// ❌ 错误：定义但未使用的变量
|
||
const unusedVariable = 'test';
|
||
let tempData = [];
|
||
|
||
// ✅ 正确：删除未使用的变量
|
||
// 只保留实际使用的变量
|
||
```
|
||
|
||
### 清理未使用的方法
|
||
```typescript
|
||
// ❌ 错误：定义但未调用的私有方法
|
||
private generateVerificationCode(): string {
|
||
// 如果这个方法没有被调用，应该删除
|
||
}
|
||
|
||
// ✅ 正确：删除未使用的私有方法
|
||
// 或者确保方法被正确调用
|
||
```
|
||
|
||
## 📊 常量定义规范
|
||
|
||
### 使用SCREAMING_SNAKE_CASE
|
||
```typescript
|
||
// ✅ 正确：使用全大写+下划线
|
||
const SALT_ROUNDS = 10;
|
||
const MAX_LOGIN_ATTEMPTS = 5;
|
||
const DEFAULT_PAGE_SIZE = 20;
|
||
const WEBSOCKET_TIMEOUT = 30000;
|
||
const MAX_ROOM_CAPACITY = 100;
|
||
|
||
// ❌ 错误：使用小驼峰
|
||
const saltRounds = 10;
|
||
const maxLoginAttempts = 5;
|
||
const defaultPageSize = 20;
|
||
```
|
||
|
||
### 提取魔法数字为常量
|
||
```typescript
|
||
// ❌ 错误：使用魔法数字
|
||
if (attempts > 5) {
|
||
throw new Error('Too many attempts');
|
||
}
|
||
setTimeout(callback, 30000);
|
||
|
||
// ✅ 正确：提取为常量
|
||
const MAX_LOGIN_ATTEMPTS = 5;
|
||
const WEBSOCKET_TIMEOUT = 30000;
|
||
|
||
if (attempts > MAX_LOGIN_ATTEMPTS) {
|
||
throw new Error('Too many attempts');
|
||
}
|
||
setTimeout(callback, WEBSOCKET_TIMEOUT);
|
||
```
|
||
|
||
## 📏 方法长度检查
|
||
|
||
### 长度限制
|
||
- **建议**：方法不超过50行
|
||
- **原则**：一个方法只做一件事
|
||
- **拆分**：复杂方法拆分为多个小方法
|
||
|
||
### 方法拆分示例
|
||
```typescript
|
||
// ❌ 错误：方法过长（超过50行）
|
||
async processUserRegistration(userData: CreateUserDto): Promise<User> {
|
||
// 验证用户数据
|
||
// 检查邮箱是否存在
|
||
// 生成密码哈希
|
||
// 创建用户记录
|
||
// 发送欢迎邮件
|
||
// 记录操作日志
|
||
// 返回用户信息
|
||
// ... 超过50行的复杂逻辑
|
||
}
|
||
|
||
// ✅ 正确：拆分为多个小方法
|
||
async processUserRegistration(userData: CreateUserDto): Promise<User> {
|
||
await this.validateUserData(userData);
|
||
await this.checkEmailExists(userData.email);
|
||
const hashedPassword = await this.generatePasswordHash(userData.password);
|
||
const user = await this.createUserRecord({ ...userData, password: hashedPassword });
|
||
await this.sendWelcomeEmail(user.email);
|
||
await this.logUserRegistration(user.id);
|
||
return user;
|
||
}
|
||
|
||
private async validateUserData(userData: CreateUserDto): Promise<void> {
|
||
// 验证逻辑
|
||
}
|
||
|
||
private async checkEmailExists(email: string): Promise<void> {
|
||
// 邮箱检查逻辑
|
||
}
|
||
```
|
||
|
||
## 🔄 代码重复消除
|
||
|
||
### 识别重复代码
|
||
```typescript
|
||
// ❌ 错误：重复的验证逻辑
|
||
async createUser(userData: CreateUserDto): Promise<User> {
|
||
if (!userData.email || !userData.name) {
|
||
throw new BadRequestException('Required fields missing');
|
||
}
|
||
if (!this.isValidEmail(userData.email)) {
|
||
throw new BadRequestException('Invalid email format');
|
||
}
|
||
// 创建用户逻辑
|
||
}
|
||
|
||
async updateUser(id: string, userData: UpdateUserDto): Promise<User> {
|
||
if (!userData.email || !userData.name) {
|
||
throw new BadRequestException('Required fields missing');
|
||
}
|
||
if (!this.isValidEmail(userData.email)) {
|
||
throw new BadRequestException('Invalid email format');
|
||
}
|
||
// 更新用户逻辑
|
||
}
|
||
|
||
// ✅ 正确：抽象为可复用方法
|
||
async createUser(userData: CreateUserDto): Promise<User> {
|
||
this.validateUserData(userData);
|
||
// 创建用户逻辑
|
||
}
|
||
|
||
async updateUser(id: string, userData: UpdateUserDto): Promise<User> {
|
||
this.validateUserData(userData);
|
||
// 更新用户逻辑
|
||
}
|
||
|
||
private validateUserData(userData: CreateUserDto | UpdateUserDto): void {
|
||
if (!userData.email || !userData.name) {
|
||
throw new BadRequestException('Required fields missing');
|
||
}
|
||
if (!this.isValidEmail(userData.email)) {
|
||
throw new BadRequestException('Invalid email format');
|
||
}
|
||
}
|
||
```
|
||
|
||
## 🚨 异常处理完整性检查（关键规范）
|
||
|
||
### 问题定义
|
||
**异常吞没（Exception Swallowing）** 是指在 catch 块中捕获异常后，只记录日志但不重新抛出，导致：
|
||
- 调用方无法感知错误
|
||
- 方法返回 undefined 而非声明的类型
|
||
- 数据不一致或静默失败
|
||
- 难以调试和定位问题
|
||
|
||
### 检查规则
|
||
|
||
#### 规则1：catch 块必须有明确的异常处理策略
|
||
```typescript
|
||
// ❌ 严重错误：catch 块吞没异常
|
||
async create(createDto: CreateDto): Promise<ResponseDto> {
|
||
try {
|
||
const result = await this.repository.create(createDto);
|
||
return this.toResponseDto(result);
|
||
} catch (error) {
|
||
this.logger.error('创建失败', error);
|
||
// 错误：没有 throw，方法返回 undefined
|
||
// 但声明返回 Promise<ResponseDto>
|
||
}
|
||
}
|
||
|
||
// ❌ 错误：只记录日志不处理
|
||
async findById(id: string): Promise<Entity> {
|
||
try {
|
||
return await this.repository.findById(id);
|
||
} catch (error) {
|
||
monitor.error(error);
|
||
// 错误：异常被吞没，调用方无法感知
|
||
}
|
||
}
|
||
|
||
// ✅ 正确：重新抛出异常
|
||
async create(createDto: CreateDto): Promise<ResponseDto> {
|
||
try {
|
||
const result = await this.repository.create(createDto);
|
||
return this.toResponseDto(result);
|
||
} catch (error) {
|
||
this.logger.error('创建失败', error);
|
||
throw error; // 必须重新抛出
|
||
}
|
||
}
|
||
|
||
// ✅ 正确：转换为特定异常类型
|
||
async create(createDto: CreateDto): Promise<ResponseDto> {
|
||
try {
|
||
const result = await this.repository.create(createDto);
|
||
return this.toResponseDto(result);
|
||
} catch (error) {
|
||
this.logger.error('创建失败', error);
|
||
if (error.message.includes('duplicate')) {
|
||
throw new ConflictException('记录已存在');
|
||
}
|
||
throw error; // 其他错误继续抛出
|
||
}
|
||
}
|
||
|
||
// ✅ 正确：返回错误响应（仅限顶层API）
|
||
async create(createDto: CreateDto): Promise<ApiResponse<ResponseDto>> {
|
||
try {
|
||
const result = await this.repository.create(createDto);
|
||
return { success: true, data: this.toResponseDto(result) };
|
||
} catch (error) {
|
||
this.logger.error('创建失败', error);
|
||
return {
|
||
success: false,
|
||
error: error.message,
|
||
errorCode: 'CREATE_FAILED'
|
||
}; // 顶层API可以返回错误响应
|
||
}
|
||
}
|
||
```
|
||
|
||
#### 规则2：Service 层方法必须传播异常
|
||
```typescript
|
||
// ❌ 错误：Service 层吞没异常
|
||
@Injectable()
|
||
export class UserService {
|
||
async update(id: string, dto: UpdateDto): Promise<ResponseDto> {
|
||
try {
|
||
const result = await this.repository.update(id, dto);
|
||
return this.toResponseDto(result);
|
||
} catch (error) {
|
||
this.logger.error('更新失败', { id, error });
|
||
// 错误：Service 层不应吞没异常
|
||
}
|
||
}
|
||
}
|
||
|
||
// ✅ 正确：Service 层传播异常
|
||
@Injectable()
|
||
export class UserService {
|
||
async update(id: string, dto: UpdateDto): Promise<ResponseDto> {
|
||
try {
|
||
const result = await this.repository.update(id, dto);
|
||
return this.toResponseDto(result);
|
||
} catch (error) {
|
||
this.logger.error('更新失败', { id, error });
|
||
throw error; // 传播给调用方处理
|
||
}
|
||
}
|
||
}
|
||
```
|
||
|
||
#### 规则3：Repository 层必须传播数据库异常
|
||
```typescript
|
||
// ❌ 错误：Repository 层吞没数据库异常
|
||
@Injectable()
|
||
export class UserRepository {
|
||
async findById(id: bigint): Promise<User | null> {
|
||
try {
|
||
return await this.repository.findOne({ where: { id } });
|
||
} catch (error) {
|
||
this.logger.error('查询失败', { id, error });
|
||
// 错误：数据库异常被吞没，调用方以为查询成功但返回 null
|
||
}
|
||
}
|
||
}
|
||
|
||
// ✅ 正确：Repository 层传播异常
|
||
@Injectable()
|
||
export class UserRepository {
|
||
async findById(id: bigint): Promise<User | null> {
|
||
try {
|
||
return await this.repository.findOne({ where: { id } });
|
||
} catch (error) {
|
||
this.logger.error('查询失败', { id, error });
|
||
throw error; // 数据库异常必须传播
|
||
}
|
||
}
|
||
}
|
||
```
|
||
|
||
### 异常处理层级规范
|
||
|
||
| 层级 | 异常处理策略 | 说明 |
|
||
|------|-------------|------|
|
||
| **Repository 层** | 必须 throw | 数据访问异常必须传播 |
|
||
| **Service 层** | 必须 throw | 业务异常必须传播给调用方 |
|
||
| **Business 层** | 必须 throw | 业务逻辑异常必须传播 |
|
||
| **Gateway/Controller 层** | 可以转换为 HTTP 响应 | 顶层可以将异常转换为错误响应 |
|
||
|
||
### 检查清单
|
||
|
||
- [ ] **所有 catch 块是否有 throw 语句？**
|
||
- [ ] **方法返回类型与实际返回是否一致？**（避免返回 undefined）
|
||
- [ ] **Service/Repository 层是否传播异常？**
|
||
- [ ] **只有顶层 API 才能将异常转换为错误响应？**
|
||
- [ ] **异常日志是否包含足够的上下文信息？**
|
||
|
||
### 快速检查命令
|
||
```bash
|
||
# 搜索可能吞没异常的 catch 块（没有 throw 的 catch）
|
||
# 在代码审查时重点关注这些位置
|
||
grep -rn "catch.*error" --include="*.ts" | grep -v "throw"
|
||
```
|
||
|
||
### 常见错误模式
|
||
|
||
#### 模式1：性能监控后忘记抛出
|
||
```typescript
|
||
// ❌ 常见错误
|
||
} catch (error) {
|
||
monitor.error(error); // 只记录监控
|
||
// 忘记 throw error;
|
||
}
|
||
|
||
// ✅ 正确
|
||
} catch (error) {
|
||
monitor.error(error);
|
||
throw error; // 必须抛出
|
||
}
|
||
```
|
||
|
||
#### 模式2：条件分支遗漏 throw
|
||
```typescript
|
||
// ❌ 常见错误
|
||
} catch (error) {
|
||
if (error.code === 'DUPLICATE') {
|
||
throw new ConflictException('已存在');
|
||
}
|
||
// else 分支忘记 throw
|
||
this.logger.error(error);
|
||
}
|
||
|
||
// ✅ 正确
|
||
} catch (error) {
|
||
if (error.code === 'DUPLICATE') {
|
||
throw new ConflictException('已存在');
|
||
}
|
||
this.logger.error(error);
|
||
throw error; // else 分支也要抛出
|
||
}
|
||
```
|
||
|
||
#### 模式3：返回类型不匹配
|
||
```typescript
|
||
// ❌ 错误：声明返回 Promise<Entity> 但可能返回 undefined
|
||
async findById(id: string): Promise<Entity> {
|
||
try {
|
||
return await this.repo.findById(id);
|
||
} catch (error) {
|
||
this.logger.error(error);
|
||
// 没有 throw，TypeScript 不会报错但运行时返回 undefined
|
||
}
|
||
}
|
||
|
||
// ✅ 正确
|
||
async findById(id: string): Promise<Entity> {
|
||
try {
|
||
return await this.repo.findById(id);
|
||
} catch (error) {
|
||
this.logger.error(error);
|
||
throw error;
|
||
}
|
||
}
|
||
```
|
||
|
||
## 🚫 TODO项处理（强制要求）
|
||
|
||
### 处理原则
|
||
**最终文件不能包含TODO项**，必须：
|
||
1. **真正实现功能**
|
||
2. **删除未完成代码**
|
||
|
||
### 常见TODO处理
|
||
```typescript
|
||
// ❌ 错误：包含TODO项的代码
|
||
async getUserProfile(id: string): Promise<UserProfile> {
|
||
// TODO: 实现用户档案查询
|
||
throw new Error('Not implemented');
|
||
}
|
||
|
||
async sendSmsVerification(phone: string): Promise<void> {
|
||
// TODO: 集成短信服务提供商
|
||
throw new Error('SMS service not implemented');
|
||
}
|
||
|
||
// ✅ 正确：真正实现功能
|
||
async getUserProfile(id: string): Promise<UserProfile> {
|
||
const profile = await this.userProfileRepository.findOne({
|
||
where: { userId: id }
|
||
});
|
||
|
||
if (!profile) {
|
||
throw new NotFoundException('用户档案不存在');
|
||
}
|
||
|
||
return profile;
|
||
}
|
||
|
||
// ✅ 正确：如果功能不需要，删除方法
|
||
// 删除sendSmsVerification方法及其调用
|
||
```
|
||
|
||
## 🎮 游戏服务器特殊质量要求
|
||
|
||
### WebSocket连接管理
|
||
```typescript
|
||
// ✅ 正确：完整的连接管理
|
||
const MAX_CONNECTIONS_PER_ROOM = 100;
|
||
const CONNECTION_TIMEOUT = 30000;
|
||
const HEARTBEAT_INTERVAL = 10000;
|
||
|
||
@WebSocketGateway()
|
||
export class LocationBroadcastGateway {
|
||
private readonly connections = new Map<string, Socket>();
|
||
|
||
handleConnection(client: Socket): void {
|
||
this.validateConnection(client);
|
||
this.setupHeartbeat(client);
|
||
this.trackConnection(client);
|
||
}
|
||
|
||
private validateConnection(client: Socket): void {
|
||
// 连接验证逻辑
|
||
}
|
||
|
||
private setupHeartbeat(client: Socket): void {
|
||
// 心跳检测逻辑
|
||
}
|
||
}
|
||
```
|
||
|
||
### 双模式服务质量
|
||
```typescript
|
||
// ✅ 正确：确保两种模式行为一致
|
||
const DEFAULT_USER_STATUS = UserStatus.PENDING;
|
||
const MAX_BATCH_SIZE = 1000;
|
||
|
||
@Injectable()
|
||
export class UsersMemoryService {
|
||
private readonly users = new Map<string, User>();
|
||
|
||
async create(userData: CreateUserDto): Promise<User> {
|
||
this.validateUserData(userData);
|
||
const user = this.buildUserEntity(userData);
|
||
this.users.set(user.id, user);
|
||
return user;
|
||
}
|
||
|
||
private validateUserData(userData: CreateUserDto): void {
|
||
// 与数据库模式相同的验证逻辑
|
||
}
|
||
|
||
private buildUserEntity(userData: CreateUserDto): User {
|
||
// 与数据库模式相同的实体构建逻辑
|
||
}
|
||
}
|
||
```
|
||
|
||
### 属性测试质量
|
||
```typescript
|
||
// ✅ 正确：完整的属性测试实现
|
||
import * as fc from 'fast-check';
|
||
|
||
const PROPERTY_TEST_RUNS = 1000;
|
||
const MAX_USER_ID = 1000000;
|
||
|
||
describe('AdminService Properties', () => {
|
||
it('should handle any valid user status update', () => {
|
||
fc.assert(fc.property(
|
||
fc.integer({ min: 1, max: MAX_USER_ID }),
|
||
fc.constantFrom(...Object.values(UserStatus)),
|
||
async (userId, status) => {
|
||
try {
|
||
const result = await adminService.updateUserStatus(userId, status);
|
||
expect(result).toBeDefined();
|
||
expect(result.status).toBe(status);
|
||
} catch (error) {
|
||
expect(error).toBeInstanceOf(NotFoundException || BadRequestException);
|
||
}
|
||
}
|
||
), { numRuns: PROPERTY_TEST_RUNS });
|
||
});
|
||
});
|
||
```
|
||
|
||
## 🔍 检查执行步骤
|
||
|
||
1. **扫描未使用的导入**
|
||
- 检查每个import语句是否被使用
|
||
- 删除未使用的导入
|
||
|
||
2. **扫描未使用的变量和方法**
|
||
- 检查变量是否被引用
|
||
- 检查私有方法是否被调用
|
||
- 删除未使用的代码
|
||
|
||
3. **检查常量定义**
|
||
- 识别魔法数字和字符串
|
||
- 提取为SCREAMING_SNAKE_CASE常量
|
||
- 确保常量命名清晰
|
||
|
||
4. **检查方法长度**
|
||
- 统计每个方法的行数
|
||
- 识别超过50行的方法
|
||
- 建议拆分复杂方法
|
||
|
||
5. **识别重复代码**
|
||
- 查找相似的代码块
|
||
- 抽象为可复用的工具方法
|
||
- 消除代码重复
|
||
|
||
6. **🚨 检查异常处理完整性（关键步骤）**
|
||
- 扫描所有 catch 块
|
||
- 检查是否有 throw 语句
|
||
- 验证 Service/Repository 层是否传播异常
|
||
- 确认方法返回类型与实际返回一致
|
||
- 识别异常吞没模式并修复
|
||
|
||
7. **处理所有TODO项**
|
||
- 搜索所有TODO注释
|
||
- 要求真正实现功能或删除代码
|
||
- 确保最终文件无TODO项
|
||
|
||
8. **游戏服务器特殊检查**
|
||
- WebSocket连接管理完整性
|
||
- 双模式服务行为一致性
|
||
- 属性测试实现质量
|
||
|
||
## 🔥 重要提醒
|
||
|
||
**如果在本步骤中执行了任何修改操作（删除未使用代码、提取常量、实现TODO项等），必须立即重新执行步骤3的完整检查！**
|
||
|
||
- ✅ 执行修改 → 🔥 立即重新执行步骤3 → 提供验证报告 → 等待用户确认
|
||
- ❌ 执行修改 → 直接进入步骤4（错误做法）
|
||
|
||
**🚨 重要强调：纯检查步骤不更新修改记录**
|
||
**如果检查发现代码质量已经符合规范，无需任何修改，则：**
|
||
- ❌ **禁止添加检查记录**：不要添加"AI代码检查步骤3：代码质量检查和优化"
|
||
- ❌ **禁止更新时间戳**：不要修改@lastModified字段
|
||
- ❌ **禁止递增版本号**：不要修改@version字段
|
||
- ✅ **仅提供检查报告**：说明检查结果，确认符合规范
|
||
|
||
**不能跳过重新检查环节！** |