diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c599523..04acd97 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -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 文件？ diff --git a/README.md b/README.md index 4bcb61c..7ae043c 100644 --- a/README.md +++ b/README.md @@ -99,7 +99,7 @@ It covers **20+ languages and frameworks** with over **16,000 lines** of careful Backend ☕ Java 17/21 + Spring Boot 3 reference/java.md - ~800 + ~410 ⚡ FastAPI @@ -155,12 +155,12 @@ It covers **20+ languages and frameworks** with over **16,000 lines** of careful ⚙️ C reference/c.md - ~210 + ~290 🔩 C++ reference/cpp.md - ~300 + ~390 🖥️ Qt Framework @@ -176,12 +176,12 @@ It covers **20+ languages and frameworks** with over **16,000 lines** of careful ⚡ Performance Review reference/performance-review-guide.md - ~850 + ~820 🔍 Universal Quality Anti-Patterns reference/code-quality-universal.md - ~320 + ~490 @@ -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...） - 补充检查清单和审查模板 - 核心文档的多语言翻译 diff --git a/SKILL.md b/SKILL.md index 1913087..edc8db6 100644 --- a/SKILL.md +++ b/SKILL.md @@ -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 根据审查的代码语言，查阅对应的详细指南： diff --git a/assets/pr-review-template.md b/assets/pr-review-template.md index 33f7e71..5c1b9d9 100644 --- a/assets/pr-review-template.md +++ b/assets/pr-review-template.md @@ -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. ``` diff --git a/assets/review-checklist.md b/assets/review-checklist.md index 4ff3b8b..60beda0 100644 --- a/assets/review-checklist.md +++ b/assets/review-checklist.md @@ -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! | --- diff --git a/reference/c.md b/reference/c.md index cfd31ed..83c3988 100644 --- a/reference/c.md +++ b/reference/c.md @@ -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); ``` diff --git a/reference/code-quality-universal.md b/reference/code-quality-universal.md index 59dfe55..97b3d3c 100644 --- a/reference/code-quality-universal.md +++ b/reference/code-quality-universal.md @@ -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() ``` diff --git a/reference/cpp.md b/reference/cpp.md index 58743f6..98d764f 100644 --- a/reference/cpp.md +++ b/reference/cpp.md @@ -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 make_foo() { auto foo = std::make_unique(); if (!foo->Init()) { @@ -47,7 +47,7 @@ std::unique_ptr make_foo() { ### Wrap C resources ```cpp -// ? Good: wrap FILE* with unique_ptr +// ✅ Good: wrap FILE* with unique_ptr using FilePtr = std::unique_ptr; 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 make_task() { int value = 42; return [&]() { use(value); }; // dangling } -// ? Good: capture by value +// ✅ Good: capture by value std::function make_task() { int value = 42; return [value]() { use(value); }; @@ -106,7 +106,7 @@ std::function 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 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 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 parse_int(const std::string& s) { try { return std::stoi(s); @@ -226,11 +226,11 @@ std::optional 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 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 build(int n) { std::vector out; for (int i = 0; i < n; ++i) { @@ -263,7 +263,7 @@ std::vector build(int n) { return out; } -// ? Good: reserve upfront +// ✅ Good: reserve upfront std::vector build(int n) { std::vector out; out.reserve(static_cast(n)); @@ -277,7 +277,7 @@ std::vector build(int n) { ### String concatenation ```cpp -// ? Bad: repeated allocation +// ❌ Bad: repeated allocation std::string join(const std::vector& parts) { std::string out; for (const auto& p : parts) { @@ -286,7 +286,7 @@ std::string join(const std::vector& parts) { return out; } -// ? Good: reserve total size +// ✅ Good: reserve total size std::string join(const std::vector& parts) { size_t total = 0; for (const auto& p : parts) { @@ -308,13 +308,13 @@ std::string join(const std::vector& parts) { ### Prefer constrained templates (C++20) ```cpp -// ? Bad: overly generic +// ❌ Bad: overly generic template T add(T a, T b) { return a + b; } -// ? Good: constrained +// ✅ Good: constrained template requires std::is_integral_v T add(T a, T b) { diff --git a/reference/csharp.md b/reference/csharp.md index aabff8a..b92a9be 100644 --- a/reference/csharp.md +++ b/reference/csharp.md @@ -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); diff --git a/reference/css-less-sass.md b/reference/css-less-sass.md index 638854a..782c5d9 100644 --- a/reference/css-less-sass.md +++ b/reference/css-less-sass.md @@ -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 { diff --git a/reference/django.md b/reference/django.md index 72a4e9d..2f5995d 100644 --- a/reference/django.md +++ b/reference/django.md @@ -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 = [ diff --git a/reference/java.md b/reference/java.md index 2e4e177..9646d67 100644 --- a/reference/java.md +++ b/reference/java.md @@ -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 cache = new ConcurrentHashMap<>(); ``` diff --git a/reference/kotlin.md b/reference/kotlin.md index 14dd221..94a35b0 100644 --- a/reference/kotlin.md +++ b/reference/kotlin.md @@ -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 diff --git a/reference/nestjs.md b/reference/nestjs.md index 32a070a..04ba6c0 100644 --- a/reference/nestjs.md +++ b/reference/nestjs.md @@ -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(); }); diff --git a/reference/qt.md b/reference/qt.md index 24fef2b..c7b723d 100644 --- a/reference/qt.md +++ b/reference/qt.md @@ -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); } diff --git a/reference/rust.md b/reference/rust.md index 1fa062c..048e7f3 100644 --- a/reference/rust.md +++ b/reference/rust.md @@ -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 diff --git a/reference/security-review-guide.md b/reference/security-review-guide.md index 80d10bc..d79fcde 100644 --- a/reference/security-review-guide.md +++ b/reference/security-review-guide.md @@ -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'); } ``` diff --git a/reference/svelte.md b/reference/svelte.md index 7083bc0..be2d4fb 100644 --- a/reference/svelte.md +++ b/reference/svelte.md @@ -76,15 +76,11 @@ Svelte 5 / SvelteKit 审查重点：Runes 响应式系统、Server/Client 边界 ``` @@ -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} -{#each items as item (item.category, item.id)} +{#each items as item (`${item.category}-${item.id}`)}
{item.name}
{/each} ``` @@ -1006,7 +1002,7 @@ event.cookies.set('session', token, { - [ ] $state 只用于会变化的值，常量直接声明 - [ ] 大型不可变数据使用 $state.raw - [ ] 没有解构 $state 对象（会丢失响应性） -- [ ] 外部库使用 $state.snapshot / unstate 传入普通对象 +- [ ] 外部库使用 $state.snapshot 传入普通对象 - [ ] $derived 中没有副作用 - [ ] 没有用 $effect 替代 $derived 做状态同步 - [ ] $effect 中不修改被追踪的状态（避免无限循环） diff --git a/reference/typescript.md b/reference/typescript.md index 4699f6b..2d724f1 100644 --- a/reference/typescript.md +++ b/reference/typescript.md @@ -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 错误修复 diff --git a/scripts/pr-analyzer.py b/scripts/pr-analyzer.py index 348e48f..0e6d18d 100644 --- a/scripts/pr-analyzer.py +++ b/scripts/pr-analyzer.py @@ -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/ b/" — 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(): diff --git a/scripts/test_pr_analyzer.py b/scripts/test_pr_analyzer.py new file mode 100644 index 0000000..21c6f1a --- /dev/null +++ b/scripts/test_pr_analyzer.py @@ -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()