Back to Details

code-review

Compare original and translation side by side

🇺🇸

Original

English
🇨🇳

Translation

Chinese

ServiceNow Code Review Checklist

ServiceNow 代码审查检查清单

Use this checklist when reviewing ServiceNow server-side code (Business Rules, Script Includes, Scheduled Jobs, etc.).
当审查ServiceNow服务器端代码（业务规则、脚本包含、计划任务等）时，请使用本检查清单。

1. ES5 Compliance (CRITICAL)

1. ES5 合规性（CRITICAL）

javascript
// CHECK FOR THESE ES6+ VIOLATIONS:
const x = 5;           // ❌ Use var
let items = [];        // ❌ Use var
() => {}               // ❌ Use function()
`template ${var}`      // ❌ Use 'string ' + var
for (x of arr)         // ❌ Use traditional for loop
{a, b} = obj           // ❌ Use obj.a, obj.b
Action: Flag ALL ES6+ syntax as CRITICAL errors.
javascript
// 检查以下ES6+违规情况:
const x = 5;           // ❌ 请使用 var
let items = [];        // ❌ 请使用 var
() => {}               // ❌ 请使用 function()
`template ${var}`      // ❌ 请使用 'string ' + var
for (x of arr)         // ❌ 请使用传统for循环
{a, b} = obj           // ❌ 请使用 obj.a, obj.b
操作： 将所有ES6+语法标记为CRITICAL错误。

2. Security Issues

2. 安全问题

2.1 SQL/GlideRecord Injection

2.1 SQL/GlideRecord 注入

javascript
// ❌ DANGEROUS - User input directly in query
gr.addEncodedQuery(userInput);
gr.addQuery('field', userInput);  // OK if validated

// ✅ SAFE - Validate and sanitize
var safeInput = new GlideSysAttachment().cleanFileName(userInput);
gr.addQuery('field', safeInput);
javascript
// ❌ 危险 - 用户输入直接用于查询
gr.addEncodedQuery(userInput);
gr.addQuery('field', userInput);  // 若经过验证则没问题

// ✅ 安全 - 验证并清理输入
var safeInput = new GlideSysAttachment().cleanFileName(userInput);
gr.addQuery('field', safeInput);

2.2 Cross-Site Scripting (XSS)

2.2 跨站脚本攻击（XSS）

javascript
// ❌ DANGEROUS - Unescaped output
gs.addInfoMessage(userInput);

// ✅ SAFE - Escape HTML
gs.addInfoMessage(GlideStringUtil.escapeHTML(userInput));
javascript
// ❌ 危险 - 未转义的输出
gs.addInfoMessage(userInput);

// ✅ 安全 - 转义HTML
gs.addInfoMessage(GlideStringUtil.escapeHTML(userInput));

2.3 Access Control

2.3 访问控制

javascript
// ❌ MISSING - No ACL check
var gr = new GlideRecord('sys_user');
gr.get(userProvidedSysId);

// ✅ SAFE - Check permissions
var gr = new GlideRecord('sys_user');
if (gr.get(userProvidedSysId) && gr.canRead()) {
  // Process record
}
javascript
// ❌ 缺失 - 无ACL检查
var gr = new GlideRecord('sys_user');
gr.get(userProvidedSysId);

// ✅ 安全 - 检查权限
var gr = new GlideRecord('sys_user');
if (gr.get(userProvidedSysId) && gr.canRead()) {
  // 处理记录
}

2.4 Sensitive Data Exposure

2.4 敏感数据泄露

javascript
// ❌ DANGEROUS - Logging sensitive data
gs.info('Password: ' + password);
gs.info('API Key: ' + apiKey);

// ✅ SAFE - Never log credentials
gs.info('Authentication attempt for user: ' + username);
javascript
// ❌ 危险 - 记录敏感数据
gs.info('Password: ' + password);
gs.info('API Key: ' + apiKey);

// ✅ 安全 - 切勿记录凭证
gs.info('针对用户的认证尝试: ' + username);

3. Performance Issues

3. 性能问题

3.1 Queries in Loops

3.1 循环内查询

javascript
// ❌ SLOW - N+1 query problem
for (var i = 0; i < userIds.length; i++) {
  var gr = new GlideRecord('sys_user');
  gr.get(userIds[i]);  // Query for each user!
}

