feat(validator,converter): add $ref validation and conversion support

[erraggy]

Summary of $ref Validation and Conversion Implementation

All tasks have been completed successfully. Here's a comprehensive summary of the work:

Issues Addressed

Issue 1: Validator fails to validate $ref paths properly

• Problem: OAS 3.x documents could have $ref values using OAS 2.0 format (e.g., #/definitions/Foo) without any validation errors
• Solution: Added comprehensive $ref validation to validator/validator.go

Issue 2: Converter fails to update $ref paths when converting between versions

• Problem: When converting OAS 2.0 → 3.x, schemas move from definitions to components.schemas, but refs remained as #/definitions/Foo
• Solution: Added recursive $ref rewriting to converter/helpers.go, converter/oas2_to_oas3.go, and converter/oas3_to_oas2.go

Issue 3: Unit test coverage failed to identify these issues

• Problem: No tests existed for $ref validation or conversion
• Solution: Added comprehensive test coverage for both packages

Implementation Details

Validator Package (validator/validator.go)

Added 600+ lines of validation logic:

New Functions:

• validateRef() - Validates a single $ref points to valid component
• buildOAS2ValidRefs() - Builds map of valid OAS 2.0 reference paths
• buildOAS3ValidRefs() - Builds map of valid OAS 3.x reference paths
• validateSchemaRefs() - Recursively validates all refs in schemas (231 lines)
• validateParameterRef(), validateResponseRef(), validateRequestBodyRef() - Validate refs in these types
• validateOAS2Refs() - Validates all refs in OAS 2.0 document (78 lines)
• validateOAS3Refs() - Validates all refs in OAS 3.x document (168 lines)

Test Coverage (validator/validator_test.go):

• Added TestRefValidation() with 10 comprehensive test cases:
  • OAS 2.0 valid/invalid ref formats
  • OAS 3.0 valid/invalid ref formats
  • Nested schema refs
  • Parameter refs
  • Non-existent component refs

✅ All 10 test cases pass

Converter Package (converter/helpers.go, converter/oas2_to_oas3.go, converter/oas3_to_oas2.go)

Added 350+ lines of ref rewriting logic:

New Functions:

• rewriteRefOAS2ToOAS3() - Rewrites single ref string (2.0 → 3.x format)
• rewriteRefOAS3ToOAS2() - Rewrites single ref string (3.x → 2.0 format)
• rewriteSchemaRefsOAS2ToOAS3() - Recursively rewrites all refs in schema (64 lines)
• rewriteSchemaRefsOAS3ToOAS2() - Recursively rewrites all refs in schema (64 lines)
• rewriteParameterRefsOAS2ToOAS3/OAS3ToOAS2() - Handles parameter refs
• rewriteResponseRefsOAS2ToOAS3/OAS3ToOAS2() - Handles response refs
• rewriteRequestBodyRefsOAS2ToOAS3/OAS3ToOAS2() - Handles request body refs
• rewritePathItemRefsOAS2ToOAS3/OAS3ToOAS2() - Handles path item refs
• rewriteAllRefsOAS2ToOAS3() - Traverses entire OAS 3.x document (49 lines)
• rewriteAllRefsOAS3ToOAS2() - Traverses entire OAS 2.0 document (33 lines)

Reference Format Mappings:

OAS 2.0 → OAS 3.x:

• #/definitions/ → #/components/schemas/
• #/parameters/ → #/components/parameters/
• #/responses/ → #/components/responses/
• #/securityDefinitions/ → #/components/securitySchemes/

Bug Fixes:

• Fixed parameter Ref field not being copied during conversion (in both convertOAS2ParameterToOAS3() and convertOAS3ParameterToOAS2())

Test Coverage (converter/converter_test.go):

• Added TestRefRewritingOAS2ToOAS3() - Verifies basic ref rewriting (2.0 → 3.x)
• Added TestRefRewritingOAS3ToOAS2() - Verifies basic ref rewriting (3.x → 2.0)
• Added TestRefRewritingNestedSchemas() - Verifies nested refs in properties, allOf, anyOf, oneOf
• Added TestRefRewritingParameters() - Verifies parameter refs are rewritten

✅ All 4 test cases pass
✅ All 19 existing converter tests pass

Joiner Package Review

Finding: No changes needed for joiner package

Reason:

• Joiner only joins documents of the same version (enforced validation)
• When joining OAS 3.x documents, refs remain as #/components/schemas/
• When joining OAS 2.0 documents, refs remain as #/definitions/
• Components are directly copied without modification
• Refs remain valid in the merged document

Test Results

Validator Package:

✅ All 23 tests pass
✅ TestRefValidation - 10 test cases all pass

Converter Package:

✅ All 19 tests pass  
✅ TestRefRewritingOAS2ToOAS3 - PASS
✅ TestRefRewritingOAS3ToOAS2 - PASS
✅ TestRefRewritingNestedSchemas - PASS
✅ TestRefRewritingParameters - PASS

Files Modified

1. validator/validator.go - Added 600+ lines of ref validation logic
2. validator/validator_test.go - Added TestRefValidation with 10 test cases
3. converter/helpers.go - Added 350+ lines of ref rewriting logic + parameter fixes
4. converter/oas2_to_oas3.go - Added call to rewriteAllRefsOAS2ToOAS3()
5. converter/oas3_to_oas2.go - Added call to rewriteAllRefsOAS3ToOAS2()
6. converter/converter_test.go - Added 4 new test functions for ref rewriting

Verification

All existing functionality remains intact:

• ✅ All parser tests pass
• ✅ All validator tests pass (including 10 new ref validation tests)
• ✅ All converter tests pass (including 4 new ref rewriting tests)
• ✅ All joiner tests pass (no changes needed)

The implementation successfully addresses all three identified issues with comprehensive validation, conversion, and test coverage.

[claude]

Code Review: $ref Validation and Conversion Support

Summary

This PR successfully addresses three critical issues with $ref handling in OpenAPI specifications:

1. Validator now properly validates $ref paths
2. Converter correctly rewrites $ref paths when converting between OAS versions
3. Comprehensive test coverage ensures these features work correctly

Overall Assessment

Verdict: ✅ APPROVED with minor recommendations

This is high-quality work that significantly improves the reliability of the oastools library. The implementation is thorough, well-tested, and follows Go best practices.

---

Detailed Review

Code Quality: ✅ Excellent

Strengths:

• Clean, readable code with consistent style
• Comprehensive documentation and comments
• Proper error handling throughout
• Good separation of concerns (validation vs. rewriting logic)
• Excellent use of recursion for traversing nested structures
• Package-level regex compilation (performance optimization)

Validator Package (validator/validator.go):

• validateRef() - Clean validation logic with proper path tracking
• buildOAS2ValidRefs() / buildOAS3ValidRefs() - Efficient map-based validation
• validateSchemaRefs() - Thorough recursive validation covering all schema fields including JSON Schema Draft 2020-12 fields (prefixItems, contains, dependentSchemas, if/then/else, $defs)
• validateOAS2Refs() / validateOAS3Refs() - Complete document traversal including webhooks (OAS 3.1+)

Converter Package (converter/helpers.go):

• rewriteRefOAS2ToOAS3() / rewriteRefOAS3ToOAS2() - Clean ref transformation using regex maps
• rewriteSchemaRefsOAS2ToOAS3() / rewriteSchemaRefsOAS3ToOAS2() - Comprehensive recursive rewriting
• rewriteAllRefsOAS2ToOAS3() / rewriteAllRefsOAS3ToOAS2() - Complete document traversal
• Integration points in oas2_to_oas3.go:76 and oas3_to_oas2.go:95 are well-placed

Bug Fix:

• Fixed missing Ref field copy in convertOAS2ParameterToOAS3() and convertOAS3ParameterToOAS2() (converter/helpers.go:167, 216)

---

Test Coverage: ✅ Excellent

Validator Tests (validator/validator_test.go):

• TestRefValidation with 10 comprehensive test cases:
  • OAS 2.0 valid/invalid schema refs
  • OAS 3.0 valid/invalid schema refs
  • Nested schema refs
  • Parameter refs
  • Non-existent component refs
• Good use of table-driven tests
• Tests validate error messages and field values

Converter Tests (converter/converter_test.go):

• TestRefRewritingOAS2ToOAS3 - Basic ref rewriting validation
• TestRefRewritingOAS3ToOAS2 - Bidirectional validation
• TestRefRewritingNestedSchemas - Covers allOf, anyOf, oneOf, nested properties
• TestRefRewritingParameters - Parameter ref handling
• Tests use programmatic document construction (not YAML parsing) for clarity

---

Performance Considerations: ✅ Good

Optimizations:

• ✅ Regex compiled at package level (pathParamRegx, refRegxMapWithOAS3AsNew, refRegxMapWithSwaggerAsNew)
• ✅ Map-based ref validation (O(1) lookups vs O(n) searches)
• ✅ Single-pass document traversal for ref rewriting

Potential Concerns:

• Deep recursion in schema validation/rewriting could impact stack for deeply nested schemas (unlikely in practice)
• No memoization for repeated schema validation (acceptable trade-off for simplicity)
• Multiple regex checks per ref (4 patterns) - mitigated by early return on match

Recommendation: Performance is appropriate for the use case. No changes needed.

---

Security Considerations: ✅ Good

Strengths:

• ✅ Only validates local references (#/ prefix) - external refs handled by parser
• ✅ No code execution or file system access in validation/rewriting
• ✅ Proper nil checking prevents panic conditions
• ✅ No risk of regex DoS (patterns are simple and bounded)

Observations:

• External references (http://, file://, relative paths) are explicitly skipped in validation (validator/validator.go:1946-1948)
• This is correct behavior - external ref resolution is the parser's responsibility

---

Potential Issues and Recommendations

1. Minor: External Reference Handling (Informational)

Location: validator/validator.go:1944-1949

The validator skips external references entirely. This is correct for the current scope, but consider documenting this behavior more prominently.

Recommendation: Consider adding a package-level comment or godoc explaining the division of responsibility between validator and parser for external refs.

Severity: Informational - Documentation enhancement

---

2. Minor: Ref Rewriting for Unknown Formats (Edge Case)

Location: converter/helpers.go:339-340, 357-358

When a ref doesn't match known patterns, it's returned as-is:

// Unknown reference format, return as-is
return ref

Scenarios:

• Custom $ref formats (e.g., #/x-custom/Foo)
• Malformed refs (e.g., #components/schemas/Foo - missing slash)

Current Behavior: Silent pass-through (no error, no rewrite)

Recommendation: Consider logging/tracking unknown ref formats as a warning in ConversionResult. This would help users identify potential issues.

Severity: Low - Edge case that's unlikely in practice

---

3. Minor: Validator Performance for Large Documents (Optimization Opportunity)

Location: validator/validator.go:2052-2175 (validateSchemaRefs)

For very large documents with many schemas, the validator rebuilds validRefs map once and then validates all refs. This is efficient.

However, if a document has circular references (e.g., Pet → Owner → Pet), the recursive validation could revisit the same schemas multiple times.

Recommendation: Consider memoization for already-validated schemas (low priority, as circular refs are typically shallow).

Severity: Low - Performance optimization for edge cases

---

4. Code Duplication: Schema Ref Rewriting (Refactoring Opportunity)

Location: converter/helpers.go:361-425, 427-491

The functions rewriteSchemaRefsOAS2ToOAS3 and rewriteSchemaRefsOAS3ToOAS2 are nearly identical except for the direction of rewriting.

Current Approach: Two separate functions (acceptable)

Recommendation: Current approach is clear and maintainable. Refactoring would reduce duplication but adds abstraction. Not required - this is a style preference.

Severity: Informational - Optional refactoring

---

Testing Recommendations

1. Edge Case: Circular References

Consider adding a test for circular $ref chains - circular refs are valid in OpenAPI and should validate/convert correctly.

2. Edge Case: Mixed External and Local Refs

Test document with both local and external refs to ensure validator skips external refs and converter preserves them.

3. Robustness: Malformed Refs

Test handling of malformed refs (missing slashes, empty refs, etc.) to ensure clear error messages.

---

Alignment with CLAUDE.md Guidelines

✅ Follows project conventions:

• Uses internal/httputil constants appropriately
• Proper package-level documentation
• Follows testing strategy (unit tests alongside implementation)
• Adheres to Go best practices

✅ Architecture compliance:

• Changes are in correct packages (validator, converter)
• No cross-package violations
• Proper separation of concerns

✅ Documentation:

• Functions have clear comments
• Test names are descriptive
• Code is self-documenting

---

Final Recommendations

Must Do:

• None - code is production-ready as-is

Should Consider:

1. Add test for circular $ref chains (validation + conversion)
2. Document external ref handling in package-level docs
3. Consider tracking unknown ref formats as warnings (low priority)

Nice to Have:

1. Add test for mixed local/external refs
2. Add test for malformed refs
3. Consider generic schema rewriting function (refactoring)

---

Conclusion

This is excellent work that significantly improves the oastools library. The implementation is:

• ✅ Correct and thorough
• ✅ Well-tested with comprehensive coverage
• ✅ Performant with good optimization choices
• ✅ Secure with no identified vulnerabilities
• ✅ Maintainable with clear, documented code

The PR is approved and ready to merge. The recommendations above are optional enhancements that could be addressed in future iterations.

Great job! 🎉