Merge pull request #859 from tdakkota/fix/unused-query-decoder

fix(gen): define form decoder variable only if necessary
ogen-go · Apr 12, 2023 · 847dd1e · 847dd1e
2 parents ba39646 + 76d775d
commit 847dd1e
Show file tree

Hide file tree

Showing 16 changed files with 1,271 additions and 30 deletions.
diff --git a/_testdata/positive/http_requests.json b/_testdata/positive/http_requests.json
@@ -218,6 +218,91 @@
           }
         }
       }
+    },
+    "/onlyForm": {
+      "post": {
+        "operationId": "onlyForm",
+        "requestBody": {
+          "required": true,
+          "content": {
+            "application/x-www-form-urlencoded": {
+              "schema": {
+                "type": "object",
+                "required": [
+                  "field"
+                ],
+                "properties": {
+                  "field": {
+                    "type": "integer"
+                  }
+                }
+              }
+            }
+          }
+        },
+        "responses": {
+          "200": {
+            "description": "Ok"
+          }
+        }
+      }
+    },
+    "/onlyMultipartForm": {
+      "post": {
+        "operationId": "onlyMultipartForm",
+        "requestBody": {
+          "required": true,
+          "content": {
+            "multipart/form-data": {
+              "schema": {
+                "type": "object",
+                "required": [
+                  "field"
+                ],
+                "properties": {
+                  "field": {
+                    "type": "integer"
+                  }
+                }
+              }
+            }
+          }
+        },
+        "responses": {
+          "200": {
+            "description": "Ok"
+          }
+        }
+      }
+    },
+    "/onlyMultipartFile": {
+      "post": {
+        "operationId": "onlyMultipartFile",
+        "requestBody": {
+          "required": true,
+          "content": {
+            "multipart/form-data": {
+              "schema": {
+                "type": "object",
+                "required": [
+                  "file"
+                ],
+                "properties": {
+                  "file": {
+                    "type": "string",
+                    "format": "binary"
+                  }
+                }
+              }
+            }
+          }
+        },
+        "responses": {
+          "200": {
+            "description": "Ok"
+          }
+        }
+      }
     }
   },
   "components": {
@@ -253,4 +338,4 @@
       }
     }
   }
-}
+}
diff --git a/examples/ex_2ch/oas_request_decoders_gen.go b/examples/ex_2ch/oas_request_decoders_gen.go
diff --git a/gen/_template/request_decode.tmpl b/gen/_template/request_decode.tmpl
@@ -145,7 +145,9 @@ func (s *{{ if $op.WebhookInfo }}Webhook{{ end }}Server) decode{{ $op.Name }}Req
 		// Notice that the closers are called in reverse order, to match defer behavior, so
 		// any opened file will be closed before RemoveAll call.
 		closers = append(closers, r.MultipartForm.RemoveAll)
+		// Form values may be unused.
 		form := url.Values(r.MultipartForm.Value)
+		_ = form
         {{- end }}
 
 		var request {{ $t.Go }}
@@ -192,36 +194,38 @@ func (s *{{ if $op.WebhookInfo }}Webhook{{ end }}Server) decode{{ $op.Name }}Req
 		}
 	}
 {{- else if $t.IsStruct }}
+	{{- with $t.FormParameters }}
 	q := uri.NewQueryDecoder(form)
-	{{- range $p := $t.FormParameters }}
-	{
-		{{- $el := elem $p.Type (printf "%s.%s" $.Var $p.Name) }}
-		cfg := uri.QueryParameterDecodingConfig{
-			Name:    {{ quote $p.Spec.Name }},
-			Style:   uri.QueryStyle{{ capitalize $p.Spec.Style.String }},
-			Explode: {{ if $p.Spec.Explode }}true{{ else }}false{{ end }},
-			{{- if isObjectParam $p }}
-			Fields: {{ paramObjectFields $p.Type }},
-			{{- end }}
-		}
-		if err := q.HasParam(cfg); err == nil {
-			if err := q.DecodeParam(cfg, func(d uri.Decoder) error {
-				{{- template "uri/decode" $el }}
-			}); err != nil {
-				return req, close, errors.Wrap(err, {{ printf "decode %q" $p.Spec.Name | quote }})
+		{{- range $p := $t.FormParameters }}
+		{
+			{{- $el := elem $p.Type (printf "%s.%s" $.Var $p.Name) }}
+			cfg := uri.QueryParameterDecodingConfig{
+				Name:    {{ quote $p.Spec.Name }},
+				Style:   uri.QueryStyle{{ capitalize $p.Spec.Style.String }},
+				Explode: {{ if $p.Spec.Explode }}true{{ else }}false{{ end }},
+				{{- if isObjectParam $p }}
+				Fields: {{ paramObjectFields $p.Type }},
+				{{- end }}
 			}
+			if err := q.HasParam(cfg); err == nil {
+				if err := q.DecodeParam(cfg, func(d uri.Decoder) error {
+					{{- template "uri/decode" $el }}
+				}); err != nil {
+					return req, close, errors.Wrap(err, {{ printf "decode %q" $p.Spec.Name | quote }})
+				}
 
-			{{- if $p.Type.NeedValidation }}
-			if err := func() error {
-				{{- template "validate" $el }}
-			}(); err != nil {
-				return req, close, errors.Wrap(err, "validate")
-			}
-			{{- end }}
-		} {{- if $p.Spec.Required }} else {
-			return req, close, errors.Wrap(err, "query")
-		} {{- end }}
-	}
+				{{- if $p.Type.NeedValidation }}
+				if err := func() error {
+					{{- template "validate" $el }}
+				}(); err != nil {
+					return req, close, errors.Wrap(err, "validate")
+				}
+				{{- end }}
+			} {{- if $p.Spec.Required }} else {
+				return req, close, errors.Wrap(err, "query")
+			} {{- end }}
+		}
+		{{- end }}
 	{{- end }}
     {{- range $p := $t.FileParameters }}
 	{

diff --git a/internal/integration/requests_test.go b/internal/integration/requests_test.go
@@ -13,11 +13,11 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	ht "github.com/ogen-go/ogen/http"
 	api "github.com/ogen-go/ogen/internal/integration/test_http_requests"
 )
 
-type testHTTPRequests struct {
-}
+type testHTTPRequests struct{}
 
 func (t testHTTPRequests) Base64Request(ctx context.Context, req api.Base64RequestReq) (api.Base64RequestOK, error) {
 	return api.Base64RequestOK{
@@ -101,6 +101,18 @@ func (t testHTTPRequests) MaskContentTypeOptional(ctx context.Context, req *api.
 	}, nil
 }
 
+func (t testHTTPRequests) OnlyForm(ctx context.Context, req *api.OnlyFormReq) error {
+	return ht.ErrNotImplemented
+}
+
+func (t testHTTPRequests) OnlyMultipartFile(ctx context.Context, req *api.OnlyMultipartFileReqForm) error {
+	return ht.ErrNotImplemented
+}
+
+func (t testHTTPRequests) OnlyMultipartForm(ctx context.Context, req *api.OnlyMultipartFormReq) error {
+	return ht.ErrNotImplemented
+}
+
 func (t testHTTPRequests) StreamJSON(ctx context.Context, req []float64) (v float64, _ error) {
 	for _, f := range req {
 		v += f

diff --git a/internal/integration/test_allof/oas_request_decoders_gen.go b/internal/integration/test_allof/oas_request_decoders_gen.go
diff --git a/internal/integration/test_form/oas_request_decoders_gen.go b/internal/integration/test_form/oas_request_decoders_gen.go