Fix null type in openapi31 parameter schemas for non-pointer types

[akfaiz]

Problem

When a Go struct field is a slice (e.g. []int) or a map (e.g. map[string]int) used as a query/path/header/cookie parameter, the OpenAPI 3.1 reflector generates a schema with type: ["array","null"] or type: ["object","null"]:

parameters:
  - name: ids
    in: query
    required: true
    schema:
      items:
        type: integer
      type:
        - array
        - "null"   # ← incorrect for HTTP parameters

HTTP parameters cannot be null — they can only be present or absent. Allowing null in the schema is semantically incorrect and conflicts with the required: true constraint.

---

Root Cause

The jsonschema-go library correctly marks Go slices and maps as nullable in checkNullability(), because Go slices/maps can be nil. This is valid for JSON body schemas.

However, the openapi3 reflector already strips this for parameter schemas:

// openapi3/reflect.go
s.FromJSONSchema(propertySchema.ToSchemaOrBool())
if s.Schema != nil && s.Schema.Nullable != nil && field.Type.Kind() != reflect.Ptr {
    s.Schema.Nullable = nil  // strips nullable for non-pointer param fields
}

The openapi31 reflector was never updated with the equivalent logic. In OAS 3.1, nullable: true is replaced by type: ["X","null"] (per JSON Schema), so the stripping must use RemoveType instead.

---

Fix

Added the equivalent null-removal logic to openapi31/reflect.go:

// mirrors openapi3 behavior: s.Schema != nil && s.Schema.Nullable != nil && field.Type.Kind() != reflect.Ptr
if propertySchema.Ref == nil && field.Type.Kind() != reflect.Ptr {
    propertySchema.RemoveType(jsonschema.Null)
}

---

Before / After

Before:

parameters:
  - name: ids
    in: query
    schema:
      items:
        type: integer
      type:
        - array
        - "null"

After:

parameters:
  - name: ids
    in: query
    schema:
      items:
        type: integer
      type: array

Response body schemas are unaffected — slices/maps in response bodies still correctly generate type: ["array","null"].

---

Testing

• Updated openapi31/testdata/openapi.json — 4 array query params and 2 map query params now show non-nullable types
• All existing tests pass: go test ./...

[llamapreview]

AI Code Review by LlamaPReview

🎯 TL;DR & Recommendation

Recommendation: Approve with suggestions.
This PR fixes a semantic inconsistency in OpenAPI 3.1 parameter schemas by removing the incorrect 'null' type for non-pointer fields, aligning with OpenAPI 3 behavior.

📄 Documentation Diagram

This diagram documents the null type removal process in OpenAPI 3.1 parameter schemas.

sequenceDiagram
    participant GS as Go Struct
    participant R as Reflector
    participant PS as Parameter Schema
    participant O as OpenAPI Output
    GS->>R: Request schema for parameter
    R->>PS: Generate schema with possible null type
    note over PS: PR #35;158: Remove null type for non-pointer fields
    PS->>PS: Check if field is non-pointer and not ref
    PS->>PS: Remove jsonschema.Null if present
    R->>O: Emit schema without null type

Loading

🌟 Strengths

• Correctly addresses a known bug where HTTP parameters were incorrectly marked as nullable.
• Well-documented with clear before/after examples and testing updates.

💡 Suggestions (P2)

• openapi31/reflect.go: The broader condition could lead to unnecessary performance overhead by calling RemoveType even when the null type is absent.
• openapi31/reflect.go: Missing targeted unit tests risk undetected regressions in the parameter null-removal logic.

---

💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.

[llamapreview]

P2 | Confidence: High

Speculative: The fix correctly removes the "null" type from the OpenAPI 3.1 schema for non-pointer parameter fields, aligning with OpenAPI 3 behavior. However, the condition propertySchema.Ref == nil && field.Type.Kind() != reflect.Ptr is slightly broader than the OpenAPI 3 version, which also checks s.Schema.Nullable != nil. The current logic will call RemoveType(jsonschema.Null) on every non-pointer, non-ref field, even if the schema's type array doesn't actually contain jsonschema.Null. While the RemoveType method is likely idempotent, this represents a conceptual mismatch and a minor, unnecessary performance overhead for many fields. The more precise condition would check if the type array includes jsonschema.Null before attempting removal.

Code Suggestion:

if propertySchema.Ref == nil && field.Type.Kind() != reflect.Ptr && propertySchema.Type.Includes(jsonschema.Null) {
    propertySchema.RemoveType(jsonschema.Null)
}

Evidence: symbol:jsonschema.Null, method:RemoveType, path:openapi3/reflect.go

[llamapreview]

[Contextual Comment]
This comment refers to code near real line 426. Anchored to nearest_changed(432) line 432.

---

P2 | Confidence: Medium

Speculative: The fix is applied within a callback function triggered by internal.ReflectParametersIn. This function's logic and the InterceptPropParams structure are shared with the OpenAPI 3 reflector. While the provided test updates confirm the fix works for the specific test cases, there is no new unit test directly validating the logic of the null-removal condition itself (e.g., for pointer vs. non-pointer fields, or for referenced schemas). Adding a targeted unit test would ensure the behavior is explicitly documented, protected against future regressions, and clarifies the expected interaction between the jsonschema-go library's nullability and the OpenAPI reflector's parameter-specific logic.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 39.85%. Comparing base (b19848c) to head (de83e33).
⚠️ Report is 17 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #158      +/-   ##
==========================================
- Coverage   40.72%   39.85%   -0.88%     
==========================================
  Files          16       16              
  Lines        6607     8122    +1515     
==========================================
+ Hits         2691     3237     +546     
- Misses       3440     4370     +930     
- Partials      476      515      +39     

Flag	Coverage Δ
unittests	39.85% <100.00%> (-0.88%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[vearutop]

Looks reasonable, thank you for contribution.