fix(gin): address more code review feedback from PR #13

- Add lazy initialization in gin.Extractor to handle zero-value usage
- Fix nil-deref in schema_extractor when SchemaRef.Value is nil for custom types
- Fix binding tag required check to handle multiple comma-separated rules
- Add proper support for ShouldBindQuery (BindingSourceQuery)
- Separate FormParams and FileParams from QueryParams
- Add BindingSource type to track binding source (json, xml, yaml, query, form, multipart)
- Update generator to handle form/file uploads with correct content types
- Update tests to check new FormParams and FileParams fields

Signed-off-by: spencercjh <spencercjh@gmail.com>

Author: spencercjh

internal/extractor/gin/generator.go

@@ -224,25 +224,8 @@ func (g *Generator) buildOperation(route *Route, handlerInfo *HandlerInfo, schem
 		})
 	}
 
-	// Request body
-	if handlerInfo.BodyType != "" && handlerInfo.BodyType != ginHType {
-		var schemaRef *openapi3.SchemaRef
-		if _, exists := schemas[handlerInfo.BodyType]; exists {
-			schemaRef = &openapi3.SchemaRef{Ref: "#/components/schemas/" + handlerInfo.BodyType}
-		} else {
-			// Fallback to generic object if schema not found
-			schemaRef = &openapi3.SchemaRef{Value: &openapi3.Schema{Type: &openapi3.Types{"object"}}}
-		}
-		operation.RequestBody = &openapi3.RequestBodyRef{
-			Value: &openapi3.RequestBody{
-				Content: openapi3.Content{
-					"application/json": {
-						Schema: schemaRef,
-					},
-				},
-			},
-		}
-	}
+	// Handle request body / form parameters based on binding source
+	g.buildRequestBody(operation, handlerInfo, schemas)
 
 	// Responses
 	operation.Responses = openapi3.NewResponses()
@@ -289,6 +272,89 @@ func (g *Generator) buildOperation(route *Route, handlerInfo *HandlerInfo, schem
 	return operation
 }
 
