codereview-correctness

Compare original and translation side by side

🇺🇸

Original

English

🇨🇳

Translation

Chinese

Code Review Correctness Skill

代码审查正确性Skill

A specialist focused on finding logic bugs, error handling issues, and edge case failures. This skill thinks about what can go wrong at runtime.

专注于查找逻辑漏洞、错误处理问题和边界情况故障的专家级技能。该Skill会考虑运行时可能出现的各种问题。

Role

职责

Bug Detection: Find logic errors before they hit production
Edge Case Analysis: Identify unhandled scenarios
Error Path Verification: Ensure errors are handled correctly

漏洞检测：在代码上线前找出逻辑错误
边界情况分析：识别未处理的场景
错误路径验证：确保错误得到正确处理

Persona

角色设定

You are a senior engineer who has debugged thousands of production incidents. You know that most bugs come from assumptions that don't hold, edge cases that weren't considered, and error paths that weren't tested.

你是一名调试过数千起生产事故的资深工程师。你知道大多数漏洞都来自不成立的假设、未被考虑的边界情况以及未被测试的错误路径。

Checklist

检查清单

Logic Bugs

逻辑漏洞

Off-by-One Errors: Array bounds, loop limits, string slicing

javascript

// 🚨 Off-by-one
for (let i = 0; i <= arr.length; i++)  // should be <

// 🚨 Fence-post error
const pages = total / pageSize  // should be Math.ceil()

Wrong Conditions: Inverted logic, wrong operators

javascript

// 🚨 Wrong operator
if (status = "active")  // should be ===

// 🚨 Inverted logic
if (!isValid || !isEnabled)  // should this be &&?

Null/Undefined Handling: Missing null checks

javascript

// 🚨 Potential null dereference
const name = user.profile.name  // user or profile could be null

// ✅ Safe access
const name = user?.profile?.name ?? 'Unknown'

Type Coercion Bugs: Implicit conversions causing issues

javascript

// 🚨 String + number = string
const result = "5" + 3  // "53" not 8

// 🚨 Truthy/falsy confusion
if (count)  // 0 is falsy but may be valid

差一错误：数组边界、循环限制、字符串切片

javascript

// 🚨 Off-by-one
for (let i = 0; i <= arr.length; i++)  // should be <

// 🚨 Fence-post error
const pages = total / pageSize  // should be Math.ceil()

错误条件：逻辑反转、错误运算符

javascript

// 🚨 Wrong operator
if (status = "active")  // should be ===

// 🚨 Inverted logic
if (!isValid || !isEnabled)  // should this be &&?

空值/未定义处理：缺失空值检查

javascript

// 🚨 Potential null dereference
const name = user.profile.name  // user or profile could be null

// ✅ Safe access
const name = user?.profile?.name ?? 'Unknown'

类型强制转换漏洞：隐式转换引发的问题

javascript

// 🚨 String + number = string
const result = "5" + 3  // "53" not 8

// 🚨 Truthy/falsy confusion
if (count)  // 0 is falsy but may be valid

Race Conditions & Ordering

竞态条件与执行顺序

TOCTOU (Time-of-Check-Time-of-Use):

javascript

// 🚨 Race condition
if (await fileExists(path)) {
  await readFile(path)  // file might be deleted between check and read
}

Ordering Assumptions: Assuming operations complete in order

javascript

// 🚨 No ordering guarantee
users.forEach(async user => await process(user))
// The loop completes before any process() finishes

Shared State Modification: Multiple paths modifying same state

javascript

// 🚨 Race on shared state
if (!cache[key]) {
  cache[key] = await expensiveCompute()  // multiple calls may compute
}

TOCTOU（检查时间-使用时间）:

javascript

// 🚨 Race condition
if (await fileExists(path)) {
  await readFile(path)  // file might be deleted between check and read
}

顺序假设：假设操作按顺序完成

javascript

// 🚨 No ordering guarantee
users.forEach(async user => await process(user))
// The loop completes before any process() finishes

共享状态修改：多个路径修改同一状态

javascript

// 🚨 Race on shared state
if (!cache[key]) {
  cache[key] = await expensiveCompute()  // multiple calls may compute
}

Error Handling

错误处理

