mirror of
https://github.com/awesome-skills/code-review-skill.git
synced 2026-09-28 06:45:38 +08:00
fix: correct review-guide errors and sync skill metadata
Correctness fixes in the reference guides: - django: len()/count() caching explanation was reversed; fix slicing/index cache example; replace nonexistent __acall__ async middleware with the documented markcoroutinefunction pattern; drop removed SECURE_BROWSER_XSS_FILTER; REFERRER_POLICY -> SECURE_REFERRER_POLICY - security: path-traversal guard compared a relative path to an absolute one (rejected every valid file); compare absolute-vs-absolute - nestjs: e2e ValidationPipe needs forbidNonWhitelisted/transform for the "extra field -> 400" test to pass - rust: select! cancel-safety example used read on both sides; bad case now uses non-cancel-safe read_exact - code-quality: "read less" example still read the whole file; use readline() - svelte: drop nonexistent unstate() (use $state.snapshot); fix devalue/Date note; comma-operator each-key -> template literal - c/cpp/qt: restore mangled markers that rendered as a literal ? - csharp: drop fabricated perf numbers - java: scope HashMap infinite-loop note to Java 7 and earlier - kotlin: closeableScope -> built-in viewModelScope - css: deprecated darken()/clip:rect() -> color.adjust/clip-path - typescript: legacy .eslintrc -> flat config (typescript-eslint v8) Tooling and metadata: - pr-analyzer.py: filename regex corrupted lib//web//db/ paths; parse the diff header via backreference and add utf-8/error handling; add test - SKILL.md: canonical name code-review-skill; document severity tiers; wire in pr-analyzer.py - README/CONTRIBUTING: fix stale skill name and line counts; complete the guide tree; add a conventions section
This commit is contained in:
Tu Shaokun
+23
-2
@@ -17,11 +17,19 @@ code-review-skill/
|
||||
├── reference/ # On-demand language/framework guides
|
||||
│ ├── react.md # React 19 / Next.js / TanStack Query v5
|
||||
│ ├── vue.md # Vue 3.5 Composition API
|
||||
│ ├── angular.md # Angular 17+, Signals, Standalone, RxJS
|
||||
│ ├── svelte.md # Svelte 5 / SvelteKit, runes, SSR boundary
|
||||
│ ├── rust.md # Ownership, async, unsafe, cancellation
|
||||
│ ├── typescript.md # Type safety, generics, strict mode
|
||||
│ ├── nestjs.md # NestJS DI, modules, Guards/Pipes, DTOs
|
||||
│ ├── python.md # Type hints, async, testing
|
||||
│ ├── django.md # Django / DRF, N+1, serializers, async views
|
||||
│ ├── fastapi.md # FastAPI, Depends, Pydantic v2, async
|
||||
│ ├── java.md # Java 17/21, Spring Boot 3, virtual threads
|
||||
│ ├── kotlin.md # Kotlin / Android, coroutines, Flow, Compose
|
||||
│ ├── go.md # Error handling, goroutines, context
|
||||
│ ├── csharp.md # C# / .NET 8, async, EF Core, ASP.NET Core
|
||||
│ ├── php.md # PHP 8.x, types, PDO, security, Composer
|
||||
│ ├── c.md # Memory safety, UB, error handling
|
||||
│ ├── cpp.md # RAII, move semantics, exception safety
|
||||
│ ├── qt.md # Object model, signals/slots, GUI perf
|
||||
@@ -30,6 +38,7 @@ code-review-skill/
|
||||
│ ├── performance-review-guide.md # Web Vitals, N+1, complexity
|
||||
│ ├── security-review-guide.md # OWASP Top 10, JWT, validation
|
||||
│ ├── common-bugs-checklist.md # Quick-reference bug patterns
|
||||
│ ├── code-quality-universal.md # Language-agnostic quality anti-patterns
|
||||
│ └── code-review-best-practices.md # Communication & process
|
||||
├── assets/ # Templates and quick reference
|
||||
│ ├── review-checklist.md
|
||||
@@ -73,7 +82,7 @@ allowed-tools: ["Read", "Grep", "Glob"] # 可选：限制工具访问
|
||||
- 避免下划线或大写字母
|
||||
|
||||
```
|
||||
✅ 正确：code-review-excellence, typescript-advanced-types
|
||||
✅ 正确：code-review-skill, typescript-advanced-types
|
||||
❌ 错误：CodeReview, code_review, TYPESCRIPT
|
||||
```
|
||||
|
||||
@@ -149,6 +158,18 @@ Claude 只在需要时加载支持文件，不会一次性加载所有内容。
|
||||
- 使用正斜杠 `/`，不使用反斜杠
|
||||
- 不需要 `./` 前缀
|
||||
|
||||
### 约定（Conventions）
|
||||
|
||||
**严重级别（severity）**：审查意见统一使用 SKILL.md「Technique 4」的标记方案，三档由红到绿表示优先级：
|
||||
|
||||
- 🔴 `[blocking]` - 合并前必须修复
|
||||
- 🟡 `[important]` - 应当修复，有异议可讨论
|
||||
- 🟢 `[nit]` - 可选优化，不阻塞合并
|
||||
|
||||
新增 reference 指南时请沿用这套标记，不要自创等价的名称（如 critical/warning/suggestion）。
|
||||
|
||||
**语言策略**：现有指南是中英混合的——部分通篇中文，部分（如 fastapi.md、php.md）以英文为主。新增内容时**跟随同一领域既有指南的语言**：改某个指南就用它的语言；新建指南可自行选择中文或英文，但单个文件内部保持一致。
|
||||
|
||||
---
|
||||
|
||||
## 贡献类型
|
||||
@@ -306,7 +327,7 @@ feat: 添加 Go 语言代码审查指南
|
||||
|
||||
将修改后的 Skill 复制到 `~/.claude/skills/` 目录，然后在 Claude Code 中测试：
|
||||
```bash
|
||||
cp -r ai-code-review-guide ~/.claude/skills/code-review-excellence
|
||||
cp -r code-review-skill ~/.claude/skills/code-review-skill
|
||||
```
|
||||
|
||||
### Q: 我应该更新 SKILL.md 还是 reference 文件？
|
||||
|
||||
@@ -99,7 +99,7 @@ It covers **20+ languages and frameworks** with over **16,000 lines** of careful
|
||||
<td rowspan="9"><strong>Backend</strong></td>
|
||||
<td>☕ Java 17/21 + Spring Boot 3</td>
|
||||
<td><code>reference/java.md</code></td>
|
||||
<td>~800</td>
|
||||
<td>~410</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td>⚡ FastAPI</td>
|
||||
@@ -155,12 +155,12 @@ It covers **20+ languages and frameworks** with over **16,000 lines** of careful
|
||||
<tr>
|
||||
<td>⚙️ C</td>
|
||||
<td><code>reference/c.md</code></td>
|
||||
<td>~210</td>
|
||||
<td>~290</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td>🔩 C++</td>
|
||||
<td><code>reference/cpp.md</code></td>
|
||||
<td>~300</td>
|
||||
<td>~390</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td>🖥️ Qt Framework</td>
|
||||
@@ -176,12 +176,12 @@ It covers **20+ languages and frameworks** with over **16,000 lines** of careful
|
||||
<tr>
|
||||
<td>⚡ Performance Review</td>
|
||||
<td><code>reference/performance-review-guide.md</code></td>
|
||||
<td>~850</td>
|
||||
<td>~820</td>
|
||||
</tr>
|
||||
<tr>
|
||||
<td>🔍 Universal Quality Anti-Patterns</td>
|
||||
<td><code>reference/code-quality-universal.md</code></td>
|
||||
<td>~320</td>
|
||||
<td>~490</td>
|
||||
</tr>
|
||||
</tbody>
|
||||
</table>
|
||||
@@ -382,7 +382,7 @@ Contributions are welcome! See [CONTRIBUTING.md](./CONTRIBUTING.md) for guidelin
|
||||
|
||||
**Ideas:**
|
||||
- New language guides (Ruby, Elixir, Scala...)
|
||||
- Framework-specific guides (Laravel, Spring WebFlux, FastAPI...)
|
||||
- Framework-specific guides (Laravel, Spring WebFlux...)
|
||||
- Additional checklists and templates
|
||||
- Translations of core documentation
|
||||
|
||||
@@ -427,7 +427,7 @@ MIT © [awesome-skills](https://github.com/awesome-skills)
|
||||
| | 🔥 Svelte 5 / SvelteKit | `reference/svelte.md` | ~1,060 |
|
||||
| | 🎨 CSS / Less / Sass | `reference/css-less-sass.md` | ~660 |
|
||||
| | 🔷 TypeScript | `reference/typescript.md` | ~540 |
|
||||
| **后端** | ☕ Java 17/21 + Spring Boot 3 | `reference/java.md` | ~800 |
|
||||
| **后端** | ☕ Java 17/21 + Spring Boot 3 | `reference/java.md` | ~410 |
|
||||
| | ⚡ FastAPI | `reference/fastapi.md` | ~590 |
|
||||
| | PHP 8.x | `reference/php.md` | ~700 |
|
||||
| | 📦 NestJS | `reference/nestjs.md` | ~590 |
|
||||
@@ -438,12 +438,12 @@ MIT © [awesome-skills](https://github.com/awesome-skills)
|
||||
| | 💻 C# / .NET 8 | `reference/csharp.md` | ~520 |
|
||||
| **移动 / 系统** | 📱 Kotlin / Android | `reference/kotlin.md` | ~1,020 |
|
||||
| | 🍎 Swift / SwiftUI | `reference/swift.md` | ~930 |
|
||||
| | ⚙️ C | `reference/c.md` | ~210 |
|
||||
| | 🔩 C++ | `reference/cpp.md` | ~300 |
|
||||
| | ⚙️ C | `reference/c.md` | ~290 |
|
||||
| | 🔩 C++ | `reference/cpp.md` | ~390 |
|
||||
| | 🖥️ Qt 框架 | `reference/qt.md` | ~190 |
|
||||
| **架构** | 🏛️ 架构设计审查 | `reference/architecture-review-guide.md` | ~470 |
|
||||
| | ⚡ 性能审查 | `reference/performance-review-guide.md` | ~850 |
|
||||
| | 🔍 通用质量反模式 | `reference/code-quality-universal.md` | ~320 |
|
||||
| | ⚡ 性能审查 | `reference/performance-review-guide.md` | ~820 |
|
||||
| | 🔍 通用质量反模式 | `reference/code-quality-universal.md` | ~490 |
|
||||
|
||||
---
|
||||
|
||||
@@ -641,7 +641,7 @@ Use code-review-skill to review this PR
|
||||
|
||||
**可贡献方向：**
|
||||
- 新增语言指南（Ruby、Elixir、Scala...）
|
||||
- 框架专属指南（Laravel、Spring WebFlux、FastAPI...）
|
||||
- 框架专属指南（Laravel、Spring WebFlux...）
|
||||
- 补充检查清单和审查模板
|
||||
- 核心文档的多语言翻译
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
---
|
||||
name: code-review-excellence
|
||||
name: code-review-skill
|
||||
description: |
|
||||
Provides comprehensive code review guidance for React 19, Vue 3, Angular 17+, Svelte 5, Rust, TypeScript, Java, PHP, Python, Django, Go, C#/.NET, Kotlin, Swift, NestJS, C/C++, and more.
|
||||
Helps catch bugs, improve code quality, and give constructive feedback.
|
||||
@@ -14,7 +14,7 @@ allowed-tools:
|
||||
- WebFetch # 查阅最新文档和最佳实践
|
||||
---
|
||||
|
||||
# Code Review Excellence
|
||||
# Code Review Skill
|
||||
|
||||
Transform code reviews from gatekeeping to knowledge sharing through constructive feedback, systematic analysis, and collaborative improvement.
|
||||
|
||||
@@ -99,6 +99,8 @@ Before diving into code, understand:
|
||||
4. Understand the business requirement
|
||||
5. Note any relevant architectural decisions
|
||||
|
||||
> For large diffs, pipe the diff through [`scripts/pr-analyzer.py`](scripts/pr-analyzer.py) (`git diff main...HEAD | python scripts/pr-analyzer.py`) to triage complexity and get a suggested review approach before reading.
|
||||
|
||||
### Phase 2: High-Level Review (5-10 minutes)
|
||||
|
||||
1. **Architecture & Design** - Does the solution fit the problem?
|
||||
@@ -170,6 +172,8 @@ Use labels to indicate priority:
|
||||
- 📚 `[learning]` - Educational comment, no action needed
|
||||
- 🎉 `[praise]` - Good work, keep it up!
|
||||
|
||||
**Severity levels:** 🔴 / 🟡 / 🟢 are the three severity tiers used as the standard across all guides in this skill — 🔴 blocks the merge, 🟡 should be addressed, 🟢 is optional. The remaining markers (💡 / 📚 / 🎉) are non-blocking annotations.
|
||||
|
||||
## Language-Specific Guides
|
||||
|
||||
根据审查的代码语言，查阅对应的详细指南：
|
||||
|
||||
@@ -38,11 +38,11 @@ Copy and use this template for your code reviews.
|
||||
|
||||
💡 **[suggestion]** [Alternative approach to consider]
|
||||
|
||||
## Questions
|
||||
## Learning Notes
|
||||
|
||||
❓ [Clarification needed about X]
|
||||
📚 [Educational context worth sharing about X]
|
||||
|
||||
❓ [Question about design decision Y]
|
||||
📚 [Background behind design decision Y]
|
||||
|
||||
## Security Considerations
|
||||
|
||||
@@ -106,9 +106,9 @@ Not blocking, but consider [improvement].
|
||||
[Why this is good]
|
||||
```
|
||||
|
||||
### Question
|
||||
### Learning
|
||||
```
|
||||
❓ **[question]** [Your question]
|
||||
📚 **[learning]** [Educational note]
|
||||
|
||||
I'm curious about the decision to [X]. Could you explain [Y]?
|
||||
For context, [X] works this way because [Y]. No action needed — just sharing.
|
||||
```
|
||||
|
||||
@@ -76,7 +76,7 @@ Quick reference checklist for code reviews.
|
||||
| 🟡 `[important]` | Should fix | Discuss if disagree |
|
||||
| 🟢 `[nit]` | Nice to have | Non-blocking |
|
||||
| 💡 `[suggestion]` | Alternative | Consider |
|
||||
| ❓ `[question]` | Need clarity | Respond |
|
||||
| 📚 `[learning]` | Educational comment | No action needed |
|
||||
| 🎉 `[praise]` | Good work | Celebrate! |
|
||||
|
||||
---
|
||||
|
||||
+22
-22
@@ -22,13 +22,13 @@
|
||||
### Always carry size with buffers
|
||||
|
||||
```c
|
||||
// ? Bad: ignores destination size
|
||||
// ❌ Bad: ignores destination size
|
||||
bool copy_name(char *dst, size_t dst_size, const char *src) {
|
||||
strcpy(dst, src);
|
||||
return true;
|
||||
}
|
||||
|
||||
// ? Good: validate size and terminate
|
||||
// ✅ Good: validate size and terminate
|
||||
bool copy_name(char *dst, size_t dst_size, const char *src) {
|
||||
size_t len = strlen(src);
|
||||
if (len + 1 > dst_size) {
|
||||
@@ -44,20 +44,20 @@ bool copy_name(char *dst, size_t dst_size, const char *src) {
|
||||
Prefer `snprintf`, `fgets`, and explicit bounds over `gets`, `strcpy`, or `sprintf`.
|
||||
|
||||
```c
|
||||
// ? Bad: unbounded write
|
||||
// ❌ Bad: unbounded write
|
||||
sprintf(buf, "%s", input);
|
||||
|
||||
// ? Good: bounded write
|
||||
// ✅ Good: bounded write
|
||||
snprintf(buf, buf_size, "%s", input);
|
||||
```
|
||||
|
||||
### Use the right copy primitive
|
||||
|
||||
```c
|
||||
// ? Bad: memcpy with overlapping regions
|
||||
// ❌ Bad: memcpy with overlapping regions
|
||||
memcpy(dst, src, len);
|
||||
|
||||
// ? Good: memmove handles overlap
|
||||
// ✅ Good: memmove handles overlap
|
||||
memmove(dst, src, len);
|
||||
```
|
||||
|
||||
@@ -70,7 +70,7 @@ memmove(dst, src, len);
|
||||
Track ownership and clean up on every error path.
|
||||
|
||||
```c
|
||||
// ? Good: cleanup label avoids leaks
|
||||
// ✅ Good: cleanup label avoids leaks
|
||||
int load_file(const char *path) {
|
||||
int rc = -1;
|
||||
FILE *f = NULL;
|
||||
@@ -107,23 +107,23 @@ cleanup:
|
||||
### Common UB patterns
|
||||
|
||||
```c
|
||||
// ? Bad: use after free
|
||||
// ❌ Bad: use after free
|
||||
char *p = malloc(10);
|
||||
free(p);
|
||||
p[0] = 'a';
|
||||
|
||||
// ? Bad: uninitialized read
|
||||
// ❌ Bad: uninitialized read
|
||||
int x;
|
||||
if (x > 0) { /* UB */ }
|
||||
|
||||
// ? Bad: signed overflow
|
||||
// ❌ Bad: signed overflow
|
||||
int sum = a + b;
|
||||
```
|
||||
|
||||
### Avoid pointer arithmetic past the object
|
||||
|
||||
```c
|
||||
// ? Bad: pointer past the end then dereference
|
||||
// ❌ Bad: pointer past the end then dereference
|
||||
int arr[4];
|
||||
int *p = arr + 4;
|
||||
int v = *p; // UB
|
||||
@@ -136,11 +136,11 @@ int v = *p; // UB
|
||||
### Avoid signed/unsigned surprises
|
||||
|
||||
```c
|
||||
// ? Bad: negative converted to large size_t
|
||||
// ❌ Bad: negative converted to large size_t
|
||||
int len = -1;
|
||||
size_t n = len;
|
||||
|
||||
// ? Good: validate before converting
|
||||
// ✅ Good: validate before converting
|
||||
if (len < 0) {
|
||||
return -1;
|
||||
}
|
||||
@@ -150,10 +150,10 @@ size_t n = (size_t)len;
|
||||
### Check for overflow in size calculations
|
||||
|
||||
```c
|
||||
// ? Bad: potential overflow in multiplication
|
||||
// ❌ Bad: potential overflow in multiplication
|
||||
size_t bytes = count * sizeof(Item);
|
||||
|
||||
// ? Good: check before multiplying
|
||||
// ✅ Good: check before multiplying
|
||||
if (count > SIZE_MAX / sizeof(Item)) {
|
||||
return NULL;
|
||||
}
|
||||
@@ -167,10 +167,10 @@ size_t bytes = count * sizeof(Item);
|
||||
### Always check return values
|
||||
|
||||
```c
|
||||
// ? Bad: ignore errors
|
||||
// ❌ Bad: ignore errors
|
||||
fread(buf, 1, size, f);
|
||||
|
||||
// ? Good: handle errors
|
||||
// ✅ Good: handle errors
|
||||
size_t read = fread(buf, 1, size, f);
|
||||
if (read != size && ferror(f)) {
|
||||
return -1;
|
||||
@@ -190,13 +190,13 @@ if (read != size && ferror(f)) {
|
||||
### volatile is not synchronization
|
||||
|
||||
```c
|
||||
// ? Bad: data race
|
||||
// ❌ Bad: data race
|
||||
volatile int stop = 0;
|
||||
void worker(void) {
|
||||
while (!stop) { /* ... */ }
|
||||
}
|
||||
|
||||
// ? Good: C11 atomics
|
||||
// ✅ Good: C11 atomics
|
||||
_Atomic int stop = 0;
|
||||
void worker(void) {
|
||||
while (!atomic_load(&stop)) { /* ... */ }
|
||||
@@ -214,11 +214,11 @@ Protect shared data with `pthread_mutex_t` or equivalent. Avoid holding locks wh
|
||||
### Parenthesize arguments
|
||||
|
||||
```c
|
||||
// ? Bad: macro with side effects
|
||||
// ❌ Bad: macro with side effects
|
||||
#define MIN(a, b) ((a) < (b) ? (a) : (b))
|
||||
int x = MIN(i++, j++);
|
||||
|
||||
// ? Good: static inline function
|
||||
// ✅ Good: static inline function
|
||||
static inline int min_int(int a, int b) {
|
||||
return a < b ? a : b;
|
||||
}
|
||||
@@ -231,7 +231,7 @@ static inline int min_int(int a, int b) {
|
||||
### Const-correctness and sizes
|
||||
|
||||
```c
|
||||
// ? Good: explicit size and const input
|
||||
// ✅ Good: explicit size and const input
|
||||
int hash_bytes(const uint8_t *data, size_t len, uint8_t *out);
|
||||
```
|
||||
|
||||
|
||||
@@ -395,9 +395,7 @@ try {
|
||||
content = Path("log.txt").read_text()
|
||||
first_line = content.split("\n")[0]
|
||||
|
||||
# ✅ 只读需要的内容
|
||||
first_line = Path("log.txt").read_text().split("\n", 1)[0]
|
||||
# 或更好的方式：逐行读取
|
||||
# ✅ 只读第一行，不加载整个文件
|
||||
with open("log.txt") as f:
|
||||
first_line = f.readline()
|
||||
```
|
||||
|
||||
+22
-22
@@ -24,7 +24,7 @@
|
||||
Use RAII to express ownership. Default to `std::unique_ptr`, use `std::shared_ptr` only for shared lifetime.
|
||||
|
||||
```cpp
|
||||
// ? Bad: manual new/delete with early returns
|
||||
// ❌ Bad: manual new/delete with early returns
|
||||
Foo* make_foo() {
|
||||
Foo* foo = new Foo();
|
||||
if (!foo->Init()) {
|
||||
@@ -34,7 +34,7 @@ Foo* make_foo() {
|
||||
return foo;
|
||||
}
|
||||
|
||||
// ? Good: RAII with unique_ptr
|
||||
// ✅ Good: RAII with unique_ptr
|
||||
std::unique_ptr<Foo> make_foo() {
|
||||
auto foo = std::make_unique<Foo>();
|
||||
if (!foo->Init()) {
|
||||
@@ -47,7 +47,7 @@ std::unique_ptr<Foo> make_foo() {
|
||||
### Wrap C resources
|
||||
|
||||
```cpp
|
||||
// ? Good: wrap FILE* with unique_ptr
|
||||
// ✅ Good: wrap FILE* with unique_ptr
|
||||
using FilePtr = std::unique_ptr<FILE, decltype(&fclose)>;
|
||||
|
||||
FilePtr open_file(const char* path) {
|
||||
@@ -64,18 +64,18 @@ FilePtr open_file(const char* path) {
|
||||
`std::string_view` and `std::span` do not own data. Make sure the owner outlives the view.
|
||||
|
||||
```cpp
|
||||
// ? Bad: returning string_view to a temporary
|
||||
// ❌ Bad: returning string_view to a temporary
|
||||
std::string_view bad_view() {
|
||||
std::string s = make_name();
|
||||
return s; // dangling
|
||||
}
|
||||
|
||||
// ? Good: return owning string
|
||||
// ✅ Good: return owning string
|
||||
std::string good_name() {
|
||||
return make_name();
|
||||
}
|
||||
|
||||
// ? Good: view tied to caller-owned data
|
||||
// ✅ Good: view tied to caller-owned data
|
||||
std::string_view good_view(const std::string& s) {
|
||||
return s;
|
||||
}
|
||||
@@ -84,13 +84,13 @@ std::string_view good_view(const std::string& s) {
|
||||
### Lambda captures
|
||||
|
||||
```cpp
|
||||
// ? Bad: capture reference that escapes
|
||||
// ❌ Bad: capture reference that escapes
|
||||
std::function<void()> make_task() {
|
||||
int value = 42;
|
||||
return [&]() { use(value); }; // dangling
|
||||
}
|
||||
|
||||
// ? Good: capture by value
|
||||
// ✅ Good: capture by value
|
||||
std::function<void()> make_task() {
|
||||
int value = 42;
|
||||
return [value]() { use(value); };
|
||||
@@ -106,7 +106,7 @@ std::function<void()> make_task() {
|
||||
Prefer the Rule of 0 by using RAII types. If you own a resource, define or delete copy and move operations.
|
||||
|
||||
```cpp
|
||||
// ? Bad: raw ownership with default copy
|
||||
// ❌ Bad: raw ownership with default copy
|
||||
struct Buffer {
|
||||
int* data;
|
||||
size_t size;
|
||||
@@ -115,7 +115,7 @@ struct Buffer {
|
||||
// copy ctor/assign are implicitly generated -> double delete
|
||||
};
|
||||
|
||||
// ? Good: Rule of 0 with std::vector
|
||||
// ✅ Good: Rule of 0 with std::vector
|
||||
struct Buffer {
|
||||
std::vector<int> data;
|
||||
explicit Buffer(size_t n) : data(n) {}
|
||||
@@ -164,10 +164,10 @@ struct Millis {
|
||||
struct Shape { virtual ~Shape() = default; };
|
||||
struct Circle : Shape { void draw() const; };
|
||||
|
||||
// ? Bad: slices Circle into Shape
|
||||
// ❌ Bad: slices Circle into Shape
|
||||
void draw(Shape shape);
|
||||
|
||||
// ? Good: pass by reference
|
||||
// ✅ Good: pass by reference
|
||||
void draw(const Shape& shape);
|
||||
```
|
||||
|
||||
@@ -190,7 +190,7 @@ struct Worker final : Base {
|
||||
### Prefer RAII for cleanup
|
||||
|
||||
```cpp
|
||||
// ? Good: RAII handles cleanup on exceptions
|
||||
// ✅ Good: RAII handles cleanup on exceptions
|
||||
void process() {
|
||||
std::vector<int> data = load_data(); // safe cleanup
|
||||
do_work(data);
|
||||
@@ -209,7 +209,7 @@ struct File {
|
||||
### Use expected results for normal failures
|
||||
|
||||
```cpp
|
||||
// ? Expected error: use optional or expected
|
||||
// ✅ Expected error: use optional or expected
|
||||
std::optional<int> parse_int(const std::string& s) {
|
||||
try {
|
||||
return std::stoi(s);
|
||||
@@ -226,11 +226,11 @@ std::optional<int> parse_int(const std::string& s) {
|
||||
### Protect shared data
|
||||
|
||||
```cpp
|
||||
// ? Bad: data race
|
||||
// ❌ Bad: data race
|
||||
int counter = 0;
|
||||
void inc() { counter++; }
|
||||
|
||||
// ? Good: atomic
|
||||
// ✅ Good: atomic
|
||||
std::atomic<int> counter{0};
|
||||
void inc() { counter.fetch_add(1, std::memory_order_relaxed); }
|
||||
```
|
||||
@@ -254,7 +254,7 @@ void add(int v) {
|
||||
### Avoid repeated allocations
|
||||
|
||||
```cpp
|
||||
// ? Bad: repeated reallocation
|
||||
// ❌ Bad: repeated reallocation
|
||||
std::vector<int> build(int n) {
|
||||
std::vector<int> out;
|
||||
for (int i = 0; i < n; ++i) {
|
||||
@@ -263,7 +263,7 @@ std::vector<int> build(int n) {
|
||||
return out;
|
||||
}
|
||||
|
||||
// ? Good: reserve upfront
|
||||
// ✅ Good: reserve upfront
|
||||
std::vector<int> build(int n) {
|
||||
std::vector<int> out;
|
||||
out.reserve(static_cast<size_t>(n));
|
||||
@@ -277,7 +277,7 @@ std::vector<int> build(int n) {
|
||||
### String concatenation
|
||||
|
||||
```cpp
|
||||
// ? Bad: repeated allocation
|
||||
// ❌ Bad: repeated allocation
|
||||
std::string join(const std::vector<std::string>& parts) {
|
||||
std::string out;
|
||||
for (const auto& p : parts) {
|
||||
@@ -286,7 +286,7 @@ std::string join(const std::vector<std::string>& parts) {
|
||||
return out;
|
||||
}
|
||||
|
||||
// ? Good: reserve total size
|
||||
// ✅ Good: reserve total size
|
||||
std::string join(const std::vector<std::string>& parts) {
|
||||
size_t total = 0;
|
||||
for (const auto& p : parts) {
|
||||
@@ -308,13 +308,13 @@ std::string join(const std::vector<std::string>& parts) {
|
||||
### Prefer constrained templates (C++20)
|
||||
|
||||
```cpp
|
||||
// ? Bad: overly generic
|
||||
// ❌ Bad: overly generic
|
||||
template <typename T>
|
||||
T add(T a, T b) {
|
||||
return a + b;
|
||||
}
|
||||
|
||||
// ? Good: constrained
|
||||
// ✅ Good: constrained
|
||||
template <typename T>
|
||||
requires std::is_integral_v<T>
|
||||
T add(T a, T b) {
|
||||
|
||||
+2
-2
@@ -247,7 +247,7 @@ var blogs = await context.Blogs
|
||||
// ❌ 默认跟踪——只读查询也付出跟踪开销
|
||||
var products = await context.Products.ToListAsync();
|
||||
|
||||
// ✅ AsNoTracking——性能提升 ~30%，内存减少 ~40%
|
||||
// ✅ AsNoTracking——跳过变更跟踪，更快且更省内存
|
||||
var products = await context.Products
|
||||
.AsNoTracking()
|
||||
.ToListAsync();
|
||||
@@ -334,7 +334,7 @@ var form = await HttpContext.Request.ReadFormAsync();
|
||||
### 异常用于控制流
|
||||
|
||||
```csharp
|
||||
// ❌ 用异常判断是否存在——比检查慢 10-100 倍
|
||||
// ❌ 用异常判断是否存在——异常开销大，比直接检查慢得多
|
||||
try
|
||||
{
|
||||
var user = await _db.Users.FirstAsync(u => u.Id == id);
|
||||
|
||||
@@ -562,6 +562,8 @@ module.exports = {
|
||||
### Mixin vs Extend vs 变量
|
||||
|
||||
```scss
|
||||
@use 'sass:color';
|
||||
|
||||
/* 变量 - 用于单个值 */
|
||||
$primary-color: #3b82f6;
|
||||
|
||||
@@ -570,7 +572,9 @@ $primary-color: #3b82f6;
|
||||
background: $bg;
|
||||
color: $text;
|
||||
&:hover {
|
||||
background: darken($bg, 10%);
|
||||
// Dart Sass 已弃用全局 darken()/lighten()，改用 color 模块
|
||||
background: color.adjust($bg, $lightness: -10%);
|
||||
// color.scale($bg, $lightness: -10%) 按比例调整，深浅过渡更自然
|
||||
}
|
||||
}
|
||||
|
||||
@@ -580,7 +584,8 @@ $primary-color: #3b82f6;
|
||||
width: 1px;
|
||||
height: 1px;
|
||||
overflow: hidden;
|
||||
clip: rect(0, 0, 0, 0);
|
||||
clip-path: inset(50%); /* clip: rect() 已弃用，改用 clip-path */
|
||||
white-space: nowrap; /* 避免内容被挤成一列后撑开布局 */
|
||||
}
|
||||
|
||||
.sr-only {
|
||||
|
||||
+30
-28
@@ -223,15 +223,15 @@ for author in authors:
|
||||
### QuerySet 缓存误用
|
||||
|
||||
```python
|
||||
# ❌ 重复评估同一个 QuerySet
|
||||
# ❌ count() 后再迭代 —— 两次查询
|
||||
qs = Book.objects.all()
|
||||
count = len(qs) # 评估 1: SELECT COUNT(*)
|
||||
titles = [b.title for b in qs] # 评估 2: SELECT * — 缓存失效！
|
||||
count = qs.count() # 查询 1: SELECT COUNT(*) — 不填充缓存
|
||||
titles = [b.title for b in qs] # 查询 2: SELECT * — 重新评估
|
||||
|
||||
# ✅ 使用 count() 和一次性迭代
|
||||
# ✅ 既要对象又要数量时，用 len() 触发一次评估并复用缓存
|
||||
qs = Book.objects.all()
|
||||
count = qs.count() # SELECT COUNT(*) — 不填充缓存
|
||||
titles = [b.title for b in qs] # SELECT * — 唯一一次评估
|
||||
count = len(qs) # 查询 1: SELECT * — 全部加载并缓存
|
||||
titles = [b.title for b in qs] # 复用缓存，无新查询
|
||||
|
||||
# ✅ 如果需要多次迭代，先转 list
|
||||
books = list(Book.objects.all()) # 一次查询
|
||||
@@ -239,18 +239,24 @@ count = len(books)
|
||||
titles = [b.title for b in books]
|
||||
```
|
||||
|
||||
### 切片不填充缓存
|
||||
### 切片/索引不填充缓存
|
||||
|
||||
```python
|
||||
# ❌ 切片后迭代触发两次查询
|
||||
qs = Book.objects.all()[:10] # 切片：不填充缓存
|
||||
first = list(qs) # 查询 1
|
||||
second = list(qs) # 查询 2 — 重复！
|
||||
# ❌ 反复索引未评估的 QuerySet —— 每次都查库
|
||||
qs = Book.objects.all()
|
||||
qs[0] # 查询 1: SELECT ... LIMIT 1
|
||||
qs[0] # 查询 2 — 切片/索引不会填充缓存
|
||||
|
||||
# ✅ 切片后立即转 list
|
||||
books = list(Book.objects.all()[:10]) # 一次查询
|
||||
first = books
|
||||
second = list(books) # 使用 Python list，无查询
|
||||
# ✅ 先整体评估，缓存保存所有行，之后索引走缓存
|
||||
qs = Book.objects.all()
|
||||
list(qs) # SELECT * — 评估并缓存全部行
|
||||
qs[0] # 走缓存，无查询
|
||||
qs[5] # 走缓存，无查询
|
||||
|
||||
# ✅ 只需要前 N 条时，切一次并转 list
|
||||
books = list(Book.objects.all()[:10]) # 一次查询：SELECT ... LIMIT 10
|
||||
first = books[0]
|
||||
rest = books[1:] # 已是 Python list，无查询
|
||||
```
|
||||
|
||||
### len() vs count()
|
||||
@@ -743,31 +749,28 @@ class TimingMiddleware:
|
||||
response["X-Elapsed"] = str(elapsed)
|
||||
return response
|
||||
|
||||
# ✅ 同时支持同步和异步的中间件
|
||||
# ✅ async-capable 中间件：async def __call__，并在 __init__ 里标记实例
|
||||
import time
|
||||
from asgiref.sync import iscoroutinefunction, markcoroutinefunction
|
||||
|
||||
class TimingMiddleware:
|
||||
async_capable = True
|
||||
sync_capable = True
|
||||
sync_capable = False
|
||||
|
||||
def __init__(self, get_response):
|
||||
self.get_response = get_response
|
||||
# get_response 是协程函数时标记自己，Django 才会 await 这个实例
|
||||
if iscoroutinefunction(self.get_response):
|
||||
markcoroutinefunction(self)
|
||||
|
||||
async def __acall__(self, request):
|
||||
async def __call__(self, request):
|
||||
start = time.time()
|
||||
response = await self.get_response(request)
|
||||
elapsed = time.time() - start
|
||||
response["X-Elapsed"] = str(elapsed)
|
||||
return response
|
||||
|
||||
def __call__(self, request):
|
||||
start = time.time()
|
||||
response = self.get_response(request)
|
||||
elapsed = time.time() - start
|
||||
response["X-Elapsed"] = str(elapsed)
|
||||
return response
|
||||
|
||||
# ✅ 或者使用 Django 内置的 async 安全装饰器
|
||||
# ✅ 要同时兼容同步和异步，用工厂函数 + 内置装饰器
|
||||
from django.utils.decorators import sync_and_async_middleware
|
||||
```
|
||||
|
||||
@@ -839,9 +842,8 @@ SECURE_HSTS_SECONDS = 31536000 # 1 year HSTS
|
||||
SECURE_HSTS_INCLUDE_SUBDOMAINS = True
|
||||
SECURE_HSTS_PRELOAD = True
|
||||
SECURE_CONTENT_TYPE_NOSNIFF = True # X-Content-Type-Options: nosniff
|
||||
SECURE_BROWSER_XSS_FILTER = True # X-XSS-Protection: 1; mode=block
|
||||
X_FRAME_OPTIONS = "DENY" # 防止 clickjacking
|
||||
REFERRER_POLICY = "strict-origin-when-cross-origin"
|
||||
SECURE_REFERRER_POLICY = "strict-origin-when-cross-origin"
|
||||
|
||||
# --- 密码验证 ---
|
||||
AUTH_PASSWORD_VALIDATORS = [
|
||||
|
||||
+1
-1
@@ -292,7 +292,7 @@ private static final SimpleDateFormat sdf = new SimpleDateFormat("yyyy-MM-dd");
|
||||
// ✅ 使用 DateTimeFormatter (Java 8+)
|
||||
private static final DateTimeFormatter dtf = DateTimeFormatter.ofPattern("yyyy-MM-dd");
|
||||
|
||||
// ❌ HashMap 在多线程环境可能死循环或数据丢失
|
||||
// ❌ HashMap 在多线程环境会数据丢失（Java 7 及之前 resize 还可能死循环，Java 8 修复了死循环但仍非线程安全）
|
||||
// ✅ 使用 ConcurrentHashMap
|
||||
Map<String, String> cache = new ConcurrentHashMap<>();
|
||||
```
|
||||
|
||||
+1
-1
@@ -694,7 +694,7 @@ class MyManager(private val scope: CoroutineScope) {
|
||||
}
|
||||
}
|
||||
|
||||
// ✅ ViewModel 中使用 closeableScope（Kotlin 2.1+）
|
||||
// ✅ ViewModel 里直接用内置的 viewModelScope，不用自己管生命周期
|
||||
class MyViewModel : ViewModel() {
|
||||
private val scope = viewModelScope + Dispatchers.IO
|
||||
// Automatically cancelled when ViewModel is cleared
|
||||
|
||||
+7
-1
@@ -516,7 +516,13 @@ describe('UsersController (e2e)', () => {
|
||||
|
||||
app = moduleFixture.createNestApplication();
|
||||
// 必须与 main.ts 中相同的全局配置
|
||||
app.useGlobalPipes(new ValidationPipe({ whitelist: true }));
|
||||
app.useGlobalPipes(
|
||||
new ValidationPipe({
|
||||
whitelist: true,
|
||||
forbidNonWhitelisted: true,
|
||||
transform: true,
|
||||
}),
|
||||
);
|
||||
await app.init();
|
||||
});
|
||||
|
||||
|
||||
+1
-1
@@ -74,7 +74,7 @@ Check logic that might cause infinite signal loops (e.g., `valueChanged` -> `set
|
||||
|
||||
```cpp
|
||||
void MyClass::setValue(int v) {
|
||||
if (m_value == v) return; // ? Good: Break loop
|
||||
if (m_value == v) return; // ✅ Good: Break loop
|
||||
m_value = v;
|
||||
emit valueChanged(v);
|
||||
}
|
||||
|
||||
+8
-6
@@ -309,10 +309,11 @@ async fn bad_select(stream: &mut TcpStream) {
|
||||
let mut buffer = vec![0u8; 1024];
|
||||
loop {
|
||||
select! {
|
||||
// 如果 timeout 先完成，read 被取消
|
||||
// 部分读取的数据可能丢失！
|
||||
result = stream.read(&mut buffer) => {
|
||||
handle_data(&buffer[..result?]);
|
||||
// read_exact 不是取消安全的：timeout 先完成时，
|
||||
// 已经读进 buffer 的部分字节会随 Future 一起丢弃
|
||||
result = stream.read_exact(&mut buffer) => {
|
||||
result?;
|
||||
handle_data(&buffer);
|
||||
}
|
||||
_ = tokio::time::sleep(Duration::from_secs(5)) => {
|
||||
println!("Timeout");
|
||||
@@ -326,8 +327,9 @@ async fn good_select(stream: &mut TcpStream) {
|
||||
let mut buffer = vec![0u8; 1024];
|
||||
loop {
|
||||
select! {
|
||||
// tokio::io::AsyncReadExt::read 是取消安全的
|
||||
// 取消时，未读取的数据留在流中
|
||||
// read 是取消安全的：被取消时未读取的数据仍留在流中
|
||||
// 真的需要按定长读取时，把 read_exact 丢到单独的 task 里，
|
||||
// 这里 select! 它的 JoinHandle，取消就不会丢字节
|
||||
result = stream.read(&mut buffer) => {
|
||||
match result {
|
||||
Ok(0) => break, // EOF
|
||||
|
||||
@@ -96,10 +96,11 @@ const filePath = `./uploads/${req.params.filename}`;
|
||||
// ✅ Validate and sanitize path
|
||||
const path = require('path');
|
||||
const safeName = path.basename(req.params.filename);
|
||||
const filePath = path.join('./uploads', safeName);
|
||||
const uploadsDir = path.resolve('./uploads');
|
||||
const filePath = path.resolve(uploadsDir, safeName);
|
||||
|
||||
// Verify it's still within uploads directory
|
||||
if (!filePath.startsWith(path.resolve('./uploads'))) {
|
||||
// Verify it's still within uploads directory (both sides absolute)
|
||||
if (!filePath.startsWith(uploadsDir + path.sep)) {
|
||||
throw new Error('Invalid path');
|
||||
}
|
||||
```
|
||||
|
||||
+3
-7
@@ -76,15 +76,11 @@ Svelte 5 / SvelteKit 审查重点：Runes 响应式系统、Server/Client 边界
|
||||
|
||||
<!-- ✅ $state.snapshot 获取普通对象副本 -->
|
||||
<script lang="ts">
|
||||
import { unstate } from 'svelte';
|
||||
|
||||
let state = $state({ x: 0, y: 0 });
|
||||
|
||||
onMount(() => {
|
||||
// $state.snapshot produces a plain object (Svelte 5)
|
||||
chartLibrary.update($state.snapshot(state));
|
||||
// or use unstate() for the same purpose
|
||||
chartLibrary.update(unstate(state));
|
||||
});
|
||||
</script>
|
||||
```
|
||||
@@ -385,7 +381,7 @@ export async function load() {
|
||||
return {
|
||||
stream: fs.createReadStream('data.csv'), // not serializable!
|
||||
callback: () => console.log('hi'), // functions not serializable!
|
||||
date: new Date(), // becomes string via devalue
|
||||
date: new Date(), // OK — devalue serializes Date/Map/Set fine
|
||||
};
|
||||
}
|
||||
|
||||
@@ -863,7 +859,7 @@ export const actions = {
|
||||
{/each}
|
||||
|
||||
<!-- ✅ 复合 key -->
|
||||
{#each items as item (item.category, item.id)}
|
||||
{#each items as item (`${item.category}-${item.id}`)}
|
||||
<div>{item.name}</div>
|
||||
{/each}
|
||||
```
|
||||
@@ -1006,7 +1002,7 @@ event.cookies.set('session', token, {
|
||||
- [ ] $state 只用于会变化的值，常量直接声明
|
||||
- [ ] 大型不可变数据使用 $state.raw
|
||||
- [ ] 没有解构 $state 对象（会丢失响应性）
|
||||
- [ ] 外部库使用 $state.snapshot / unstate 传入普通对象
|
||||
- [ ] 外部库使用 $state.snapshot 传入普通对象
|
||||
- [ ] $derived 中没有副作用
|
||||
- [ ] 没有用 $effect 替代 $derived 做状态同步
|
||||
- [ ] $effect 中不修改被追踪的状态（避免无限循环）
|
||||
|
||||
+36
-26
@@ -446,34 +446,44 @@ const routes = createConfig(['home', 'about', 'contact'] as const);
|
||||
### 推荐的 @typescript-eslint 规则
|
||||
|
||||
```javascript
|
||||
// .eslintrc.js
|
||||
module.exports = {
|
||||
extends: [
|
||||
'eslint:recommended',
|
||||
'plugin:@typescript-eslint/recommended',
|
||||
'plugin:@typescript-eslint/recommended-requiring-type-checking',
|
||||
'plugin:@typescript-eslint/strict'
|
||||
],
|
||||
rules: {
|
||||
// ✅ 类型安全
|
||||
'@typescript-eslint/no-explicit-any': 'error',
|
||||
'@typescript-eslint/no-unsafe-assignment': 'error',
|
||||
'@typescript-eslint/no-unsafe-member-access': 'error',
|
||||
'@typescript-eslint/no-unsafe-call': 'error',
|
||||
'@typescript-eslint/no-unsafe-return': 'error',
|
||||
// eslint.config.js（flat config，typescript-eslint v8）
|
||||
import eslint from '@eslint/js';
|
||||
import tseslint from 'typescript-eslint';
|
||||
|
||||
// ✅ 最佳实践
|
||||
'@typescript-eslint/explicit-function-return-type': 'warn',
|
||||
'@typescript-eslint/no-floating-promises': 'error',
|
||||
'@typescript-eslint/await-thenable': 'error',
|
||||
'@typescript-eslint/no-misused-promises': 'error',
|
||||
export default tseslint.config(
|
||||
eslint.configs.recommended,
|
||||
// 需要类型信息的规则集，对应旧的 recommended-requiring-type-checking
|
||||
tseslint.configs.recommendedTypeChecked,
|
||||
tseslint.configs.strictTypeChecked,
|
||||
{
|
||||
languageOptions: {
|
||||
parserOptions: {
|
||||
// 让带类型的规则自动找到对应 tsconfig
|
||||
projectService: true,
|
||||
tsconfigRootDir: import.meta.dirname,
|
||||
},
|
||||
},
|
||||
rules: {
|
||||
// ✅ 类型安全
|
||||
'@typescript-eslint/no-explicit-any': 'error',
|
||||
'@typescript-eslint/no-unsafe-assignment': 'error',
|
||||
'@typescript-eslint/no-unsafe-member-access': 'error',
|
||||
'@typescript-eslint/no-unsafe-call': 'error',
|
||||
'@typescript-eslint/no-unsafe-return': 'error',
|
||||
|
||||
// ✅ 代码风格
|
||||
'@typescript-eslint/consistent-type-imports': 'error',
|
||||
'@typescript-eslint/prefer-nullish-coalescing': 'error',
|
||||
'@typescript-eslint/prefer-optional-chain': 'error'
|
||||
}
|
||||
};
|
||||
// ✅ 最佳实践
|
||||
'@typescript-eslint/explicit-function-return-type': 'warn',
|
||||
'@typescript-eslint/no-floating-promises': 'error',
|
||||
'@typescript-eslint/await-thenable': 'error',
|
||||
'@typescript-eslint/no-misused-promises': 'error',
|
||||
|
||||
// ✅ 代码风格
|
||||
'@typescript-eslint/consistent-type-imports': 'error',
|
||||
'@typescript-eslint/prefer-nullish-coalescing': 'error',
|
||||
'@typescript-eslint/prefer-optional-chain': 'error',
|
||||
},
|
||||
},
|
||||
);
|
||||
```
|
||||
|
||||
### 常见 ESLint 错误修复
|
||||
|
||||
+21
-10
@@ -131,8 +131,13 @@ def parse_diff(diff_content: str) -> List[FileStats]:
|
||||
if line.startswith('diff --git'):
|
||||
if current_file:
|
||||
files.append(current_file)
|
||||
# Extract filename from "diff --git a/path b/path"
|
||||
match = re.search(r'b/(.+)$', line)
|
||||
# "diff --git a/<path> b/<path>" — match the b/ side via a
|
||||
# backreference so a literal "b/" inside paths like lib/, web/ or
|
||||
# db/ can't be mistaken for the prefix. Renames have differing
|
||||
# paths, so fall back to the b/ side after the separating space.
|
||||
match = re.match(r'diff --git a/(.+?) b/\1', line)
|
||||
if not match:
|
||||
match = re.search(r' b/(.+)$', line)
|
||||
if match:
|
||||
filename = match.group(1)
|
||||
current_file = FileStats(
|
||||
@@ -141,6 +146,8 @@ def parse_diff(diff_content: str) -> List[FileStats]:
|
||||
is_test=is_test_file(filename),
|
||||
is_config=is_config_file(filename),
|
||||
)
|
||||
else:
|
||||
current_file = None
|
||||
elif current_file:
|
||||
if line.startswith('+') and not line.startswith('+++'):
|
||||
current_file.additions += 1
|
||||
@@ -355,14 +362,18 @@ def main():
|
||||
args = parser.parse_args()
|
||||
|
||||
# Read diff from file or stdin
|
||||
if args.diff_file:
|
||||
with open(args.diff_file, 'r') as f:
|
||||
diff_content = f.read()
|
||||
elif not sys.stdin.isatty():
|
||||
diff_content = sys.stdin.read()
|
||||
else:
|
||||
print("Usage: git diff main...HEAD | python pr-analyzer.py")
|
||||
print(" python pr-analyzer.py -f diff.txt")
|
||||
try:
|
||||
if args.diff_file:
|
||||
with open(args.diff_file, 'r', encoding='utf-8', errors='replace') as f:
|
||||
diff_content = f.read()
|
||||
elif not sys.stdin.isatty():
|
||||
diff_content = sys.stdin.buffer.read().decode('utf-8', errors='replace')
|
||||
else:
|
||||
print("Usage: git diff main...HEAD | python pr-analyzer.py")
|
||||
print(" python pr-analyzer.py -f diff.txt")
|
||||
sys.exit(1)
|
||||
except OSError as e:
|
||||
print(f"Error reading diff input: {e}", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
||||
if not diff_content.strip():
|
||||
|
||||
@@ -0,0 +1,75 @@
|
||||
#!/usr/bin/env python3
|
||||
"""Tests for pr-analyzer.py diff parsing (stdlib unittest, no extra deps)."""
|
||||
|
||||
import importlib.util
|
||||
import os
|
||||
import unittest
|
||||
|
||||
# The script has a hyphen in its name, so load it by path.
|
||||
_HERE = os.path.dirname(os.path.abspath(__file__))
|
||||
_spec = importlib.util.spec_from_file_location(
|
||||
'pr_analyzer', os.path.join(_HERE, 'pr-analyzer.py')
|
||||
)
|
||||
pr_analyzer = importlib.util.module_from_spec(_spec)
|
||||
_spec.loader.exec_module(pr_analyzer)
|
||||
|
||||
|
||||
class ParseDiffFilenameTest(unittest.TestCase):
|
||||
def test_lib_prefixed_path(self):
|
||||
# "lib/" embeds a literal "b/" that the old regex swallowed.
|
||||
diff = (
|
||||
"diff --git a/lib/foo.py b/lib/foo.py\n"
|
||||
"index 1234567..89abcde 100644\n"
|
||||
"--- a/lib/foo.py\n"
|
||||
"+++ b/lib/foo.py\n"
|
||||
"@@ -1,2 +1,3 @@\n"
|
||||
" unchanged\n"
|
||||
"+added line\n"
|
||||
"-removed line\n"
|
||||
)
|
||||
files = pr_analyzer.parse_diff(diff)
|
||||
self.assertEqual(len(files), 1)
|
||||
self.assertEqual(files[0].filename, 'lib/foo.py')
|
||||
self.assertEqual(files[0].additions, 1)
|
||||
self.assertEqual(files[0].deletions, 1)
|
||||
|
||||
def test_normal_path(self):
|
||||
diff = (
|
||||
"diff --git a/src/main.py b/src/main.py\n"
|
||||
"index 1111111..2222222 100644\n"
|
||||
"--- a/src/main.py\n"
|
||||
"+++ b/src/main.py\n"
|
||||
"@@ -0,0 +1 @@\n"
|
||||
"+print('hi')\n"
|
||||
)
|
||||
files = pr_analyzer.parse_diff(diff)
|
||||
self.assertEqual(len(files), 1)
|
||||
self.assertEqual(files[0].filename, 'src/main.py')
|
||||
|
||||
def test_other_embedded_b_slash_prefixes(self):
|
||||
# web/ and db/ also contain a literal "b/".
|
||||
diff = (
|
||||
"diff --git a/web/x.js b/web/x.js\n"
|
||||
"+++ b/web/x.js\n"
|
||||
"+console.log(1)\n"
|
||||
"diff --git a/db/y.sql b/db/y.sql\n"
|
||||
"+++ b/db/y.sql\n"
|
||||
"+SELECT 1;\n"
|
||||
)
|
||||
files = pr_analyzer.parse_diff(diff)
|
||||
self.assertEqual([f.filename for f in files], ['web/x.js', 'db/y.sql'])
|
||||
|
||||
def test_rename_falls_back_to_b_side(self):
|
||||
diff = (
|
||||
"diff --git a/old/name.py b/new/name.py\n"
|
||||
"similarity index 100%\n"
|
||||
"rename from old/name.py\n"
|
||||
"rename to new/name.py\n"
|
||||
)
|
||||
files = pr_analyzer.parse_diff(diff)
|
||||
self.assertEqual(len(files), 1)
|
||||
self.assertEqual(files[0].filename, 'new/name.py')
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
unittest.main()
|
||||
Reference in New Issue
Block a user