Conversation
…iminate regex overhead Replace fmt.Sprintf with string concatenation for JSON path construction, replace regexp with strings.HasPrefix for ref prefix matching, replace fmt.Sprintf with strconv for numeric formatting, pre-allocate maps with capacity hints, thread schema depth as explicit counter, refactor fixer ref collection to pointer-accumulator pattern, and use map clear() to reuse allocations in SchemaHasher. Benchmark results (benchstat, 10 iterations, Apple M4): validator/ValidateParsed/Large: -29% time, -23% memory, -57% allocs validator/ValidateParsed/Medium: -28% time, -23% memory, -54% allocs validator/ValidateParsed/Small: -24% time, -18% memory, -55% allocs schemautil/Hash/Simple: -23% time, -83% memory, -29% allocs schemautil/GroupByHash/1000: -14% time, -56% memory, -14% allocs converter/Parsed/OAS2→3/Medium: -9% time converter/Parsed/OAS3→2/Medium: -6% time Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe pull request refactors the codebase to optimize string construction and simplify control flow. Changes include replacing Changes
Possibly Related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #344 +/- ##
==========================================
+ Coverage 84.72% 84.78% +0.05%
==========================================
Files 193 193
Lines 27273 27249 -24
==========================================
- Hits 23107 23102 -5
+ Misses 2847 2828 -19
Partials 1319 1319
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
validator/refs.go (1)
47-92:⚠️ Potential issue | 🟠 MajorGuard map capacity sums against int overflow.
The summedlen(...)values can overflowintand produce a negative capacity, which will panic inmake(map, ...)for extreme inputs. Please sum inuint64and clamp tomath.MaxInt(or drop the capacity hint).🛡️ Proposed fix with overflow guard
@@ -import ( - "fmt" - "strconv" - "strings" +import ( + "fmt" + "math" + "strconv" + "strings" @@ func buildOAS2ValidRefs(doc *parser.OAS2Document) map[string]bool { - capacity := len(doc.Definitions) + len(doc.Parameters) + len(doc.Responses) + len(doc.SecurityDefinitions) - validRefs := make(map[string]bool, capacity) + var cap64 uint64 + cap64 += uint64(len(doc.Definitions)) + cap64 += uint64(len(doc.Parameters)) + cap64 += uint64(len(doc.Responses)) + cap64 += uint64(len(doc.SecurityDefinitions)) + if cap64 > math.MaxInt { + cap64 = math.MaxInt + } + validRefs := make(map[string]bool, int(cap64)) @@ func buildOAS3ValidRefs(doc *parser.OAS3Document) map[string]bool { if doc.Components == nil { return make(map[string]bool) } - capacity := len(doc.Components.Schemas) + - len(doc.Components.Responses) + - len(doc.Components.Parameters) + - len(doc.Components.Examples) + - len(doc.Components.RequestBodies) + - len(doc.Components.Headers) + - len(doc.Components.SecuritySchemes) + - len(doc.Components.Links) + - len(doc.Components.Callbacks) + - len(doc.Components.PathItems) - validRefs := make(map[string]bool, capacity) + var cap64 uint64 + cap64 += uint64(len(doc.Components.Schemas)) + cap64 += uint64(len(doc.Components.Responses)) + cap64 += uint64(len(doc.Components.Parameters)) + cap64 += uint64(len(doc.Components.Examples)) + cap64 += uint64(len(doc.Components.RequestBodies)) + cap64 += uint64(len(doc.Components.Headers)) + cap64 += uint64(len(doc.Components.SecuritySchemes)) + cap64 += uint64(len(doc.Components.Links)) + cap64 += uint64(len(doc.Components.Callbacks)) + cap64 += uint64(len(doc.Components.PathItems)) + if cap64 > math.MaxInt { + cap64 = math.MaxInt + } + validRefs := make(map[string]bool, int(cap64))As per coding guidelines: Handle size computation overflow (CWE-190) by converting to uint64, checking sum against math.MaxInt, then converting back to int before using as slice capacity.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@validator/refs.go` around lines 47 - 92, The map capacity computation can overflow when summing many len(...) values; update buildOAS2ValidRefs and buildOAS3ValidRefs to compute the sum in uint64, compare against uint64(math.MaxInt), clamp to math.MaxInt if exceeded, and then convert back to int for make(map[string]bool, capacity); alternatively you may omit the capacity hint if clamping is undesirable—apply this uint64-sum-and-clamp pattern to the capacity variables used in both functions (e.g., the local variables named capacity and validRefs).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@validator/refs.go`:
- Around line 47-92: The map capacity computation can overflow when summing many
len(...) values; update buildOAS2ValidRefs and buildOAS3ValidRefs to compute the
sum in uint64, compare against uint64(math.MaxInt), clamp to math.MaxInt if
exceeded, and then convert back to int for make(map[string]bool, capacity);
alternatively you may omit the capacity hint if clamping is undesirable—apply
this uint64-sum-and-clamp pattern to the capacity variables used in both
functions (e.g., the local variables named capacity and validRefs).
|
Re: map capacity overflow guard — Rejected.
The |
Summary
fmt.Sprintfwith string concatenation for JSON path construction across validator, converter, and fixerregexp.MatchString/ReplaceAllStringwithstrings.HasPrefix+ string slicing in converter ref rewritingfmt.Sprintfwithstrconv.Itoa/strconv.FormatFloatfor numeric formatting in validator and schemautilclear()to reuse map allocations in SchemaHasherBenchmark Results
benchstat, 10 iterations, Apple M4:Validator (primary target — hot path optimization)
SchemaUtil (hasher)
Converter
Fixer
Allocation pattern improved (pointer-accumulator), but no measurable time/memory impact — ref collection is a small fraction of overall fix cost.
Optimization Techniques
fmt.Sprintf→ string concatregexp→strings.HasPrefixfmt.Sprintf→strconv%v/%dformat parsing for known typesclear(map)vsmake(map)strings.Count(path, ".")per recursion level*[]stringaccumulatorTest plan
make checkpasses (8498 tests)p=0.000confirming results are not noise🤖 Generated with Claude Code