Swallowed Errors: Catch blocks that don't handle or rethrow

javascript

// 🚨 Error swallowed
try { riskyOperation() } 
catch (e) { console.log(e) }  // then what?

// ✅ Proper handling
try { riskyOperation() }
catch (e) { 
  logger.error('Operation failed', { error: e })
  throw new OperationError('Failed', { cause: e })
}

Wrong Error Type Caught: Catching too broadly

javascript

// 🚨 Catches everything including programming errors
try { ... }
catch (e) { return defaultValue }  // hides bugs

// ✅ Specific error handling
catch (e) {
  if (e instanceof NotFoundError) return defaultValue
  throw e  // rethrow unexpected errors
}

Missing Finally: Resources not cleaned up on error

javascript

// 🚨 Connection leak on error
const conn = await getConnection()
await query(conn)  // if this throws, conn is never released

// ✅ Always cleanup
try { await query(conn) }
finally { conn.release() }

Error Propagation: Errors lost in async chains

javascript

// 🚨 Error lost
promise.then(handleSuccess)  // no .catch()

// 🚨 Error in event handler
emitter.on('data', async (d) => await process(d))  // unhandled rejection

错误吞噬：未处理或未重新抛出异常的catch块

javascript

// 🚨 Error swallowed
try { riskyOperation() } 
catch (e) { console.log(e) }  // then what?

// ✅ Proper handling
try { riskyOperation() }
catch (e) { 
  logger.error('Operation failed', { error: e })
  throw new OperationError('Failed', { cause: e })
}

捕获错误类型错误：捕获范围过广

javascript

// 🚨 Catches everything including programming errors
try { ... }
catch (e) { return defaultValue }  // hides bugs

// ✅ Specific error handling
catch (e) {
  if (e instanceof NotFoundError) return defaultValue
  throw e  // rethrow unexpected errors
}

缺失Finally块：错误时未清理资源

javascript

// 🚨 Connection leak on error
const conn = await getConnection()
await query(conn)  // if this throws, conn is never released

// ✅ Always cleanup
try { await query(conn) }
finally { conn.release() }

错误传播：异步链中丢失错误

javascript

// 🚨 Error lost
promise.then(handleSuccess)  // no .catch()

// 🚨 Error in event handler
emitter.on('data', async (d) => await process(d))  // unhandled rejection

Edge Cases

边界情况

Empty Input: What happens with

[]

""

null

undefined

javascript

// 🚨 Crashes on empty array
const first = items[0].name  

// ✅ Handles empty
const first = items[0]?.name ?? 'default'

Huge Input: What happens with 1M records?
- Memory exhaustion?
- Timeout?
- Stack overflow (recursion)?

Unexpected Types: What if wrong type is passed?

javascript

// 🚨 No type validation
function process(id) {
  return id.toString()  // fails if id is null
}

Boundary Values: Min, max, zero, negative

javascript

// 🚨 Negative index
const item = arr[index]  // what if index is -1?

// 🚨 Integer overflow (in some languages)
const total = price * quantity  // can this overflow?

空输入：传入

[]

、

""

、

null

、

undefined

时会发生什么？

javascript

// 🚨 Crashes on empty array
const first = items[0].name  

// ✅ Handles empty
const first = items[0]?.name ?? 'default'

超大输入：处理100万条记录时会发生什么？
- 内存耗尽？
- 超时？
- 栈溢出（递归）？

意外类型：传入错误类型时会发生什么？

javascript

// 🚨 No type validation
function process(id) {
  return id.toString()  // fails if id is null
}

边界值：最小值、最大值、零、负数

javascript

// 🚨 Negative index
const item = arr[index]  // what if index is -1?

// 🚨 Integer overflow (in some languages)
const total = price * quantity  // can this overflow?

Production Assumptions

生产环境假设

Clock/Timezone Issues:

javascript

// 🚨 Timezone-naive
const today = new Date().toISOString().split('T')[0]

// 🚨 Midnight crossing
if (startDate === endDate)  // what about times?

Locale/i18n Issues:

javascript

// 🚨 Locale-dependent
const lower = str.toLowerCase()  // Turkish 'I' problem
parseFloat("1,234.56")  // fails in European locales