// ✅ FAST - Single query
var gr = new GlideRecord('sys_user');
gr.addQuery('sys_id', 'IN', userIds.join(','));
gr.query();
while (gr.next()) { }
javascript
// ❌ 缓慢 - N+1查询问题
for (var i = 0; i < userIds.length; i++) {
  var gr = new GlideRecord('sys_user');
  gr.get(userIds[i]);  // 为每个用户执行查询!
}

// ✅ 快速 - 单次查询
var gr = new GlideRecord('sys_user');
gr.addQuery('sys_id', 'IN', userIds.join(','));
gr.query();
while (gr.next()) { }

3.2 Missing setLimit()

3.2 缺失 setLimit()

javascript
// ❌ SLOW - Could return millions of records
var gr = new GlideRecord('incident');
gr.query();

// ✅ FAST - Limit results
var gr = new GlideRecord('incident');
gr.setLimit(1000);
gr.query();
javascript
// ❌ 缓慢 - 可能返回数百万条记录
var gr = new GlideRecord('incident');
gr.query();

// ✅ 快速 - 限制结果数量
var gr = new GlideRecord('incident');
gr.setLimit(1000);
gr.query();

3.3 Unnecessary Queries

3.3 不必要的查询

javascript
// ❌ WASTEFUL - Query just to check existence
var gr = new GlideRecord('incident');
gr.addQuery('number', incNumber);
gr.query();
if (gr.getRowCount() > 0) { }

// ✅ EFFICIENT - Use get() for single record
var gr = new GlideRecord('incident');
if (gr.get('number', incNumber)) { }
javascript
// ❌ 浪费资源 - 仅为检查存在性而查询
var gr = new GlideRecord('incident');
gr.addQuery('number', incNumber);
gr.query();
if (gr.getRowCount() > 0) { }

// ✅ 高效 - 对单条记录使用get()
var gr = new GlideRecord('incident');
if (gr.get('number', incNumber)) { }

3.4 GlideRecord vs GlideAggregate

3.4 GlideRecord vs GlideAggregate

javascript
// ❌ SLOW - Counting with loop
var count = 0;
var gr = new GlideRecord('incident');
gr.addQuery('active', true);
gr.query();
while (gr.next()) count++;

// ✅ FAST - Use GlideAggregate
var ga = new GlideAggregate('incident');
ga.addQuery('active', true);
ga.addAggregate('COUNT');
ga.query();
var count = ga.next() ? ga.getAggregate('COUNT') : 0;
javascript
// ❌ 缓慢 - 通过循环计数
var count = 0;
var gr = new GlideRecord('incident');
gr.addQuery('active', true);
gr.query();
while (gr.next()) count++;

// ✅ 快速 - 使用GlideAggregate
var ga = new GlideAggregate('incident');
ga.addQuery('active', true);
ga.addAggregate('COUNT');
ga.query();
var count = ga.next() ? ga.getAggregate('COUNT') : 0;

4. Code Quality Issues

4. 代码质量问题

4.1 Hard-coded sys_ids

4.1 硬编码sys_ids

javascript
// ❌ BAD - Hard-coded sys_id (breaks across instances)
var assignmentGroup = '681ccaf9c0a8016400b98a06818d57c7';

// ✅ GOOD - Use property or lookup
var assignmentGroup = gs.getProperty('my.default.assignment.group');
// OR
var gr = new GlideRecord('sys_user_group');
if (gr.get('name', 'Service Desk')) {
  var assignmentGroup = gr.getUniqueValue();
}
javascript
// ❌ 不良 - 硬编码sys_id（在不同实例中会失效）
var assignmentGroup = '681ccaf9c0a8016400b98a06818d57c7';

// ✅ 良好 - 使用属性或查找
var assignmentGroup = gs.getProperty('my.default.assignment.group');
// 或
var gr = new GlideRecord('sys_user_group');
if (gr.get('name', 'Service Desk')) {
  var assignmentGroup = gr.getUniqueValue();
}

4.2 Magic Numbers/Strings

4.2 魔法数字/字符串

javascript
// ❌ BAD - Magic numbers
if (current.state == 6) { }
if (current.priority == 1) { }

