Merge pull request #4376 from alexandear/refactor/modernize-stringsbuilder

refactor: fix modernize.stringsbuilder lint issues

Author: jandubois

.golangci.yml

@@ -75,7 +75,6 @@ linters:
     modernize:
       disable:
       - omitzero
-      - stringsbuilder
     perfsprint:
       int-conversion: false
       err-error: false

cmd/limactl/editflags/editflags.go

@@ -155,36 +155,29 @@ func BuildPortForwardExpression(portForwards []string) (string, error) {
 		return "", nil
 	}
 
-	expr := `.portForwards += [`
+	ports := make([]string, len(portForwards))
 	for i, spec := range portForwards {
 		hostPort, guestPort, isStatic, err := ParsePortForward(spec)
 		if err != nil {
 			return "", err
 		}
-		expr += fmt.Sprintf(`{"guestPort": %q, "hostPort": %q, "static": %v}`, guestPort, hostPort, isStatic)
-		if i < len(portForwards)-1 {
-			expr += ","
-		}
+		ports[i] = fmt.Sprintf(`{"guestPort": %q, "hostPort": %q, "static": %v}`, guestPort, hostPort, isStatic)
 	}
-	expr += `]`
+	expr := fmt.Sprintf(".portForwards += [%s]", strings.Join(ports, ","))
 	return expr, nil
 }
 
 func buildMountListExpression(ss []string) (string, error) {
-	expr := `[`
+	mounts := make([]string, len(ss))
 	for i, s := range ss {
 		writable := strings.HasSuffix(s, ":w")
-		loc := strings.TrimSuffix(s, ":w")
-		loc, err := localpathutil.Expand(loc)
+		loc, err := localpathutil.Expand(strings.TrimSuffix(s, ":w"))
 		if err != nil {
 			return "", err
 		}
-		expr += fmt.Sprintf(`{"location": %q, "mountPoint": %q, "writable": %v}`, loc, loc, writable)
-		if i < len(ss)-1 {
-			expr += ","
-		}
+		mounts[i] = fmt.Sprintf(`{"location": %q, "mountPoint": %q, "writable": %v}`, loc, loc, writable)
 	}
-	expr += `]`
+	expr := fmt.Sprintf("[%s]", strings.Join(mounts, ","))
 	return expr, nil
 }
 
@@ -206,14 +199,11 @@ func YQExpressions(flags *flag.FlagSet, newInstance bool) ([]string, error) {
 				if err != nil {
 					return nil, err
 				}
-				expr := `.dns += [`
+				ips := make([]string, len(ipSlice))
 				for i, ip := range ipSlice {
-					expr += fmt.Sprintf("%q", ip)
-					if i < len(ipSlice)-1 {
-						expr += ","
-					}
+					ips[i] = `"` + ip.String() + `"`
 				}
-				expr += `] | .dns |= unique | .hostResolver.enabled=false`
+				expr := fmt.Sprintf(".dns += [%s] | .dns |= unique | .hostResolver.enabled=false", strings.Join(ips, ","))
 				logrus.Warnf("Disabling HostResolver, as custom DNS addresses (%v) are specified", ipSlice)
 				return []string{expr}, nil
 			},
@@ -293,23 +283,20 @@ func YQExpressions(flags *flag.FlagSet, newInstance bool) ([]string, error) {
 				if err != nil {
 					return nil, err
 				}
-				expr := `.networks += [`
+				networks := make([]string, len(ss))
 				for i, s := range ss {
 					// CLI syntax is still experimental (YAML syntax is out of experimental)
 					switch {
 					case s == "vzNAT":
-						expr += `{"vzNAT": true}`
+						networks[i] = `{"vzNAT": true}`
 					case strings.HasPrefix(s, "lima:"):
 						network := strings.TrimPrefix(s, "lima:")
-						expr += fmt.Sprintf(`{"lima": %q}`, network)
+						networks[i] = fmt.Sprintf(`{"lima": %q}`, network)
 					default:
-						return nil, fmt.Errorf("network name must be \"vzNAT\" or \"lima:*\", got %q", s)
-					}
-					if i < len(ss)-1 {
-						expr += ","
+						return nil, fmt.Errorf(`network name must be "vzNAT" or "lima:*", got %q`, s)
 					}
 				}
-				expr += `] | .networks |= unique_by(.lima)`
+				expr := fmt.Sprintf(`.networks += [%s] | .networks |= unique_by(.lima)`, strings.Join(networks, ","))
 				return []string{expr}, nil
 			},
 			false,