+// buildRequestBody builds the request body or form parameters based on binding source.
+func (g *Generator) buildRequestBody(operation *openapi3.Operation, handlerInfo *HandlerInfo, schemas openapi3.Schemas) {
+	// Handle form parameters (application/x-www-form-urlencoded)
+	if len(handlerInfo.FormParams) > 0 {
+		for _, param := range handlerInfo.FormParams {
+			operation.Parameters = append(operation.Parameters, &openapi3.ParameterRef{
+				Value: &openapi3.Parameter{
+					Name:        param.Name,
+					In:          "query",
+					Required:    param.Required,
+					Description: "Form parameter",
+					Schema:      &openapi3.SchemaRef{Value: &openapi3.Schema{Type: &openapi3.Types{"string"}}},
+				},
+			})
+		}
+	}
+
+	// Handle file upload parameters (multipart/form-data)
+	if len(handlerInfo.FileParams) > 0 {
+		content := make(openapi3.Content)
+		schema := &openapi3.Schema{
+			Type:       &openapi3.Types{"object"},
+			Properties: make(openapi3.Schemas),
+		}
+		for _, param := range handlerInfo.FileParams {
+			schema.Properties[param.Name] = &openapi3.SchemaRef{
+				Value: &openapi3.Schema{
+					Type:   &openapi3.Types{"string"},
+					Format: "binary",
+				},
+			}
+			if param.Required {
+				schema.Required = append(schema.Required, param.Name)
+			}
+		}
+		content["multipart/form-data"] = &openapi3.MediaType{Schema: &openapi3.SchemaRef{Value: schema}}
+		operation.RequestBody = &openapi3.RequestBodyRef{
+			Value: &openapi3.RequestBody{
+				Required: true,
+				Content:  content,
+			},
+		}
+		return
+	}
+
+	// Handle body binding based on source
+	if handlerInfo.BodyType == "" || handlerInfo.BodyType == ginHType {
+		return
+	}
+
+	var schemaRef *openapi3.SchemaRef
+	if _, exists := schemas[handlerInfo.BodyType]; exists {
+		schemaRef = &openapi3.SchemaRef{Ref: "#/components/schemas/" + handlerInfo.BodyType}
+	} else {
+		// Fallback to generic object if schema not found
+		schemaRef = &openapi3.SchemaRef{Value: &openapi3.Schema{Type: &openapi3.Types{"object"}}}
+	}
+
+	// Determine content type based on binding source
+	contentType := "application/json"
+	switch handlerInfo.BindingSrc {
+	case BindingSourceXML:
+		contentType = "application/xml"
+	case BindingSourceYAML:
+		contentType = "application/x-yaml"
+	case BindingSourceQuery:
+		// Query binding - don't create request body, parameters are in query
+		return
+	case BindingSourceForm:
+		contentType = "application/x-www-form-urlencoded"
+	}
+
+	operation.RequestBody = &openapi3.RequestBodyRef{
+		Value: &openapi3.RequestBody{
+			Content: openapi3.Content{
+				contentType: {
+					Schema: schemaRef,
+				},
+			},
+		},
+	}
+}
+
 // setOperationForMethod sets the operation for the given HTTP method.
 func setOperationForMethod(pathItem *openapi3.PathItem, method string, operation *openapi3.Operation) {
 	switch method {

internal/extractor/gin/gin.go

@@ -20,6 +20,20 @@ type Extractor struct {
 	generator *Generator
 }
 
+// ensureInitialized lazily initializes the extractor fields if needed.
+// This handles the case where the extractor is used as a zero-value.
+func (e *Extractor) ensureInitialized() {
+	if e.detector == nil {
+		e.detector = NewDetector()
+	}
+	if e.patcher == nil {
+		e.patcher = NewPatcher()
+	}
+	if e.generator == nil {
+		e.generator = NewGenerator()
+	}
+}
+
 // NewExtractor creates a new Extractor instance.
 func NewExtractor() *Extractor {
 	return &Extractor{
@@ -36,11 +50,13 @@ func (e *Extractor) Name() string {
 
 // Detect implements extractor.Extractor.Detect.
 func (e *Extractor) Detect(projectPath string) (*extractor.ProjectInfo, error) {
+	e.ensureInitialized()
 	return e.detector.Detect(projectPath)
 }
 
 // Patch implements extractor.Extractor.Patch.
 func (e *Extractor) Patch(projectPath string, opts *extractor.PatchOptions) (*extractor.PatchResult, error) {
+	e.ensureInitialized()
 	// Gin projects don't need patching
 	info := &extractor.ProjectInfo{
 		Framework:     ExtractorName,
@@ -52,6 +68,7 @@ func (e *Extractor) Patch(projectPath string, opts *extractor.PatchOptions) (*ex
 
 // Generate implements extractor.Extractor.Generate.
 func (e *Extractor) Generate(ctx context.Context, projectPath string, info *extractor.ProjectInfo, opts *extractor.GenerateOptions) (*extractor.GenerateResult, error) {
+	e.ensureInitialized()
 	return e.generator.Generate(ctx, projectPath, info, opts)
 }
 

internal/extractor/gin/handler_analyzer.go

@@ -31,6 +31,8 @@ func (a *HandlerAnalyzer) AnalyzeHandler(fn *ast.FuncDecl) (*HandlerInfo, error)
 		PathParams:   []ParamInfo{},
 		QueryParams:  []ParamInfo{},
 		HeaderParams: []ParamInfo{},
+		FormParams:   []ParamInfo{},
+		FileParams:   []ParamInfo{},
 		Responses:    []ResponseInfo{},
 	}
 
@@ -184,13 +186,14 @@ func (a *HandlerAnalyzer) parseHandlerCall(call *ast.CallExpr, info *HandlerInfo
 	case "GetHeader":
 		a.extractHeaderParam(call, info)
 	case "ShouldBindJSON", "BindJSON", "ShouldBind", "Bind":
-		a.extractBodyType(call, info, varTypeMap)
+		a.extractBodyType(call, info, varTypeMap, BindingSourceJSON)
 	case "ShouldBindXML", "BindXML":
-		a.extractBodyType(call, info, varTypeMap)
+		a.extractBodyType(call, info, varTypeMap, BindingSourceXML)
 	case "ShouldBindYAML", "BindYAML":
-		a.extractBodyType(call, info, varTypeMap)
+		a.extractBodyType(call, info, varTypeMap, BindingSourceYAML)
 	case "ShouldBindQuery", "BindQuery":
-		a.extractBodyType(call, info, varTypeMap)
+		// Query binding extracts query parameters, not body
+		a.extractQueryBinding(call, info, varTypeMap)
 	case "PostForm":
 		a.extractFormParam(call, info, true)
 	case "DefaultPostForm":
@@ -251,13 +254,27 @@ func (a *HandlerAnalyzer) extractHeaderParam(call *ast.CallExpr, info *HandlerIn
 }
 
 // extractBodyType extracts body type from binding calls like c.ShouldBindJSON().
-func (a *HandlerAnalyzer) extractBodyType(call *ast.CallExpr, info *HandlerInfo, varTypeMap map[string]string) {
+func (a *HandlerAnalyzer) extractBodyType(call *ast.CallExpr, info *HandlerInfo, varTypeMap map[string]string, source BindingSource) {
 	if len(call.Args) < 1 {
 		return
 	}
 	if typeName := extractTypeFromArg(call.Args[0], varTypeMap); typeName != "" {
 		info.BodyType = typeName
-		slog.Debug("Extracted body type", "type", typeName)
+		info.BindingSrc = source
+		slog.Debug("Extracted body type", "type", typeName, "source", source)
+	}
+}
+
+// extractQueryBinding handles ShouldBindQuery by extracting query parameters from struct tags.
+// For now, we track that this is a query binding so the generator can handle it properly.
+func (a *HandlerAnalyzer) extractQueryBinding(call *ast.CallExpr, info *HandlerInfo, varTypeMap map[string]string) {
+	if len(call.Args) < 1 {
+		return
+	}
+	if typeName := extractTypeFromArg(call.Args[0], varTypeMap); typeName != "" {
+		info.BodyType = typeName
+		info.BindingSrc = BindingSourceQuery
+		slog.Debug("Extracted query binding type", "type", typeName)
 	}
 }
 
@@ -267,7 +284,7 @@ func (a *HandlerAnalyzer) extractFormParam(call *ast.CallExpr, info *HandlerInfo
 		return
 	}
 	if name := extractStringLiteral(call.Args[0]); name != "" {
-		info.QueryParams = append(info.QueryParams, ParamInfo{
+		info.FormParams = append(info.FormParams, ParamInfo{
 			Name:     name,
 			GoType:   "string",
 			Required: required,
@@ -281,7 +298,7 @@ func (a *HandlerAnalyzer) extractFileParam(call *ast.CallExpr, info *HandlerInfo
 		return
 	}
 	if name := extractStringLiteral(call.Args[0]); name != "" {
-		info.QueryParams = append(info.QueryParams, ParamInfo{
+		info.FileParams = append(info.FileParams, ParamInfo{
 			Name:     name,
 			GoType:   "file",
 			Required: true,

internal/extractor/gin/handler_analyzer_more_test.go

@@ -130,25 +130,29 @@ func uploadFile(c *gin.Context) {
 
 	info, _ := analyzer.AnalyzeHandler(handlerDecl)
 
-	// Check for form parameters
+	// Check for file parameter in FileParams
 	foundFile := false
-	foundUserID := false
-	for _, param := range info.QueryParams {
+	for _, param := range info.FileParams {
 		if param.Name == "file" {
 			foundFile = true
 			if param.GoType != "file" {
 				t.Errorf("expected file type for 'file', got '%s'", param.GoType)
 			}
 		}
+	}
+	if !foundFile {
+		t.Error("expected 'file' parameter in FileParams")
+	}
+
+	// Check for form parameter in FormParams
+	foundUserID := false
+	for _, param := range info.FormParams {
 		if param.Name == "userId" {
 			foundUserID = true
 		}
 	}
-	if !foundFile {
-		t.Error("expected 'file' parameter")
-	}
 	if !foundUserID {
-		t.Error("expected 'userId' parameter")
+		t.Error("expected 'userId' parameter in FormParams")
 	}
 }
 

internal/extractor/gin/schema_extractor.go

@@ -106,7 +106,10 @@ func (e *SchemaExtractor) extractStructSchema(typeSpec *ast.TypeSpec) (*openapi3
 		// Parse struct tags
 		if field.Tag != nil {
 			tag := strings.Trim(field.Tag.Value, "`")
-			propertyName = e.applyTags(fieldSchemaRef.Value, tag, fieldName, schema)
+			// Guard against nil Value when field is a $ref to custom type
+			if fieldSchemaRef.Value != nil {
+				propertyName = e.applyTags(fieldSchemaRef.Value, tag, fieldName, schema)
+			}
 		}
 
 		// Skip fields with json:"-"
@@ -260,9 +263,9 @@ func (e *SchemaExtractor) applyTags(schema *openapi3.Schema, tag, fieldName stri
 		return propertyName
 	}
 
-	// Parse binding tag
+	// Parse binding tag (handle multiple comma-separated rules like "required,email")
 	if bindingTag := extractTagValue(tag, "binding"); bindingTag != "" {
-		if bindingTag == "required" {
+		if strings.Contains(bindingTag, "required") {
 			parentSchema.Required = append(parentSchema.Required, propertyName)
 		}
 	}

internal/extractor/gin/types.go

@@ -28,12 +28,27 @@ type Route struct {
 	Middlewares []string // Middleware names
 }
 
+// BindingSource indicates the source of request binding.
+type BindingSource string
+
+const (
+	BindingSourceJSON      BindingSource = "json"      // application/json
+	BindingSourceXML       BindingSource = "xml"       // application/xml
+	BindingSourceYAML      BindingSource = "yaml"      // application/x-yaml
+	BindingSourceQuery     BindingSource = "query"     // query parameters
+	BindingSourceForm      BindingSource = "form"      // application/x-www-form-urlencoded
+	BindingSourceMultipart BindingSource = "multipart" // multipart/form-data
+)
+
 // HandlerInfo contains information extracted from a handler function.
 type HandlerInfo struct {
 	PathParams   []ParamInfo    // Path parameters (c.Param)
 	QueryParams  []ParamInfo    // Query parameters (c.Query)
 	HeaderParams []ParamInfo    // Header parameters (c.GetHeader)
+	FormParams   []ParamInfo    // Form parameters (c.PostForm)
+	FileParams   []ParamInfo    // File upload parameters (c.FormFile)
 	BodyType     string         // Request body type (from ShouldBindJSON)
+	BindingSrc   BindingSource  // Source of binding (json, xml, query, form, etc.)
 	Responses    []ResponseInfo // Response info (from c.JSON calls)
 }