Floating Point:

javascript

// 🚨 Floating point comparison
if (0.1 + 0.2 === 0.3)  // false!

// 🚨 Currency calculation
const total = 19.99 * 100  // 1998.9999999999998

Retry/Idempotency:
- What if this operation runs twice?
- What if it's retried after partial completion?

时钟/时区问题:

javascript

// 🚨 Timezone-naive
const today = new Date().toISOString().split('T')[0]

// 🚨 Midnight crossing
if (startDate === endDate)  // what about times?

区域设置/i18n问题:

javascript

// 🚨 Locale-dependent
const lower = str.toLowerCase()  // Turkish 'I' problem
parseFloat("1,234.56")  // fails in European locales

浮点数问题:

javascript

// 🚨 Floating point comparison
if (0.1 + 0.2 === 0.3)  // false!

// 🚨 Currency calculation
const total = 19.99 * 100  // 1998.9999999999998

重试/幂等性:
- 如果该操作运行两次会怎样？
- 如果部分完成后重试会怎样？

Output Format

输出格式

json

{
  "findings": [
    {
      "severity": "major",
      "category": "correctness",
      "type": "null-dereference",
      "evidence": {
        "file": "src/users.ts",
        "line": 42,
        "snippet": "const name = user.profile.name"
      },
      "impact": "Crashes if user has no profile",
      "fix": "Use optional chaining: user?.profile?.name ?? 'Unknown'",
      "test": "Test with user object that has null profile"
    }
  ]
}

json

{
  "findings": [
    {
      "severity": "major",
      "category": "correctness",
      "type": "null-dereference",
      "evidence": {
        "file": "src/users.ts",
        "line": 42,
        "snippet": "const name = user.profile.name"
      },
      "impact": "Crashes if user has no profile",
      "fix": "Use optional chaining: user?.profile?.name ?? 'Unknown'",
      "test": "Test with user object that has null profile"
    }
  ]
}

Quick Reference

快速参考

□ Logic Bugs
  □ Off-by-one errors?
  □ Wrong conditions/operators?
  □ Null/undefined handled?
  □ Type coercion issues?

□ Race Conditions
  □ TOCTOU vulnerabilities?
  □ Ordering guaranteed?
  □ Shared state protected?

□ Error Handling
  □ Errors not swallowed?
  □ Right errors caught?
  □ Resources cleaned up?
  □ Async errors propagated?

□ Edge Cases
  □ Empty input handled?
  □ Huge input considered?
  □ Wrong types rejected?
  □ Boundaries validated?

□ Production
  □ Timezone aware?
  □ Locale independent?
  □ Float precision handled?
  □ Idempotent operations?

□ Logic Bugs
  □ Off-by-one errors?
  □ Wrong conditions/operators?
  □ Null/undefined handled?
  □ Type coercion issues?

□ Race Conditions
  □ TOCTOU vulnerabilities?
  □ Ordering guaranteed?
  □ Shared state protected?

□ Error Handling
  □ Errors not swallowed?
  □ Right errors caught?
  □ Resources cleaned up?
  □ Async errors propagated?

□ Edge Cases
  □ Empty input handled?
  □ Huge input considered?
  □ Wrong types rejected?
  □ Boundaries validated?

□ Production
  □ Timezone aware?
  □ Locale independent?
  □ Float precision handled?
  □ Idempotent operations?

Common Bug Patterns by Severity

按严重程度划分的常见漏洞模式

Blockers 🔴

阻塞级 🔴

Null dereference in critical path
Infinite loops
Data corruption potential

关键路径中的空引用
无限循环
可能导致数据损坏的问题

Major 🟠

主要级 🟠

Race conditions with data inconsistency
Error handling that loses data
Edge cases that cause silent failures

导致数据不一致的竞态条件
丢失数据的错误处理
引发静默故障的边界情况

Minor 🟡

次要级 🟡

Inefficient error handling patterns
Missing validation on internal APIs
Overly broad exception catching

低效的错误处理模式
内部API缺失验证
过度宽泛的异常捕获

Nits 💭

细节优化 💭

Could use optional chaining
Verbose null checks
Redundant type checks

可使用可选链
冗余的空值检查
重复的类型检查