From e3d9a5dd4db7c0a8f1da585fead74d409d72ff70 Mon Sep 17 00:00:00 2001
From: jwgcooke <gordon.cooke@upstartcommerce.com>
Date: Fri, 1 Aug 2025 08:37:44 -0400
Subject: [PATCH] alpha sort for map keys and tests

---
 functions/core/alphabetical.go      | 61 +++++++++++++----------------
 functions/core/alphabetical_test.go | 56 ++++++++++++++++++++++++++
 2 files changed, 83 insertions(+), 34 deletions(-)

diff --git a/functions/core/alphabetical.go b/functions/core/alphabetical.go
index 32e102fd..0d1fa7f8 100644
--- a/functions/core/alphabetical.go
+++ b/functions/core/alphabetical.go
@@ -48,9 +48,6 @@ func (a Alphabetical) RunRule(nodes []*yaml.Node, context model.RuleFunctionCont
 
 	var keyedBy string
 
-	// extract a custom message
-	message := context.Rule.Message
-
 	// check supplied type - use cached options to avoid repeated interface conversions
 	props := context.GetOptionsStringMap()
 	if props["keyedBy"] != "" {
@@ -65,39 +62,16 @@ func (a Alphabetical) RunRule(nodes []*yaml.Node, context model.RuleFunctionCont
 
 		if utils.IsNodeMap(node) {
 			if keyedBy == "" {
-				locatedObjects, err := context.DrDocument.LocateModel(node)
-				locatedPath := pathValue
-				var allPaths []string
-				if err == nil && locatedObjects != nil {
-					for x, obj := range locatedObjects {
-						if x == 0 {
-							locatedPath = obj.GenerateJSONPath()
-						}
-						allPaths = append(allPaths, obj.GenerateJSONPath())
-					}
-				}
-				result := model.RuleFunctionResult{
-					Message: vacuumUtils.SuppliedOrDefault(message,
-						model.GetStringTemplates().BuildTypeErrorMessage(context.Rule.Description, node.Value, "map/object", a.GetSchema().ErrorMessage)),
-					StartNode: node,
-					EndNode:   vacuumUtils.BuildEndNode(node),
-					Path:      locatedPath,
-					Rule:      context.Rule,
-				}
-				if len(allPaths) > 1 {
-					result.Paths = allPaths
+				// Sort by map keys when keyedBy is not provided
+				mapKeys := a.extractMapKeys(node)
+				if len(mapKeys) > 0 {
+					mapResults := a.reportMapKeyViolation(node, mapKeys, context)
+					results = append(results, mapResults...)
 				}
-				results = append(results, result)
-				if len(locatedObjects) > 0 {
-					if arr, ok := locatedObjects[0].(v3.AcceptsRuleResults); ok {
-						arr.AddRuleFunctionResult(v3.ConvertRuleResult(&result))
-					}
-				}
-				continue
+			} else {
+				resultsFromKey := a.processMap(node, keyedBy, context)
+				results = append(results, compareStringArray(node, resultsFromKey, context)...)
 			}
-
-			resultsFromKey := a.processMap(node, keyedBy, context)
-			results = compareStringArray(node, resultsFromKey, context)
 			results = model.MapPathAndNodesToResults(pathValue, node, node, results)
 			continue
 		}
@@ -361,3 +335,22 @@ func (a Alphabetical) evaluateFloatArray(node *yaml.Node, floatArray []float64,
 	}
 	return results
 }
+
+// extractMapKeys extracts and sorts the keys from a map node
+func (a Alphabetical) extractMapKeys(node *yaml.Node) []string {
+	var keys []string
+	for i := 0; i < len(node.Content); i += 2 {
+		if i < len(node.Content) {
+			keys = append(keys, node.Content[i].Value)
+		}
+	}
+	return keys
+}
+
+// reportMapKeyViolation checks if map keys are sorted and reports violations
+func (a Alphabetical) reportMapKeyViolation(node *yaml.Node, keys []string, context model.RuleFunctionContext) []model.RuleFunctionResult {
+	if sort.StringsAreSorted(keys) {
+		return nil
+	}
+	return compareStringArray(node, keys, context)
+}
diff --git a/functions/core/alphabetical_test.go b/functions/core/alphabetical_test.go
index b9f7cf86..3c03a027 100644
--- a/functions/core/alphabetical_test.go
+++ b/functions/core/alphabetical_test.go
@@ -299,3 +299,59 @@ func TestAlphabetical_RunRule_ObjectFailNoKeyedBy(t *testing.T) {
 
 	assert.Len(t, res, 1)
 }
+
+func TestAlphabetical_RunRule_MapKeysSortedSuccess(t *testing.T) {
+	yml := `components:
+  schemas:
+    aaa:
+      type: string
+    bbb:
+      type: string
+    ccc:
+      type: string`
+
+	path := "$.components.schemas"
+
+	nodes, _ := utils.FindNodes([]byte(yml), path)
+
+	opts := make(map[string]any)
+	// No keyedBy provided - should sort by map keys
+
+	rule := buildCoreTestRule(path, model.SeverityError, "alphabetical", "", opts)
+	ctx := buildCoreTestContextFromRule(model.CastToRuleAction(rule.Then), rule)
+	ctx.Given = path
+	ctx.Rule = &rule
+
+	def := &Alphabetical{}
+	res := def.RunRule(nodes, ctx)
+
+	assert.Len(t, res, 0) // No violations - keys are already sorted
+}
+
+func TestAlphabetical_RunRule_MapKeysUnsortedFail(t *testing.T) {
+	yml := `components:
+  schemas:
+    zebra:
+      type: string
+    apple:
+      type: string
+    banana:
+      type: string`
+
+	path := "$.components.schemas"
+
+	nodes, _ := utils.FindNodes([]byte(yml), path)
+
+	opts := make(map[string]any)
+	// No keyedBy provided - should sort by map keys
+
+	rule := buildCoreTestRule(path, model.SeverityError, "alphabetical", "", opts)
+	ctx := buildCoreTestContextFromRule(model.CastToRuleAction(rule.Then), rule)
+	ctx.Given = path
+	ctx.Rule = &rule
+
+	def := &Alphabetical{}
+	res := def.RunRule(nodes, ctx)
+
+	assert.Len(t, res, 1) // Should report violation - keys are not sorted
+}