Repository: go-swagger/go-swagger
Pull request #2155 (2019-12-27 16:26:00)

fixed some edge cases when deconflicting client timeout parameters
* param "time-out" is detected as a conflict
* adopts a more agressive, deterministic, deconflicting when all baked proposals are already taken
* added unit test, which was missing on this func

Signed-off-by: Frederic BIDON <fredbi@yahoo.com>


Patch:
diff --git a/generator/operation.go b/generator/operation.go
index e1656c9e..04e6ed1b 100644
--- a/generator/operation.go
+++ b/generator/operation.go
@@ -289,14 +289,24 @@ type codeGenOpBuilder struct {
 
 // renameTimeout renames the variable in use by client template to avoid conflicting
 // with param names.
-func renameTimeout(seenIds map[string][]string, current string) string {
+//
+// NOTE: this merely protects the timeout field in the client parameter struct,
+// fields "Context" and "HTTPClient" remain exposed to name conflicts.
+func renameTimeout(seenIds map[string]bool, timeoutName string) string {
+	if seenIds == nil {
+		return timeoutName
+	}
+	current := strings.ToLower(timeoutName)
+	if _, ok := seenIds[current]; !ok {
+		return timeoutName
+	}
 	var next string
-	switch strings.ToLower(current) {
+	switch current {
 	case "timeout":
 		next = "requestTimeout"
 	case "requesttimeout":
 		next = "httpRequestTimeout"
-	case "httptrequesttimeout":
+	case "httprequesttimeout":
 		next = "swaggerTimeout"
 	case "swaggertimeout":
 		next = "operationTimeout"
@@ -304,11 +314,10 @@ func renameTimeout(seenIds map[string][]string, current string) string {
 		next = "opTimeout"
 	case "optimeout":
 		next = "operTimeout"
+	default:
+		next = timeoutName + "1"
 	}
-	if _, ok := seenIds[next]; ok {
-		return renameTimeout(seenIds, next)
-	}
-	return next
+	return renameTimeout(seenIds, next)
 }
 
 func (b *codeGenOpBuilder) MakeOperation() (GenOperation, error) {
@@ -331,7 +340,6 @@ func (b *codeGenOpBuilder) MakeOperation() (GenOperation, error) {
 	var params, qp, pp, hp, fp GenParameters
 	var hasQueryParams, hasPathParams, hasHeaderParams, hasFormParams, hasFileParams, hasFormValueParams, hasBodyParams bool
 	paramsForOperation := b.Analyzed.ParamsFor(b.Method, b.Path)
-	timeoutName := "timeout"
 
 	idMapping := map[string]map[string]string{
 		"query":    make(map[string]string, len(paramsForOperation)),
@@ -341,18 +349,16 @@ func (b *codeGenOpBuilder) MakeOperation() (GenOperation, error) {
 		"body":     make(map[string]string, len(paramsForOperation)),
 	}
 
-	seenIds := make(map[string][]string, len(paramsForOperation))
+	seenIds := make(map[string]bool, len(paramsForOperation))
 	for id, p := range paramsForOperation {
 		if _, ok := seenIds[p.Name]; ok {
 			idMapping[p.In][p.Name] = swag.ToGoName(id)
 		} else {
 			idMapping[p.In][p.Name] = swag.ToGoName(p.Name)
 		}
-		seenIds[p.Name] = append(seenIds[p.Name], p.In)
-		if strings.EqualFold(p.Name, timeoutName) {
-			timeoutName = renameTimeout(seenIds, timeoutName)
-		}
+		seenIds[strings.ToLower(idMapping[p.In][p.Name])] = true
 	}
+	timeoutName := renameTimeout(seenIds, "timeout")
 
 	for _, p := range paramsForOperation {
 		cp, err := b.MakeParameter(receiver, resolver, p, idMapping)


Test patch:
diff --git a/generator/operation_test.go b/generator/operation_test.go
index e943cb9e..1c1c64c3 100644
--- a/generator/operation_test.go
+++ b/generator/operation_test.go
@@ -1381,3 +1381,72 @@ func TestGenServer_StrictAdditionalProperties(t *testing.T) {
 		}
 	}
 }
+
+func makeClientTimeoutNameTest() []struct {
+	seenIds  map[string]bool
+	name     string
+	expected string
+} {
+	return []struct {
+		seenIds  map[string]bool
+		name     string
+		expected string
+	}{
+		{
+			seenIds:  nil,
+			name:     "witness",
+			expected: "witness",
+		},
+		{
+			seenIds: map[string]bool{
+				"id": true,
+			},
+			name:     "timeout",
+			expected: "timeout",
+		},
+		{
+			seenIds: map[string]bool{
+				"timeout":        true,
+				"requesttimeout": true,
+			},
+			name:     "timeout",
+			expected: "httpRequestTimeout",
+		},
+		{
+			seenIds: map[string]bool{
+				"timeout":            true,
+				"requesttimeout":     true,
+				"httprequesttimeout": true,
+				"swaggertimeout":     true,
+				"operationtimeout":   true,
+				"optimeout":          true,
+			},
+			name:     "timeout",
+			expected: "operTimeout",
+		},
+		{
+			seenIds: map[string]bool{
+				"timeout":            true,
+				"requesttimeout":     true,
+				"httprequesttimeout": true,
+				"swaggertimeout":     true,
+				"operationtimeout":   true,
+				"optimeout":          true,
+				"opertimeout":        true,
+				"opertimeout1":       true,
+			},
+			name:     "timeout",
+			expected: "operTimeout11",
+		},
+	}
+}
+
+func TestRenameTimeout(t *testing.T) {
+	for i, toPin := range makeClientTimeoutNameTest() {
+		testCase := toPin
+		t.Run(testCase.name, func(t *testing.T) {
+			//t.Parallel()
+			assert.Equalf(t, testCase.expected, renameTimeout(testCase.seenIds, testCase.name), "unexpected deconflicting value [%d]", i)
+		})
+	}
+}