cmd/limactl/editflags/editflags_test.go

@@ -195,6 +195,36 @@ func TestYQExpressions(t *testing.T) {
 			newInstance: false,
 			expectError: "flag `--mount` conflicts with `--mount-only`",
 		},
+		{
+			name:        "dns",
+			args:        []string{"--dns", "8.8.8.8", "--dns", "8.8.4.4", "--dns", "1.1.1.1"},
+			newInstance: false,
+			expected:    []string{`.dns += ["8.8.8.8","8.8.4.4","1.1.1.1"] | .dns |= unique | .hostResolver.enabled=false`},
+		},
+		{
+			name:        "network vzNAT",
+			args:        []string{"--network", "vzNAT"},
+			newInstance: true,
+			expected:    []string{`.networks += [{"vzNAT": true}] | .networks |= unique_by(.lima)`},
+		},
+		{
+			name:        "network lima:shared",
+			args:        []string{"--network", "lima:shared"},
+			newInstance: true,
+			expected:    []string{`.networks += [{"lima": "shared"}] | .networks |= unique_by(.lima)`},
+		},
+		{
+			name:        "multiple networks",
+			args:        []string{"--network", "vzNAT", "--network", "lima:shared", "--network", "lima:bridged"},
+			newInstance: true,
+			expected:    []string{`.networks += [{"vzNAT": true},{"lima": "shared"},{"lima": "bridged"}] | .networks |= unique_by(.lima)`},
+		},
+		{
+			name:        "invalid network",
+			args:        []string{"--network", "invalid"},
+			newInstance: true,
+			expectError: `network name must be "vzNAT" or "lima:*", got "invalid"`,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {

cmd/limactl/network.go

@@ -256,13 +256,11 @@ func networkDeleteAction(cmd *cobra.Command, args []string) error {
 		// Because the command currently does not check whether the network being removed is in use
 	}
 
-	var yq string
+	networks := make([]string, len(args))
 	for i, name := range args {
-		yq += fmt.Sprintf("del(.networks.%q)", name)
-		if i < len(args)-1 {
-			yq += " | "
-		}
+		networks[i] = fmt.Sprintf("del(.networks.%q)", name)
 	}
+	yq := strings.Join(networks, " | ")
 	return networkApplyYQ(yq)
 }
 

pkg/editutil/editutil.go

@@ -24,19 +24,19 @@ func fileWarning(filename string) string {
 	if err != nil || len(b) == 0 {
 		return ""
 	}
-	s := "# WARNING: " + filename + " includes the following settings,\n"
-	s += "# which are applied before applying this YAML:\n"
-	s += "# -----------\n"
+	var sb strings.Builder
+	sb.WriteString("# WARNING: " + filename + " includes the following settings,\n")
+	sb.WriteString("# which are applied before applying this YAML:\n")
+	sb.WriteString("# -----------\n")
 	for line := range strings.SplitSeq(strings.TrimSuffix(string(b), "\n"), "\n") {
-		s += "#"
+		sb.WriteByte('#')
 		if line != "" {
-			s += " " + line
+			sb.WriteString(" " + line)
 		}
-		s += "\n"
+		sb.WriteByte('\n')
 	}
-	s += "# -----------\n"
-	s += "\n"
-	return s
+	sb.WriteString("# -----------\n\n")
+	return sb.String()
 }
 
 // GenerateEditorWarningHeader generates the editor warning header.

pkg/hostagent/hostagent.go

@@ -500,7 +500,7 @@ func (a *HostAgent) startHostAgentRoutines(ctx context.Context) error {
 		msg := "Running in plain mode. Mounts, dynamic port forwarding, containerd, etc. will be ignored. Guest agent will not be running."
 		for _, port := range a.instConfig.PortForwards {
 			if port.Static {
-				msg += " Static port forwarding is allowed."
+				msg += " Static port forwarding is allowed." //nolint:modernize // stringsbuilder is not needed
 				break
 			}
 		}