[duplicate-code] Duplicate Code: Memory allowed-extensions Parsing in cache_config.go and drive_memory_config.go

Đã đóng Phù hợp với người mới
  • Có gì không ổn?
#67,517 3 bình luận 0 reaction 0 người được giao Xem trên GitHub

Maintainer thường phản hồi trong vòng 1 ngày

Chưa có ai nhận issue này.

Đánh giá

Độ khó
2/5
Thời gian dự kiến
1-3 giờ
Mức phù hợp với người mới
66/100
Loại issue
Tái cấu trúc
Độ rõ ràng
Đặc tả rõ ràng
Mức độ hoạt động
Sôi nổi
Công nghệ
go
Lĩnh vực
tooling

Hướng nghiên cứu

Bắt đầu với parseCacheMemoryAllowedExtensions trong pkg/workflow/cache_config.go và parseDriveMemoryAllowedExtensions trong pkg/workflow/drive_memory_config.go, hai hàm chứa vòng lặp bị trùng lặp, rồi đọc isValidFileExtension trong cache_config.go. Tách ra một hàm trợ giúp dùng chung trả về []string và để cả hai bên gọi gán kết quả của nó. Hoàn thành khi cả hai bên gọi đều dùng nó, thông báo lỗi không đổi, và các test của cache_memory và drive_memory pass dưới make test-unit.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Mô tả

automated-analysis code-quality cookie
🔍 Duplicate Code Detected: Memory allowed-extensions Parsing Loop

Analysis of commit 2b5c6d4b8fbf8480ec4c05dbc19fb349aef1f120 (#67506)

Assignee: @copilot

Summary

Two memory-subsystem config parsers — one for cache-memory and one for drive-memory — contain a near-identical loop that reads the allowed-extensions list, validates each entry with isValidFileExtension, and appends it to the entry's AllowedExtensions slice. The bodies differ only by the entry struct type (*CacheMemoryEntry vs *DriveMemoryEntry) and local variable names; the logic, validation, and error message are byte-for-byte identical.

Impact Analysis
  • Maintainability: Any change to extension validation (e.g. allowing a new character class, tightening the error message, or supporting a new config shape) must be made in two places, which is easy to miss.
  • Bug Risk: The two copies can silently drift — a fix applied to one memory type but not the other produces inconsistent validation behaviour for cache-memory vs drive-memory.
  • Code Bloat: ~22 lines are duplicated verbatim; repo-memory already uses a different helper (parseRepoMemoryStringList), so the codebase currently has three divergent approaches to the same concept.
Duplication Details
Pattern: allowed-extensions list parse + per-item validation
  • Severity: Medium
  • Occurrences: 2 (plus a third divergent variant in repo_memory.go)
  • Locations:
    • pkg/workflow/cache_config.go — parseCacheMemoryAllowedExtensions (lines 272–294)
    • pkg/workflow/drive_memory_config.go — parseDriveMemoryAllowedExtensions (lines 148–173)
  • Code Sample (cache_config.go): 
    func parseCacheMemoryAllowedExtensions(cacheMap map[string]any, entry *CacheMemoryEntry) error {
    allowedExts, exists := cacheMap["allowed-extensions"]
    if !exists {
        return nil
    }
    extArray, ok := allowedExts.([]any)
    if !ok {
        return nil
    }
    entry.AllowedExtensions = make([]string, 0, len(extArray))
    for _, ext := range extArray {
        extStr, ok := ext.(string)
        if !ok {
            continue
        }
        if !isValidFileExtension(extStr) {
            return fmt.Errorf("invalid allowed-extension %q: must start with '.' followed by alphanumeric characters only (e.g. .json)", extStr)
        }
        entry.AllowedExtensions = append(entry.AllowedExtensions, extStr)
    }
    return nil
}
  • Code Sample (drive_memory_config.go, functionally identical): 
    func parseDriveMemoryAllowedExtensions(raw map[string]any, entry *DriveMemoryEntry) error {
    value, exists := raw["allowed-extensions"]
    if !exists {
        return nil
    }
    values, ok := value.([]any)
    if !ok {
        return nil
    }
    entry.AllowedExtensions = make([]string, 0, len(values))
    for _, value := range values {
        extension, ok := value.(string)
        if !ok {
            continue
        }
        if !isValidFileExtension(extension) {
            return fmt.Errorf("invalid allowed-extension %q: must start with '.' followed by alphanumeric characters only (e.g. .json)", extension)
        }
        entry.AllowedExtensions = append(entry.AllowedExtensions, extension)
    }
    return nil
}
Refactoring Recommendations

  1. Extract a shared helper that returns the parsed slice

    • Add e.g. parseMemoryAllowedExtensions(configMap map[string]any) ([]string, error) (suggested home: pkg/workflow/memory_validation_config.go, alongside the other shared memory helpers, or cache_config.go since isValidFileExtension already lives there).
    • Have both callers assign the result: exts, err := parseMemoryAllowedExtensions(m); entry.AllowedExtensions = exts.
    • Returning the slice (rather than mutating a typed *Entry) avoids coupling the helper to either struct type and lets repo-memory adopt it too.
    • Estimated effort: ~1 hour (small, mechanical, well covered by existing cache_memory_*_test.go / drive_memory_test.go).
    • Benefits: single source of truth for extension validation and the shared error message; removes drift risk between memory types.

  2. (Optional) Converge repo-memory

    • Evaluate whether parseRepoMemoryStringList for allowed-extensions can route through the same helper so all three memory types validate extensions identically.
Implementation Checklist
  • Review duplication findings
  • Extract shared parseMemoryAllowedExtensions helper
  • Update cache_config.go and drive_memory_config.go to call it
  • (Optional) Converge repo_memory.go onto the helper
  • Run make fmt and make test-unit
  • Verify no functionality broken
Analysis Metadata
  • Analyzed Files: memory-subsystem source files under pkg/workflow/ (cache/drive/repo/comment/validation)
  • Detection Method: Serena semantic code analysis + pattern search
  • Commit: 2b5c6d4b8fbf8480ec4c05dbc19fb349aef1f120
  • Scope note: repository is a single squashed commit, so analysis focused on the commit's stated theme (memory schema contract + safe-output runtime)

Generated by 🔍 Duplicate Code Detector · pi · opus48 · 104.2 AIC · ⌖ 45.4 AIC · ⊞ 1.5K · ◷

  • expires on Oct 12, 2026, 1:56 PM UTC-08:00
Ngôn ngữ chính
Go
Star
5.5k
Fork
606
Merge trung bình
4 giờ 44 phút
Pull request đã merge (30 ngày)
799

Chuẩn bị môi trường

Mở trong Codespaces

Khởi chạy dev container của dự án ngay trên trình duyệt, bằng tài khoản GitHub của bạn.

Bắt đầu từ đâu

  1. Đọc hết issue, rồi đọc hướng dẫn đóng góp của dự án.
  2. Bình luận trên issue rằng bạn sẽ nhận — tránh hai người làm cùng một việc.
  3. Fork repository và làm thay đổi trên một nhánh.
  4. Mở pull request có tham chiếu số hiệu của issue.

Issue khác của github/gh-aw

Tất cả issue của github/gh-aw

Issue tương tự

Thêm issue về Go

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.