Investigate issue 379: Duplicate Parameters

index/spec_index_test.go

@@ -731,6 +731,85 @@ paths:
 	assert.Equal(t, 1, len(index.GetOperationParametersIndexErrors()))
 }
 
+func TestSpecIndex_ParametersWithSameNameDifferentIn(t *testing.T) {
+	spec1 := `openapi: 3.1.0
+info:
+  title: Test API
+  version: 1.0.0
+paths:
+  /test:
+    get:
+      parameters:
+        - name: random_id
+          in: path
+          required: true
+          schema:
+            type: string
+        - name: random_id
+          in: query
+          required: false
+          schema:
+            type: integer
+        - name: random_id
+          in: header
+          required: false
+          schema:
+            type: boolean`
+
+	spec2 := `openapi: 3.1.0
+info:
+  title: Test API
+  version: 1.0.0
+paths:
+  /test:
+    get:
+      parameters:
+        - name: random_id
+          in: query
+          required: false
+          schema:
+            type: integer
+        - name: random_id
+          in: path
+          required: true
+          schema:
+            type: string
+        - name: random_id
+          in: header
+          required: false
+          schema:
+            type: boolean`
+
+	var rootNode1 yaml.Node
+	_ = yaml.Unmarshal([]byte(spec1), &rootNode1)
+	index1 := NewSpecIndexWithConfig(&rootNode1, CreateOpenAPIIndexConfig())
+	params1 := index1.GetAllParametersFromOperations()
+
+	var rootNode2 yaml.Node
+	_ = yaml.Unmarshal([]byte(spec2), &rootNode2)
+	index2 := NewSpecIndexWithConfig(&rootNode2, CreateOpenAPIIndexConfig())
+	params2 := index2.GetAllParametersFromOperations()
+
+	count1 := countParametersFromOperations(params1)
+	count2 := countParametersFromOperations(params2)
+
+	assert.Equal(t, 3, count1)
+	assert.Equal(t, 3, count2)
+	assert.Equal(t, count1, count2)
+}
+
+func countParametersFromOperations(params map[string]map[string]map[string][]*Reference) int {
+	total := 0
+	for _, pathParams := range params {
+		for _, methodParams := range pathParams {
+			for _, refs := range methodParams {
+				total += len(refs)
+			}
+		}
+	}
+	return total
+}
+
 func TestSpecIndex_BurgerShop_AllTheComponents(t *testing.T) {
 	burgershop, _ := os.ReadFile("../test_specs/all-the-components.yaml")
 	var rootNode yaml.Node
@@ -1976,3 +2055,86 @@ components:
 	assert.NotNil(t, index.GetRolodex())
 
 }
+
+func TestSpecIndex_DuplicateParameterHandling(t *testing.T) {
+	// Test duplicate parameter detection: two query params + one path param with same name
+	// Should return 2 parameters (1 path + 1 query) and record 1 duplicate error
+	spec1 := `openapi: 3.1.0
+info:
+  title: Test API
+  version: 1.0.0
+paths:
+  /items/{random_id}:
+    get:
+      parameters:
+        - name: random_id
+          in: path
+          required: true
+          schema:
+            type: boolean
+        - name: random_id
+          in: query
+          required: true
+          schema:
+            type: integer
+        - name: random_id
+          in: query
+          required: true
+          schema:
+            type: boolean`
+
+	// Test with different parameter order to ensure consistency
+	spec2 := `openapi: 3.1.0
+info:
+  title: Test API
+  version: 1.0.0
+paths:
+  /items/{random_id}:
+    get:
+      parameters:
+        - name: random_id
+          in: query
+          required: true
+          schema:
+            type: integer
+        - name: random_id
+          in: query
+          required: true
+          schema:
+            type: boolean
+        - name: random_id
+          in: path
+          required: true
+          schema:
+            type: boolean`
+
+	var rootNode1 yaml.Node
+	_ = yaml.Unmarshal([]byte(spec1), &rootNode1)
+	index1 := NewSpecIndexWithConfig(&rootNode1, CreateOpenAPIIndexConfig())
+	params1 := index1.GetAllParametersFromOperations()
+	errors1 := index1.GetOperationParametersIndexErrors()
+
+	var rootNode2 yaml.Node
+	_ = yaml.Unmarshal([]byte(spec2), &rootNode2)
+	index2 := NewSpecIndexWithConfig(&rootNode2, CreateOpenAPIIndexConfig())
+	params2 := index2.GetAllParametersFromOperations()
+	errors2 := index2.GetOperationParametersIndexErrors()
+
+	count1 := countParametersFromOperations(params1)
+	count2 := countParametersFromOperations(params2)
+
+	// Should have 2 parameters total (1 path + 1 query, second query rejected as duplicate)
+	assert.Equal(t, 2, count1, "Spec1 should have 2 parameters (1 path + 1 query), not 3")
+	assert.Equal(t, 2, count2, "Spec2 should have 2 parameters (1 path + 1 query), not 3")
+	assert.Equal(t, count1, count2, "Both specs should return same parameter count regardless of order")
+
+	// Should have exactly 1 duplicate error for the second query parameter
+	assert.Equal(t, 1, len(errors1), "Spec1 should have 1 duplicate parameter error")
+	assert.Equal(t, 1, len(errors2), "Spec2 should have 1 duplicate parameter error")
+
+	// Verify the error message mentions duplicate name and in type
+	assert.Contains(t, errors1[0].Error(), "duplicate name")
+	assert.Contains(t, errors1[0].Error(), "random_id")
+	assert.Contains(t, errors2[0].Error(), "duplicate name")
+	assert.Contains(t, errors2[0].Error(), "random_id")
+}

