Fix function calls with inconsistent argument breaking (#932)

Problem:
- The code doesn't follow the guideline: when breaking up a
call, choose to break after each parameter argument. See
https://github.com/nginxinc/nginx-kubernetes-gateway/blob/0ff13226b16cfec44f509542b25bc3f2fcf610cc/docs/developer/go-style-guide.md#consistent-line-breaks
- THe guidelines don't have an example of a function call

Solution:
- Fix the problems in the code, except for ginkgo helpers in tests,
because of their readability.
- Add an example of a functional call to the guidelines

Author: pleshakov

cmd/gateway/commands.go

@@ -124,7 +124,8 @@ func createStaticModeCommand() *cobra.Command {
 		Short: "Configure NGINX in the scope of a single Gateway resource",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			logger := zap.New()
-			logger.Info("Starting NGINX Kubernetes Gateway in static mode",
+			logger.Info(
+				"Starting NGINX Kubernetes Gateway in static mode",
 				"version", version,
 				"commit", commit,
 				"date", date,
@@ -184,7 +185,8 @@ func createProvisionerModeCommand() *cobra.Command {
 		Hidden: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			logger := zap.New()
-			logger.Info("Starting NGINX Kubernetes Gateway Provisioner",
+			logger.Info(
+				"Starting NGINX Kubernetes Gateway Provisioner",
 				"version", version,
 				"commit", commit,
 				"date", date,

docs/developer/go-style-guide.md

@@ -45,20 +45,27 @@ signaller <- false // is this a signal? is this an error?
 
 ### Consistent Line Breaks
 
-When breaking up a long function definition, call, or struct initialization, choose to break after each parameter,
+When breaking up a (1) long function definition, (2) call, or (3) struct initialization, choose to break after each parameter,
 argument, or field.
 
 DO:
 
 ```go
+// 1
 func longFunctionDefinition(
   paramX int,
   paramY string,
   paramZ bool,
 ) (string, error){}
 
-// and
+// 2
+callWithManyArguments(
+    arg1,
+    arg2,
+    arg3,
+)
 
+// 3
 s := myStruct{
   field1: 1,
   field2: 2,
@@ -69,6 +76,7 @@ s := myStruct{
 DO NOT:
 
 ```go
+// 1
 func longFunctionDefinition(paramX int, paramY string,
   paramZ bool,
 ) (string, error){}
@@ -80,12 +88,25 @@ func longFunctionDefinition(
   paramZ bool,
 ) (string, error){}
 
-// or
+// 2
+callWithManyArguments(arg1, arg2,
+    arg3)
+
+// 3
 s := myStruct{field1: 1, field2: 2,
 field3: 3}
 
 ```
 
+> **Exception**: Calls to ginkgo helper functions like below in tests are OK (because of the readability):
+>
+>```go
+>DescribeTable("Edge cases for events",
+>    func(e interface{}) {
+>    . . .
+>)
+>```
+
 When constructing structs pass members during initialization.
 
 Example:

internal/framework/status/updater.go

@@ -133,7 +133,9 @@ func (upd *updaterImpl) update(
 	err := upd.cfg.Client.Get(ctx, nsname, obj)
 	if err != nil {
 		if !apierrors.IsNotFound(err) {
-			upd.cfg.Logger.Error(err, "Failed to get the recent version the resource when updating status",
+			upd.cfg.Logger.Error(
+				err,
+				"Failed to get the recent version the resource when updating status",
 				"namespace", nsname.Namespace,
 				"name", nsname.Name,
 				"kind", obj.GetObjectKind().GroupVersionKind().Kind)
@@ -145,7 +147,9 @@ func (upd *updaterImpl) update(
 
 	err = upd.cfg.Client.Status().Update(ctx, obj)
 	if err != nil {
-		upd.cfg.Logger.Error(err, "Failed to update status",
+		upd.cfg.Logger.Error(
+			err,
+			"Failed to update status",
 			"namespace", nsname.Namespace,
 			"name", nsname.Name,
 			"kind", obj.GetObjectKind().GroupVersionKind().Kind)

internal/mode/provisioner/handler.go

@@ -117,7 +117,8 @@ func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context) {
 
 		h.provisions[nsname] = deployment
 
-		h.logger.Info("Created deployment",
+		h.logger.Info(
+			"Created deployment",
 			"deployment", client.ObjectKeyFromObject(deployment),
 			"gateway", nsname,
 		)
@@ -135,7 +136,8 @@ func (h *eventHandler) ensureDeploymentsMatchGateways(ctx context.Context) {
 
 		delete(h.provisions, nsname)
 
-		h.logger.Info("Deleted deployment",
+		h.logger.Info(
+			"Deleted deployment",
 			"deployment", client.ObjectKeyFromObject(deployment),
 			"gateway", nsname,
 		)

internal/mode/static/nginx/config/validation/common.go

@@ -41,8 +41,11 @@ var escapedStringsNoVarExpansionFmtRegexp = regexp.MustCompile("^" + escapedStri
 // If the value is invalid, the function returns an error that includes the specified examples of valid values.
 func validateEscapedStringNoVarExpansion(value string, examples []string) error {
 	if !escapedStringsNoVarExpansionFmtRegexp.MatchString(value) {
-		msg := k8svalidation.RegexError(escapedStringsNoVarExpansionErrMsg, escapedStringsNoVarExpansionFmt,
-			examples...)
+		msg := k8svalidation.RegexError(
+			escapedStringsNoVarExpansionErrMsg,
+			escapedStringsNoVarExpansionFmt,
+			examples...,
+		)
 		return errors.New(msg)
 	}
 	return nil

internal/mode/static/nginx/config/validation/common_test.go

@@ -8,39 +8,56 @@ import (
 func TestValidateEscapedString(t *testing.T) {
 	validator := func(value string) error { return validateEscapedString(value, []string{"example"}) }
 
-	testValidValuesForSimpleValidator(t, validator,
+	testValidValuesForSimpleValidator(
+		t,
+		validator,
 		`test`,
 		`test test`,
 		`\"`,
-		`\\`)
-	testInvalidValuesForSimpleValidator(t, validator,
+		`\\`,
+	)
+	testInvalidValuesForSimpleValidator(
+		t,
+		validator,
 		`\`,
-		`test"test`)
+		`test"test`,
+	)
 }
 
 func TestValidateEscapedStringNoVarExpansion(t *testing.T) {
 	validator := func(value string) error { return validateEscapedStringNoVarExpansion(value, []string{"example"}) }
 
-	testValidValuesForSimpleValidator(t, validator,
+	testValidValuesForSimpleValidator(
+		t,
+		validator,
 		`test`,
 		`test test`,
 		`\"`,
-		`\\`)
-	testInvalidValuesForSimpleValidator(t, validator,
+		`\\`,
+	)
+	testInvalidValuesForSimpleValidator(
+		t,
+		validator,
 		`\`,
 		`test"test`,
-		`$test`)
+		`$test`,
+	)
 }
 
 func TestValidateValidHeaderName(t *testing.T) {
 	validator := func(value string) error { return validateHeaderName(value) }
 
-	testValidValuesForSimpleValidator(t, validator,
+	testValidValuesForSimpleValidator(
+		t,
+		validator,
 		`Content-Encoding`,
 		`X-Forwarded-For`,
 		// max supported length is 256, generate string with 16*16 chars (256)
-		strings.Repeat("very-long-header", 16))
-	testInvalidValuesForSimpleValidator(t, validator,
+		strings.Repeat("very-long-header", 16),
+	)
+	testInvalidValuesForSimpleValidator(
+		t,
+		validator,
 		`\`,
 		`test test`,
 		`test"test`,
@@ -49,5 +66,6 @@ func TestValidateValidHeaderName(t *testing.T) {
 		"host",
 		"my-header[]",
 		"my-header&",
-		strings.Repeat("very-long-header", 16)+"1")
+		strings.Repeat("very-long-header", 16)+"1",
+	)
 }

internal/mode/static/nginx/config/validation/framework_test.go

@@ -43,11 +43,16 @@ func testValidValuesForSupportedValuesValidator[T configValue](
 	f supportedValuesValidatorFunc[T],
 	values ...T,
 ) {
-	runValidatorTests(t, func(g *WithT, v T) {
-		valid, supportedValues := f(v)
-		g.Expect(valid).To(BeTrue(), createFailureMessage(v))
-		g.Expect(supportedValues).To(BeNil(), createFailureMessage(v))
-	}, "valid_value", values...)
+	runValidatorTests(
+		t,
+		func(g *WithT, v T) {
+			valid, supportedValues := f(v)
+			g.Expect(valid).To(BeTrue(), createFailureMessage(v))
+			g.Expect(supportedValues).To(BeNil(), createFailureMessage(v))
+		},
+		"valid_value",
+		values...,
+	)
 }
 
 func testInvalidValuesForSupportedValuesValidator[T configValue](
@@ -56,11 +61,16 @@ func testInvalidValuesForSupportedValuesValidator[T configValue](
 	supportedValuesMap map[T]struct{},
 	values ...T,
 ) {
-	runValidatorTests(t, func(g *WithT, v T) {
-		valid, supportedValues := f(v)
-		g.Expect(valid).To(BeFalse(), createFailureMessage(v))
-		g.Expect(supportedValues).To(Equal(getSortedKeysAsString(supportedValuesMap)), createFailureMessage(v))
-	}, "invalid_value", values...)
+	runValidatorTests(
+		t,
+		func(g *WithT, v T) {
+			valid, supportedValues := f(v)
+			g.Expect(valid).To(BeFalse(), createFailureMessage(v))
+			g.Expect(supportedValues).To(Equal(getSortedKeysAsString(supportedValuesMap)), createFailureMessage(v))
+		},
+		"invalid_value",
+		values...,
+	)
 }
 
 func TestValidateInSupportedValues(t *testing.T) {
@@ -74,12 +84,19 @@ func TestValidateInSupportedValues(t *testing.T) {
 		return validateInSupportedValues(value, supportedValues)
 	}
 
-	testValidValuesForSupportedValuesValidator(t, validator,
+	testValidValuesForSupportedValuesValidator(
+		t,
+		validator,
 		"value1",
 		"value2",
-		"value3")
-	testInvalidValuesForSupportedValuesValidator(t, validator, supportedValues,
-		"value4")
+		"value3",
+	)
+	testInvalidValuesForSupportedValuesValidator(
+		t,
+		validator,
+		supportedValues,
+		"value4",
+	)
 }
 
 func TestGetSortedKeysAsString(t *testing.T) {