// ✅ GOOD - Named constants or comments
var STATE_RESOLVED = 6;
var PRIORITY_CRITICAL = 1;
if (current.state == STATE_RESOLVED) { }
javascript
// ❌ 不良 - 魔法数字
if (current.state == 6) { }
if (current.priority == 1) { }

// ✅ 良好 - 使用命名常量或注释
var STATE_RESOLVED = 6;
var PRIORITY_CRITICAL = 1;
if (current.state == STATE_RESOLVED) { }

4.3 Missing Error Handling

4.3 缺失错误处理

javascript
// ❌ BAD - No error handling
var response = request.execute();
var data = JSON.parse(response.getBody());

// ✅ GOOD - Proper error handling
try {
  var response = request.execute();
  var status = response.getStatusCode();
  if (status != 200) {
    gs.error('API call failed: ' + status);
    return null;
  }
  var data = JSON.parse(response.getBody());
} catch (e) {
  gs.error('Exception: ' + e.message);
  return null;
}
javascript
// ❌ 不良 - 无错误处理
var response = request.execute();
var data = JSON.parse(response.getBody());

// ✅ 良好 - 适当的错误处理
try {
  var response = request.execute();
  var status = response.getStatusCode();
  if (status != 200) {
    gs.error('API调用失败: ' + status);
    return null;
  }
  var data = JSON.parse(response.getBody());
} catch (e) {
  gs.error('异常: ' + e.message);
  return null;
}

4.4 Proper Logging

4.4 正确的日志记录

javascript
// ❌ BAD - No context in logs
gs.info('Error occurred');

// ✅ GOOD - Contextual logging
gs.info('[MyScriptInclude.process] Processing incident: ' +
        current.number + ', user: ' + gs.getUserName());
javascript
// ❌ 不良 - 日志中无上下文信息
gs.info('发生错误');

// ✅ 良好 - 带上下文的日志记录
gs.info('[MyScriptInclude.process] 处理事件: ' +
        current.number + ', 用户: ' + gs.getUserName());

5. Business Rule Specific

5. 业务规则特定项

5.1 Recursion Prevention

5.1 防止递归

javascript
// ❌ DANGEROUS - Can cause infinite loop
current.update();  // In a Before rule

// ✅ SAFE - Use workflow control
current.setWorkflow(false);
current.update();
current.setWorkflow(true);
javascript
// ❌ 危险 - 可能导致无限循环
current.update();  // 在Before规则中

// ✅ 安全 - 使用工作流控制
current.setWorkflow(false);
current.update();
current.setWorkflow(true);

5.2 Appropriate Rule Type

5.2 合适的规则类型

javascript
// ❌ WRONG - Heavy processing in Before rule
// Before rules should be fast!

// ✅ RIGHT - Use Async for heavy operations
// Move integrations and heavy processing to Async rules
javascript
// ❌ 错误 - 在Before规则中进行大量处理
// Before规则应保持快速执行!

// ✅ 正确 - 对大量操作使用Async规则
// 将集成和大量处理逻辑移至Async规则

6. Review Output Format

6. 审查输出格式

When reviewing code, structure your feedback as:
markdown
undefined
当审查代码时，请按以下结构组织反馈：
markdown
undefined

Code Review Summary

代码审查摘要

Critical Issues (Must Fix)

关键问题（必须修复）

  1. [SECURITY] Description...
  2. [ES5] Description...
  1. [SECURITY] 描述...
  2. [ES5] 描述...

Performance Issues (Should Fix)

性能问题（应该修复）

  1. [PERF] Description...
  1. [PERF] 描述...

Code Quality (Nice to Have)

代码质量（建议修复）

  1. [QUALITY] Description...
  1. [QUALITY] 描述...

Positive Observations

积极评价

  • Good use of...
  • Well-structured...
undefined
  • 良好使用了...
  • 结构清晰...
undefined

7. Severity Levels

7. 严重级别

LevelActionExamples
CRITICALMust fix before deploymentSecurity vulnerabilities, ES6 syntax
HIGHShould fixPerformance issues, missing error handling
MEDIUMRecommend fixingCode quality, hard-coded values
LOWConsider fixingStyle, minor improvements
级别操作示例
CRITICAL部署前必须修复安全漏洞、ES6语法
HIGH应该修复性能问题、缺失错误处理
MEDIUM建议修复代码质量、硬编码值
LOW考虑修复风格、微小改进