index/utility_methods.go

@@ -510,26 +510,20 @@ func (index *SpecIndex) scanOperationParams(params []*yaml.Node, keyNode, pathIt
 				checkNodes := index.paramOpRefs[pathItemNode.Value][method][ref.Name]
 				_, currentIn := utils.FindKeyNodeTop("in", currentNode.Content)
 
-				for _, checkNode := range checkNodes {
-
-					_, checkIn := utils.FindKeyNodeTop("in", checkNode.Node.Content)
-
-					if currentIn != nil && checkIn != nil && currentIn.Value == checkIn.Value {
-
-						path := fmt.Sprintf("$.paths['%s'].%s.parameters[%d]", pathItemNode.Value, method, i)
-						if method == "top" {
-							path = fmt.Sprintf("$.paths['%s'].parameters[%d]", pathItemNode.Value, i)
-						}
-
-						index.operationParamErrors = append(index.operationParamErrors, &IndexingError{
-							Err: fmt.Errorf("the `%s` operation parameter at path `%s`, "+
-								"index %d has a duplicate name `%s` and `in` type", strings.ToUpper(method), pathItemNode.Value, i, vn.Value),
-							Node: param,
-							Path: path,
-						})
-					} else {
-						index.paramOpRefs[pathItemNode.Value][method][ref.Name] = append(index.paramOpRefs[pathItemNode.Value][method][ref.Name], ref)
+				if hasDuplicateInType(currentIn, checkNodes) {
+					path := fmt.Sprintf("$.paths['%s'].%s.parameters[%d]", pathItemNode.Value, method, i)
+					if method == "top" {
+						path = fmt.Sprintf("$.paths['%s'].parameters[%d]", pathItemNode.Value, i)
 					}
+
+					index.operationParamErrors = append(index.operationParamErrors, &IndexingError{
+						Err: fmt.Errorf("the `%s` operation parameter at path `%s`, "+
+							"index %d has a duplicate name `%s` and `in` type", strings.ToUpper(method), pathItemNode.Value, i, vn.Value),
+						Node: param,
+						Path: path,
+					})
+				} else {
+					index.paramOpRefs[pathItemNode.Value][method][ref.Name] = append(index.paramOpRefs[pathItemNode.Value][method][ref.Name], ref)
 				}
 			} else {
 				index.paramOpRefs[pathItemNode.Value][method][ref.Name] = append(index.paramOpRefs[pathItemNode.Value][method][ref.Name], ref)
@@ -813,3 +807,13 @@ func hashNodeSimple(n *yaml.Node, h hash.Hash, depth int) {
 		}
 	}
 }
+
+func hasDuplicateInType(currentIn *yaml.Node, existingRefs []*Reference) bool {
+	for _, ref := range existingRefs {
+		_, existingIn := utils.FindKeyNodeTop("in", ref.Node.Content)
+		if currentIn != nil && existingIn != nil && currentIn.Value == existingIn.Value {
+			return true
+		}
+	}
+	return false
+}