pkg/yqlib/doc/operators/headers/system-operators.md

@@ -7,7 +7,7 @@ The `system` operator allows you to run an external command and use its output a
 ## Usage
 
 ```bash
-yq --enable-system-operator --null-input '.field = system("command"; "arg1")'
+yq --enable-system-operator '.field = system("command"; "arg1")'
 ```
 
 The operator takes:

pkg/yqlib/doc/operators/system-operators.md

@@ -7,7 +7,7 @@ The `system` operator allows you to run an external command and use its output a
 ## Usage
 
 ```bash
-yq --enable-system-operator --null-input '.field = system("command"; "arg1")'
+yq --enable-system-operator '.field = system("command"; "arg1")'
 ```
 
 The operator takes:
@@ -47,7 +47,7 @@ country: Australia
 ```
 then
 ```bash
-yq --enable-system-operator '.country = system("/usr/bin/echo"; "test")' sample.yml
+yq '.country = system("/usr/bin/echo"; "test")' sample.yml
 ```
 will output
 ```yaml
@@ -63,7 +63,7 @@ a: hello
 ```
 then
 ```bash
-yq --enable-system-operator '.a = system("/usr/bin/echo")' sample.yml
+yq '.a = system("/bin/echo")' sample.yml
 ```
 will output
 ```yaml

pkg/yqlib/operator_system.go

@@ -8,34 +8,6 @@ import (
 	"strings"
 )
 
