Merge pull request #1500 from sulemaanhamza/bugfix-core-pkg-fixhandler-yamlhelper-edgecases

Edge-case Handling and Test Enhancements in yamlhelper

core/pkg/fixhandler/yamlhelper.go

@@ -49,19 +49,26 @@ func matchNodes(nodeOne, nodeTwo *yaml.Node) NodeRelation {
 func adjustContentLines(contentToAdd *[]contentToAdd, linesSlice *[]string) {
 	for contentIdx, content := range *contentToAdd {
 		line := content.line
-
 		// Adjust line numbers such that there are no "empty lines or comment lines of next nodes" before them
 		for idx := line - 1; idx >= 0; idx-- {
-			if isEmptyLineOrComment((*linesSlice)[idx]) {
-				(*contentToAdd)[contentIdx].line -= 1
-			} else {
-				break
+			// If idx is exceeding the length of linesSlice, skip it
+			if idx < len(*linesSlice) {
+				if isEmptyLineOrComment((*linesSlice)[idx]) {
+					(*contentToAdd)[contentIdx].line -= 1
+				} else {
+					break
+				}
 			}
 		}
 	}
 }
 
 func adjustFixedListLines(originalList, fixedList *[]nodeInfo) {
+
+	if originalList == nil || fixedList == nil || len(*originalList) == 0 || len(*fixedList) == 0 {
+		return // Check for empty slices to avoid index out of range errors
+	}
+
 	differenceAtTop := (*originalList)[0].node.Line - (*fixedList)[0].node.Line
 
 	if differenceAtTop <= 0 {
@@ -77,6 +84,11 @@ func adjustFixedListLines(originalList, fixedList *[]nodeInfo) {
 }
 
 func enocodeIntoYaml(parentNode *yaml.Node, nodeList *[]nodeInfo, tracker int) (string, error) {
+
+	if tracker < 0 || tracker >= len(*nodeList) {
+		return "", fmt.Errorf("Index out of range for nodeList: tracker=%d, length=%d", tracker, len(*nodeList))
+	}
+
 	content := make([]*yaml.Node, 0)
 	currentNode := (*nodeList)[tracker].node
 	content = append(content, currentNode)
@@ -123,7 +135,17 @@ func getContent(ctx context.Context, parentNode *yaml.Node, nodeList *[]nodeInfo
 }
 
 func indentContent(content string, indentationSpaces int) string {
+
+	if content == "" {
+		return ""
+	}
+
 	indentedContent := ""
+
+	if indentationSpaces < 0 {
+		indentationSpaces = 0
+	}
+
 	indentSpaces := strings.Repeat(" ", indentationSpaces)
 
 	scanner := bufio.NewScanner(strings.NewReader(content))
@@ -198,7 +220,7 @@ func getLastLineOfResource(linesSlice *[]string, currentLine int) (int, error) {
 }
 
 func getNodeLine(nodeList *[]nodeInfo, tracker int) int {
-	if tracker < len(*nodeList) {
+	if tracker >= 0 && tracker < len(*nodeList) {
 		return (*nodeList)[tracker].node.Line
 	} else {
 		return -1
@@ -253,6 +275,9 @@ func isOneLineSequenceNode(list *[]nodeInfo, currentTracker int) (bool, int) {
 
 // Checks if nodes are of same kind, value, line and column
 func isSameNode(nodeOne, nodeTwo *yaml.Node) bool {
+	if nodeOne == nil || nodeTwo == nil {
+		return false // Ensure neither node is nil to prevent runtime errors
+	}
 	sameLines := nodeOne.Line == nodeTwo.Line
 	sameColumns := nodeOne.Column == nodeTwo.Column
 	sameKinds := nodeOne.Kind == nodeTwo.Kind
@@ -339,15 +364,15 @@ func replaceSingleLineSequence(ctx context.Context, fixInfoMetadata *fixInfoMeta
 
 // Returns the first node in the given line that is not mapping node
 func getFirstNodeInLine(list *[]nodeInfo, line int) int {
-	tracker := 0
-
-	currentNode := (*list)[tracker].node
-	for currentNode.Line != line || currentNode.Kind == yaml.MappingNode {
-		tracker += 1
-		currentNode = (*list)[tracker].node
+	for tracker := 0; tracker < len(*list); tracker++ {
+		currentNode := (*list)[tracker].node
+		if currentNode.Line == line && currentNode.Kind != yaml.MappingNode {
+			return tracker
+		}
 	}
 
-	return tracker
+	// Return -1 to indicate that the node with the specified line was not found
+	return -1
 }
 
 // To not mess with the line number while inserting, removed lines are not deleted but replaced with "*"
@@ -357,6 +382,10 @@ func removeLines(linesToRemove *[]linesToRemove, linesSlice *[]string) {
 		startLine = lineToRemove.startLine - 1
 		endLine = lineToRemove.endLine - 1
 
+		if startLine < 0 || endLine >= len(*linesSlice) {
+			continue // Skip if the indices are out of bounds
+		}
+
 		for line := startLine; line <= endLine; line++ {
 			lineContent := (*linesSlice)[line]
 			// When determining the endLine, empty lines and comments which are not intended to be removed are included.

core/pkg/fixhandler/yamlhelper_test.go

@@ -2,6 +2,7 @@ package fixhandler
 
 import (
 	"fmt"
+	"reflect"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -69,6 +70,44 @@ func TestAdjustContentLines_AdjustsLineNumbersForContentToAddBasedOnEmptyOrComme
 	assert.Equal(t, 10, contentToAdd[9].line)
 }
 
+// Adjusts line numbers for contentToAdd based on empty or comment lines before them
+func TestAdjustContentLines_TestEdgeCaseWHereContentToAddHaveMoreLines(t *testing.T) {
+
+	contentToAdd := []contentToAdd{
+		{line: 1},
+		{line: 2},
+		{line: 3},
+		{line: 4},
+		{line: 5},
+		{line: 6},
+		{line: 7},
+		{line: 8},
+		{line: 9},
+	}
+
+	linesSlice := []string{
+		"line 1",
+		"line 2",
+		"line 3",
+		"",
+		"# comment",
+		"line 6",
+		"",
+	}
+
+	adjustContentLines(&contentToAdd, &linesSlice)
+
+	assert.Equal(t, 1, contentToAdd[0].line)
+	assert.Equal(t, 2, contentToAdd[1].line)
+	assert.Equal(t, 3, contentToAdd[2].line)
+	assert.Equal(t, 3, contentToAdd[3].line)
+	assert.Equal(t, 3, contentToAdd[4].line)
+	assert.Equal(t, 6, contentToAdd[5].line)
+	assert.Equal(t, 6, contentToAdd[6].line)
+	assert.Equal(t, 7, contentToAdd[7].line)
+	assert.Equal(t, 8, contentToAdd[8].line)
+}
+
 // If the differenceAtTop is less than or equal to 0, the function should return without modifying the fixedList.
 func TestAdjustFixedListLines_WhenDifferenceAtTopIsLessThanOrEqualTo0(t *testing.T) {
 	originalList := []nodeInfo{
@@ -91,6 +130,20 @@ func TestAdjustFixedListLines_WhenDifferenceAtTopIsLessThanOrEqualTo0(t *testing
 	}, fixedList)
 }
 
+// When the fixedList is empty, the function returns without modifying the line numbers of the fixedList.
+func TestAdjustFixedListLines_emptyFixedList(t *testing.T) {
+	originalList := []nodeInfo{
+		{node: &yaml.Node{Line: 1}},
+		{node: &yaml.Node{Line: 2}},
+		{node: &yaml.Node{Line: 3}},
+	}
+	fixedList := []nodeInfo{}
+
+	adjustFixedListLines(&originalList, &fixedList)
+
+	assert.Empty(t, fixedList)
+}
+
 // Encodes a YAML node into a string
 func TestEncodeIntoYaml_EncodesYamlNodeIntoString(t *testing.T) {
 	parentNode := &yaml.Node{
@@ -118,6 +171,32 @@ func TestEncodeIntoYaml_EncodesYamlNodeIntoString(t *testing.T) {
 	assert.Equal(t, "key: value\n", result)
 }
 
+// Encodes a YAML node into a string
+func TestEncodeIntoYaml_EncodesNodeIntoStringWithNegativeTracker(t *testing.T) {
+	parentNode := &yaml.Node{
+		Kind: yaml.MappingNode,
+	}
+	nodeList := []nodeInfo{
+		{
+			node: &yaml.Node{
+				Kind:  yaml.ScalarNode,
+				Value: "key",
+			},
+		},
+		{
+			node: &yaml.Node{
+				Kind:  yaml.ScalarNode,
+				Value: "value",
+			},
+		},
+	}
+	tracker := -1
+
+	_, err := enocodeIntoYaml(parentNode, &nodeList, tracker)
+
+	assert.Error(t, err)
+}
+
 // Given a non-empty string content and a positive integer indentationSpaces, the function should return a string with each line of the content indented by the specified number of spaces.
 func TestIndentContentNonEmptyStringPositiveIndentationSpaces(t *testing.T) {
 	content := "line1\nline2\nline3"
@@ -131,6 +210,19 @@ func TestIndentContentNonEmptyStringPositiveIndentationSpaces(t *testing.T) {
 	}
 }
 
+// Should correctly indent content with negative indentation spaces
+func TestIndentContentNegativeIndentationSpaces(t *testing.T) {
+	content := "line1\nline2\nline3"
+	indentationSpaces := -2
+	expected := "line1\nline2\nline3\n"
+
+	result := indentContent(content, indentationSpaces)
+
+	if result != expected {
+		t.Errorf("Expected %q, but got %q", expected, result)
+	}
+}
+
 // Returns the correct line to insert when originalListTracker is non-negative.
 func TestGetLineToInsertNonNegative(t *testing.T) {
 	fixInfoMetadata := &fixInfoMetadata{
@@ -184,6 +276,20 @@ func TestGetNodeLine_ReturnsLineNumberOfNodeAtGivenTrackerPosition(t *testing.T)
 	assert.Equal(t, 2, line)
 }
 
+// Returns an error if the tracker position is a negative value.
+func TestGetNodeLine_TrackerPositionNegativeValue_ReturnsError(t *testing.T) {
+	nodeList := []nodeInfo{
+		{node: &yaml.Node{Line: 1}},
+		{node: &yaml.Node{Line: 2}},
+		{node: &yaml.Node{Line: 3}},
+	}
+	tracker := -2
+
+	line := getNodeLine(&nodeList, tracker)
+
+	assert.Equal(t, -1, line)
+}
+
 // Returns true if the node is a value node in a mapping node with an odd index
 func TestIsValueNodeInMapping_ReturnsTrueIfNodeIsValueNodeInMappingWithOddIndex(t *testing.T) {
 	node := &nodeInfo{
@@ -218,6 +324,24 @@ func TestIsSameNode_ReturnsTrueIfSameLineColumnKindAndValue(t *testing.T) {
 	assert.True(t, result)
 }
 
+func TestIsSameNode_ReturnsFalseIfEitherNodeIsNil(t *testing.T) {
+	nodeOne := &yaml.Node{
+		Line:   1,
+		Column: 2,
+		Kind:   yaml.ScalarNode,
+		Value:  "value",
+	}
+	var nodeTwo *yaml.Node
+
+	result := isSameNode(nodeOne, nodeTwo)
+
+	assert.False(t, result)
+
+	result = isSameNode(nodeTwo, nodeOne)
+
+	assert.False(t, result)
+}
+
 // Returns True for empty string
 func TestReturnsTrueForEmptyString(t *testing.T) {
 	lineContent := ""
@@ -242,6 +366,20 @@ func TestGetFirstNodeInLine_ReturnsIndex(t *testing.T) {
 	}
 }
 
+// returns -1 when the given line is not found in the list
+func TestGetFirstNodeInLine_LineNotFound(t *testing.T) {
+	list := []nodeInfo{
+		{node: &yaml.Node{Line: 1}},
+		{node: &yaml.Node{Line: 2}},
+		{node: &yaml.Node{Line: 3}},
+	}
+	line := 4
+
+	index := getFirstNodeInLine(&list, line)
+
+	assert.Equal(t, -1, index)
+}
+
 // Function removes lines within specified range
 func TestRemoveLinesWithinRange(t *testing.T) {
 	linesToRemove := []linesToRemove{
@@ -267,6 +405,31 @@ func TestRemoveLinesWithinRange(t *testing.T) {
 	assert.Equal(t, expected, linesSlice)
 }
 
+// The function correctly handles cases where the startLine and endLine are out of range of the input slice.
+func TestRemoveOutOfRangeLines(t *testing.T) {
+	linesToRemove := []linesToRemove{
+		{startLine: 5, endLine: 7},
+	}
+	linesSlice := []string{
+		"line 1",
+		"line 2",
+		"line 3",
+		"line 4",
+	}
+	expected := []string{
+		"line 1",
+		"line 2",
+		"line 3",
+		"line 4",
+	}
+
+	removeLines(&linesToRemove, &linesSlice)
+
+	if !reflect.DeepEqual(linesSlice, expected) {
+		t.Errorf("Expected %v, but got %v", expected, linesSlice)
+	}
+}
+
 // The function should correctly calculate the total number of children of the given node and add it to the current tracker.
 func TestShouldCalculateTotalNumberOfChildrenAndAddToCurrentTracker(t *testing.T) {
 	node := &yaml.Node{