-
Notifications
You must be signed in to change notification settings - Fork 15
Open
Description
Part of duplicate code analysis: #1689
Summary
internal/config/guard_policy.go contains two private functions, isValidRepoOwner and isValidRepoName, that are structurally identical. They perform the same byte-by-byte character validation loop using isScopeTokenChar, differ only in the parameter name and the maximum-length bound (39 vs 100), and share no common helper.
Duplication Details
Pattern: Identical validation loop, differing only in max-length
- Severity: Low
- Occurrences: 2 functions, ~12 lines each (≈12 lines structural duplication)
- Location:
internal/config/guard_policy.golines 259–294
Duplicated code:
// lines 259-273
func isValidRepoOwner(owner string) bool {
if len(owner) < 1 || len(owner) > 39 { // <-- only difference
return false
}
for i := 0; i < len(owner); i++ {
char := owner[i]
if isScopeTokenChar(char) {
continue
}
return false
}
return true
}
// lines 275-289
func isValidRepoName(repo string) bool {
if len(repo) < 1 || len(repo) > 100 { // <-- only difference
return false
}
for i := 0; i < len(repo); i++ {
char := repo[i]
if isScopeTokenChar(char) {
continue
}
return false
}
return true
}Impact Analysis
- Maintainability: Any future change to the character-set logic (e.g., adding uppercase support) must be applied to both functions
- Bug Risk: Low today; medium if functions diverge silently after a copy-paste edit
- Code Bloat: ~12 lines
Refactoring Recommendations
- Extract a shared private helper parameterised by max length:
// isValidTokenString returns true if s is a non-empty string of at most maxLen
// lowercase-alphanumeric, underscore, or hyphen characters.
func isValidTokenString(s string, maxLen int) bool {
if len(s) < 1 || len(s) > maxLen {
return false
}
for i := 0; i < len(s); i++ {
if !isScopeTokenChar(s[i]) {
return false
}
}
return true
}
func isValidRepoOwner(owner string) bool { return isValidTokenString(owner, 39) }
func isValidRepoName(repo string) bool { return isValidTokenString(repo, 100) }- Estimated effort: < 15 minutes
- Benefits: Single validation loop; simpler to extend (e.g., add
.to allowed chars for repo names)
Implementation Checklist
- Add
isValidTokenString(s string, maxLen int) boolhelper toguard_policy.go - Replace the bodies of
isValidRepoOwnerandisValidRepoNamewith one-liner delegations - Verify
make testpasses
Parent Issue
See parent analysis report: #1689
Related to #1689
Generated by Duplicate Code Detector
- expires on Mar 16, 2026, 3:02 AM UTC
Reactions are currently unavailable