-func resolveSystemArgs(argsNode *CandidateNode) []string {
-	if argsNode.Kind == SequenceNode {
-		args := make([]string, 0, len(argsNode.Content))
-		for _, child := range argsNode.Content {
-			args = append(args, child.Value)
-		}
-		return args
-	}
-	if argsNode.Tag != "!!null" {
-		return []string{argsNode.Value}
-	}
-	return nil
-}
-
-func resolveCommandNode(commandNodes Context) (string, error) {
-	if commandNodes.MatchingNodes.Front() == nil {
-		return "", fmt.Errorf("system operator: command expression returned no results")
-	}
-	if commandNodes.MatchingNodes.Len() > 1 {
-		log.Debugf("system operator: command expression returned %d results, using first", commandNodes.MatchingNodes.Len())
-	}
-	cmdNode := commandNodes.MatchingNodes.Front().Value.(*CandidateNode)
-	if cmdNode.Kind != ScalarNode || cmdNode.Tag == "!!null" {
-		return "", fmt.Errorf("system operator: command must be a string scalar")
-	}
-	return cmdNode.Value, nil
-}
-
 func systemOperator(d *dataTreeNavigator, context Context, expressionNode *ExpressionNode) (Context, error) {
 	if !ConfiguredSecurityPreferences.EnableSystemOps {
 		log.Warning("system operator is disabled, use --enable-system-operator flag to enable")
@@ -47,46 +19,55 @@ func systemOperator(d *dataTreeNavigator, context Context, expressionNode *Expre
 		return context.ChildContext(results), nil
 	}
 
-	// determine at parse time whether we have (command; args) or just (command)
-	hasArgs := expressionNode.RHS.Operation.OperationType == blockOpType
+	var command string
+	var argsExpression *ExpressionNode
+
+	// check if it's a block operator (command; args) or just (command)
+	if expressionNode.RHS.Operation.OperationType == blockOpType {
+		block := expressionNode.RHS
+		commandNodes, err := d.GetMatchingNodes(context.ReadOnlyClone(), block.LHS)
+		if err != nil {
+			return Context{}, err
+		}
+		if commandNodes.MatchingNodes.Front() == nil {
+			return Context{}, fmt.Errorf("system operator: command expression returned no results")
+		}
+		command = commandNodes.MatchingNodes.Front().Value.(*CandidateNode).Value
+		argsExpression = block.RHS
+	} else {
+		commandNodes, err := d.GetMatchingNodes(context.ReadOnlyClone(), expressionNode.RHS)
+		if err != nil {
+			return Context{}, err
+		}
+		if commandNodes.MatchingNodes.Front() == nil {
+			return Context{}, fmt.Errorf("system operator: command expression returned no results")
+		}
+		command = commandNodes.MatchingNodes.Front().Value.(*CandidateNode).Value
+	}
+
+	// evaluate args if present
+	var args []string
+	if argsExpression != nil {
+		argsNodes, err := d.GetMatchingNodes(context.ReadOnlyClone(), argsExpression)
+		if err != nil {
+			return Context{}, err
+		}
+		if argsNodes.MatchingNodes.Front() != nil {
+			argsNode := argsNodes.MatchingNodes.Front().Value.(*CandidateNode)
+			if argsNode.Kind == SequenceNode {
+				for _, child := range argsNode.Content {
+					args = append(args, child.Value)
+				}
+			} else if argsNode.Tag != "!!null" {
+				args = []string{argsNode.Value}
+			}
+		}
+	}
 
 	var results = list.New()
 
 	for el := context.MatchingNodes.Front(); el != nil; el = el.Next() {
 		candidate := el.Value.(*CandidateNode)
-		nodeContext := context.SingleReadonlyChildContext(candidate)
-
-		var command string
-		var args []string
-
-		if hasArgs {
-			block := expressionNode.RHS
-			commandNodes, err := d.GetMatchingNodes(nodeContext, block.LHS)
-			if err != nil {
-				return Context{}, err
-			}
-			command, err = resolveCommandNode(commandNodes)
-			if err != nil {
-				return Context{}, err
-			}
-
-			argsNodes, err := d.GetMatchingNodes(nodeContext, block.RHS)
-			if err != nil {
-				return Context{}, err
-			}
-			if argsNodes.MatchingNodes.Front() != nil {
-				args = resolveSystemArgs(argsNodes.MatchingNodes.Front().Value.(*CandidateNode))
-			}
-		} else {
-			commandNodes, err := d.GetMatchingNodes(nodeContext, expressionNode.RHS)
-			if err != nil {
-				return Context{}, err
-			}
-			command, err = resolveCommandNode(commandNodes)
-			if err != nil {
-				return Context{}, err
-			}
-		}
 
 		var stdin bytes.Buffer
 		if candidate.Tag != "!!null" {
@@ -112,12 +93,7 @@ func systemOperator(d *dataTreeNavigator, context Context, expressionNode *Expre
 			return Context{}, fmt.Errorf("system command '%v' failed: %w", command, err)
 		}
 
-		result := string(output)
-		if strings.HasSuffix(result, "\r\n") {
-			result = result[:len(result)-2]
-		} else if strings.HasSuffix(result, "\n") {
-			result = result[:len(result)-1]
-		}
+		result := strings.TrimRight(string(output), "\n")
 		newNode := candidate.CreateReplacement(ScalarNode, "!!str", result)
 		results.PushBack(newNode)
 	}

pkg/yqlib/operator_system_test.go

@@ -1,19 +1,9 @@
 package yqlib
 
 import (
-	"os/exec"
 	"testing"
 )
 
-func findExec(t *testing.T, name string) string {
-	t.Helper()
-	path, err := exec.LookPath(name)
-	if err != nil {
-		t.Skipf("skipping: %v not found: %v", name, err)
-	}
-	return path
-}
-
 var systemOperatorDisabledScenarios = []expressionScenario{
 	{
 		description:    "system operator returns null when disabled",
@@ -26,7 +16,46 @@ var systemOperatorDisabledScenarios = []expressionScenario{
 	},
 }
 
+var systemOperatorEnabledScenarios = []expressionScenario{
+	{
+		description:    "Run a command with an argument",
+		subdescription: "Use `--enable-system-operator` to enable the system operator.",
+		document:       "country: Australia",
+		expression:     `.country = system("/usr/bin/echo"; "test")`,
+		expected: []string{
+			"D0, P[], (!!map)::country: test\n",
+		},
+	},
+	{
+		description:    "Run a command without arguments",
+		subdescription: "Omit the semicolon and args to run the command with no extra arguments.",
+		document:       "a: hello",
+		expression:     `.a = system("/bin/echo")`,
+		expected: []string{
+			"D0, P[], (!!map)::a: \"\"\n",
+		},
+	},
+	{
+		description:    "Run a command with multiple arguments",
+		subdescription: "Pass an array of arguments.",
+		skipDoc:        true,
+		document:       "a: hello",
+		expression:     `.a = system("/bin/echo"; ["foo", "bar"])`,
+		expected: []string{
+			"D0, P[], (!!map)::a: foo bar\n",
+		},
+	},
+	{
+		description:   "Command failure returns error",
+		skipDoc:       true,
+		document:      "a: hello",
+		expression:    `.a = system("/bin/false")`,
+		expectedError: "system command '/bin/false' failed: exit status 1",
+	},
+}
+
 func TestSystemOperatorDisabledScenarios(t *testing.T) {
+	// ensure system operator is disabled
 	originalEnableSystemOps := ConfiguredSecurityPreferences.EnableSystemOps
 	defer func() {
 		ConfiguredSecurityPreferences.EnableSystemOps = originalEnableSystemOps
@@ -41,9 +70,6 @@ func TestSystemOperatorDisabledScenarios(t *testing.T) {
 }
 
 func TestSystemOperatorEnabledScenarios(t *testing.T) {
-	echoPath := findExec(t, "echo")
-	falsePath := findExec(t, "false")
-
 	originalEnableSystemOps := ConfiguredSecurityPreferences.EnableSystemOps
 	defer func() {
 		ConfiguredSecurityPreferences.EnableSystemOps = originalEnableSystemOps
@@ -51,64 +77,8 @@ func TestSystemOperatorEnabledScenarios(t *testing.T) {
 
 	ConfiguredSecurityPreferences.EnableSystemOps = true
 
-	scenarios := []expressionScenario{
-		{
-			description:    "Run a command with an argument",
-			subdescription: "Use `--enable-system-operator` to enable the system operator.",
-			yqFlags:        "--enable-system-operator",
-			document:       "country: Australia",
-			expression:     `.country = system("` + echoPath + `"; "test")`,
-			expected: []string{
-				"D0, P[], (!!map)::country: test\n",
-			},
-		},
-		{
-			description:    "Run a command without arguments",
-			subdescription: "Omit the semicolon and args to run the command with no extra arguments.",
-			yqFlags:        "--enable-system-operator",
-			document:       "a: hello",
-			expression:     `.a = system("` + echoPath + `")`,
-			expected: []string{
-				"D0, P[], (!!map)::a: \"\"\n",
-			},
-		},
-		{
-			description:    "Run a command with multiple arguments",
-			subdescription: "Pass an array of arguments.",
-			skipDoc:        true,
-			document:       "a: hello",
-			expression:     `.a = system("` + echoPath + `"; ["foo", "bar"])`,
-			expected: []string{
-				"D0, P[], (!!map)::a: foo bar\n",
-			},
-		},
-		{
-			description: "Command and args are evaluated per matched node",
-			skipDoc:     true,
-			document:    "cmd: " + echoPath + "\narg: hello",
-			expression:  `.result = system(.cmd; .arg)`,
-			expected: []string{
-				"D0, P[], (!!map)::cmd: " + echoPath + "\narg: hello\nresult: hello\n",
-			},
-		},
-		{
-			description:   "Command failure returns error",
-			skipDoc:       true,
-			document:      "a: hello",
-			expression:    `.a = system("` + falsePath + `")`,
-			expectedError: "system command '" + falsePath + "' failed: exit status 1",
-		},
-		{
-			description:   "Null command returns error",
-			skipDoc:       true,
-			document:      "a: hello",
-			expression:    `.a = system(null)`,
-			expectedError: "system operator: command must be a string scalar",
-		},
-	}
-
-	for _, tt := range scenarios {
+	for _, tt := range systemOperatorEnabledScenarios {
 		testScenario(t, &tt)
 	}
-	appendOperatorDocumentScenario(t, "system-operators", scenarios)
+	appendOperatorDocumentScenario(t, "system-operators", systemOperatorEnabledScenarios)
 }

pkg/yqlib/operators_test.go

@@ -31,7 +31,6 @@ type expressionScenario struct {
 	dontFormatInputForDoc bool // dont format input doc for documentation generation
 	requiresFormat        string
 	skipForGoccy          bool
-	yqFlags               string // extra yq flags to include in generated doc command snippets
 }
 
 var goccyTesting = false
@@ -357,22 +356,14 @@ func documentInput(w *bufio.Writer, s expressionScenario) (string, string) {
 
 		writeOrPanic(w, "then\n")
 
-		flagsPrefix := ""
-		if s.yqFlags != "" {
-			flagsPrefix = s.yqFlags + " "
-		}
 		if s.expression != "" {
-			writeOrPanic(w, fmt.Sprintf("```bash\n%vyq %v%v'%v' %v\n```\n", envCommand, flagsPrefix, command, strings.ReplaceAll(s.expression, "'", `'\''`), files))
+			writeOrPanic(w, fmt.Sprintf("```bash\n%vyq %v'%v' %v\n```\n", envCommand, command, strings.ReplaceAll(s.expression, "'", `'\''`), files))
 		} else {
-			writeOrPanic(w, fmt.Sprintf("```bash\n%vyq %v%v%v\n```\n", envCommand, flagsPrefix, command, files))
+			writeOrPanic(w, fmt.Sprintf("```bash\n%vyq %v%v\n```\n", envCommand, command, files))
 		}
 	} else {
 		writeOrPanic(w, "Running\n")
-		flagsPrefix := ""
-		if s.yqFlags != "" {
-			flagsPrefix = s.yqFlags + " "
-		}
-		writeOrPanic(w, fmt.Sprintf("```bash\n%vyq %v%v--null-input '%v'\n```\n", envCommand, flagsPrefix, command, s.expression))
+		writeOrPanic(w, fmt.Sprintf("```bash\n%vyq %v--null-input '%v'\n```\n", envCommand, command, s.expression))
 	}
 	return formattedDoc, formattedDoc2
 }