review: updates based on review from Joel

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

Author: everettraven

docs/linters.md

@@ -318,6 +318,7 @@ Naming conventions must have:
 - A human-readable message to be included in violation errors.
 - A regular expression that will match text within the field name that violates the convention.
 - A defined "operation". Allowed operations are `Inform`, `Drop`, `DropField`, and `Replace`.
+
 The `Inform` operation will simply inform when a field name violates the naming convention.
 The `Drop` operation will suggest a fix that drops violating text from the field name.
 The `DropField` operation will suggest a fix that removes the field in it's entirety.
@@ -374,8 +375,8 @@ linterConfig:
 ```yaml
 linterConfig:
   namingconventions:
-    conventions: 
-      - name: englishcolour 
+    conventions:
+      - name: BritishEnglishColour 
         violationMatcher: (?i)color
         operation: Replace
         replace: colour

pkg/analysis/namingconventions/analyzer.go

@@ -93,7 +93,7 @@ func checkField(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.F
 		case OperationDrop:
 			reportDrop(pass, field, tagInfo, convention, matcher)
 
-		case OperationReplace:
+		case OperationReplacement:
 			reportReplace(pass, field, tagInfo, convention, matcher)
 		}
 	}
@@ -102,23 +102,24 @@ func checkField(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.F
 func reportConventionWithSuggestedFixes(pass *analysis.Pass, field *ast.Field, convention Convention, suggestedFixes ...analysis.SuggestedFix) {
 	pass.Report(analysis.Diagnostic{
 		Pos:            field.Pos(),
-		Message:        fmt.Sprintf("naming convention %q: %s", convention.Name, convention.Message),
+		Message:        fmt.Sprintf("naming convention %q: field %s: %s", convention.Name, utils.FieldName(field), convention.Message),
 		SuggestedFixes: suggestedFixes,
 	})
 }
 
 func reportDropField(pass *analysis.Pass, field *ast.Field, convention Convention) {
-	suggestedFixes := []analysis.SuggestedFix{}
-	suggestedFixes = append(suggestedFixes, analysis.SuggestedFix{
-		Message: "remove the field",
-		TextEdits: []analysis.TextEdit{
-			{
-				Pos:     field.Pos(),
-				NewText: []byte(""),
-				End:     field.End(),
+	suggestedFixes := []analysis.SuggestedFix{
+		{
+			Message: "remove the field",
+			TextEdits: []analysis.TextEdit{
+				{
+					Pos:     field.Pos(),
+					NewText: []byte(""),
+					End:     field.End(),
+				},
 			},
 		},
-	})
+	}
 
 	reportConventionWithSuggestedFixes(pass, field, convention, suggestedFixes...)
 }
@@ -129,7 +130,7 @@ func reportDrop(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.F
 }
 
 func reportReplace(pass *analysis.Pass, field *ast.Field, tagInfo extractjsontags.FieldTagInfo, convention Convention, matcher *regexp.Regexp) {
-	suggestedFixes := suggestedFixesForReplacement(field, tagInfo, matcher, convention.Replace)
+	suggestedFixes := suggestedFixesForReplacement(field, tagInfo, matcher, convention.Replacement)
 	reportConventionWithSuggestedFixes(pass, field, convention, suggestedFixes...)
 }
 
@@ -151,8 +152,6 @@ func suggestFixesForSerializedFieldName(tagInfo extractjsontags.FieldTagInfo, ma
 		return nil
 	}
 
-	suggestedFixes := []analysis.SuggestedFix{}
-
 	// This should prevent panics from slice access when the replacement
 	// string ends up being a length of 1 and still result in a technically
 	// correct JSON tag name value.
@@ -169,18 +168,18 @@ func suggestFixesForSerializedFieldName(tagInfo extractjsontags.FieldTagInfo, ma
 		tagReplacementMessage = "remove offending text from serialized field name"
 	}
 
-	suggestedFixes = append(suggestedFixes, analysis.SuggestedFix{
-		Message: tagReplacementMessage,
-		TextEdits: []analysis.TextEdit{
-			{
-				Pos:     tagInfo.Pos,
-				NewText: []byte(tagReplacement),
-				End:     tagInfo.End,
+	return []analysis.SuggestedFix{
+		{
+			Message: tagReplacementMessage,
+			TextEdits: []analysis.TextEdit{
+				{
+					Pos:     tagInfo.Pos,
+					NewText: []byte(tagReplacement),
+					End:     tagInfo.End,
+				},
 			},
 		},
-	})
-
-	return suggestedFixes
+	}
 }
 
 func suggestFixesForGoFieldName(field *ast.Field, matcher *regexp.Regexp, replacementStr string) []analysis.SuggestedFix {
@@ -193,24 +192,22 @@ func suggestFixesForGoFieldName(field *ast.Field, matcher *regexp.Regexp, replac
 		return nil
 	}
 
-	suggestedFixes := []analysis.SuggestedFix{}
-
 	replacementMessage := fmt.Sprintf("replace offending text in Go type with %q", replacementStr)
 
 	if len(replacementStr) == 0 {
 		replacementMessage = "remove offending text from Go type field"
 	}
 
-	suggestedFixes = append(suggestedFixes, analysis.SuggestedFix{
-		Message: replacementMessage,
-		TextEdits: []analysis.TextEdit{
-			{
-				Pos:     field.Pos(),
-				NewText: []byte(replacement),
-				End:     field.Pos() + token.Pos(len(fieldName)),
+	return []analysis.SuggestedFix{
+		{
+			Message: replacementMessage,
+			TextEdits: []analysis.TextEdit{
+				{
+					Pos:     field.Pos(),
+					NewText: []byte(replacement),
+					End:     field.Pos() + token.Pos(len(fieldName)),
+				},
 			},
 		},
-	})
-
-	return suggestedFixes
+	}
 }

pkg/analysis/namingconventions/analyzer_test.go

@@ -36,9 +36,9 @@ func Test(t *testing.T) {
 			{
 				Name:             "preferbehaviour",
 				ViolationMatcher: "(?i)behavior",
-				Operation:        namingconventions.OperationReplace,
+				Operation:        namingconventions.OperationReplacement,
 				Message:          "prefer the use of the word 'behaviour' instead of 'behavior'.",
-				Replace:          "Behaviour",
+				Replacement:      "Behaviour",
 			},
 			{
 				Name:             "nounsupported",

pkg/analysis/namingconventions/config.go

@@ -30,9 +30,10 @@ type Config struct {
 type Convention struct {
 	// name is a required human-readable name to
 	// associate with the field naming convention.
+	// name is case-sensitive.
 	Name string `json:"name,omitempty"`
 
-	// violationMatcher is a required regular expression
+	// violationMatcher is a required RE2 compliant regular expression
 	// used to identify violating portions of a field name.
 	ViolationMatcher string `json:"violationMatcher,omitempty"`
 
@@ -55,18 +56,18 @@ type Convention struct {
 	// portion of the field name matched by the violationMatcher
 	// expression.
 	//
-	// When set to Replace, the namingconventions linter will
+	// When set to Replacement, the namingconventions linter will
 	// suggest that any fields matching this conventions should
 	// replace the matched portion of the field name with the value
 	// specified in the replace field.
 	Operation Operation `json:"operation,omitempty"`
 
-	// replace configures the string that should
+	// replacement configures the string that should
 	// replace the matched portion of a field name
 	// that violates this conventions.
-	// replace is required when operation is set to Replace
+	// replacement is required when operation is set to Replacement
 	// and forbidden otherwise.
-	Replace string `json:"replace,omitempty,omitzero"`
+	Replacement string `json:"replacement,omitempty,omitzero"`
 
 	// message is a required human-readable message
 	// to be included in the linter error if a field
@@ -88,10 +89,10 @@ const (
 	// the naming convention is violated.
 	OperationDrop Operation = "Drop"
 
-	// OperationReplace signals that the offending text
+	// OperationReplacement signals that the offending text
 	// should be replaced in the field name when
 	// the naming convention is violated.
-	OperationReplace Operation = "Replace"
+	OperationReplacement Operation = "Replacement"
 
 	// OperationInform signals that no action
 	// should be taken, beyond issuing a linter error

pkg/analysis/namingconventions/doc.go

@@ -26,6 +26,7 @@ Naming conventions must have:
 - A human-readable message to be included in violation errors.
 - A regular expression that will match text within the field name that violates the convention.
 - A defined "operation". Allowed operations are "Inform", "Drop", "DropField", and "Replace".
+
 The "Inform" operation will simply inform via a linter error when a field name violates the naming convention.
 The "Drop" operation will suggest a fix that drops violating text from the field name.
 The "DropField" operation will suggest a fix that removes the field in it's entirety.
@@ -78,10 +79,10 @@ linterConfig:
 
 	namingconventions:
 	  conventions:
-	    - name: englishcolour
+	    - name: BritishEnglishColour
 	      violationMatcher: (?i)color
-	      operation: Replace
-	      replace: colour
+	      operation: Replacement
+	      replacement: colour
 	      message: prefer 'colour' over 'color' when referring to colours in field names
 
 ```

pkg/analysis/namingconventions/initializer.go

@@ -76,7 +76,6 @@ func validateConventions(fldPath *field.Path, conventions ...Convention) field.E
 	return fieldErrs
 }
 
-// The cyclop linter says this has cyclomatic complexity of 15.
 func validateConvention(fldPath *field.Path, convention Convention) field.ErrorList {
 	fieldErrs := field.ErrorList{}
 
@@ -98,32 +97,32 @@ func validateConvention(fldPath *field.Path, convention Convention) field.ErrorL
 	}
 
 	fieldErrs = append(fieldErrs, validateOperation(fldPath.Child("operation"), convention.Operation)...)
-	fieldErrs = append(fieldErrs, validateReplace(fldPath.Child("replace"), convention.Operation, matcher, convention.Replace)...)
+	fieldErrs = append(fieldErrs, validateReplace(fldPath.Child("replacement"), convention.Operation, matcher, convention.Replacement)...)
 
 	return fieldErrs
 }
 
 func validateOperation(fldPath *field.Path, operation Operation) field.ErrorList {
-	allowedOperations := sets.New(OperationDrop, OperationInform, OperationReplace, OperationDropField)
+	allowedOperations := sets.New(OperationDrop, OperationInform, OperationReplacement, OperationDropField)
 
 	if len(operation) == 0 {
 		return field.ErrorList{field.Required(fldPath, "operation is required")}
 	}
 
 	if len(operation) > 0 && !allowedOperations.Has(operation) {
-		return field.ErrorList{field.Invalid(fldPath, operation, fmt.Sprintf("operation must be one of %q, %q, %q, or %q", OperationInform, OperationDrop, OperationDropField, OperationReplace))}
+		return field.ErrorList{field.Invalid(fldPath, operation, fmt.Sprintf("operation must be one of %q, %q, %q, or %q", OperationInform, OperationDrop, OperationDropField, OperationReplacement))}
 	}
 
 	return field.ErrorList{}
 }
 
 func validateReplace(fldPath *field.Path, operation Operation, matcher *regexp.Regexp, replace string) field.ErrorList {
-	if (len(replace) > 0 && operation != OperationReplace) || (len(replace) == 0 && operation == OperationReplace) {
-		return field.ErrorList{field.Invalid(fldPath, replace, "replace must be specified when operation is 'Replace' and is forbidden otherwise")}
+	if (len(replace) > 0 && operation != OperationReplacement) || (len(replace) == 0 && operation == OperationReplacement) {
+		return field.ErrorList{field.Invalid(fldPath, replace, "replacement must be specified when operation is 'Replacement' and is forbidden otherwise")}
 	}
 
-	if len(replace) > 0 && operation == OperationReplace && matcher.MatchString(replace) {
-		return field.ErrorList{field.Invalid(fldPath, replace, "replace must not match violationMatcher")}
+	if len(replace) > 0 && operation == OperationReplacement && matcher.MatchString(replace) {
+		return field.ErrorList{field.Invalid(fldPath, replace, "replacement must not be matched by violationMatcher")}
 	}
 
 	return field.ErrorList{}

pkg/analysis/namingconventions/initializer_test.go

@@ -151,48 +151,48 @@ var _ = Describe("namingconventions initializer", func() {
 						},
 					},
 				},
-				expectedErr: "namingconventions.conventions[0].operation: Invalid value: \"Unknown\": operation must be one of \"Inform\", \"Drop\", \"DropField\", or \"Replace\"",
+				expectedErr: "namingconventions.conventions[0].operation: Invalid value: \"Unknown\": operation must be one of \"Inform\", \"Drop\", \"DropField\", or \"Replacement\"",
 			}),
-			Entry("With an invalid namingconventions configuration with a replace when operation is not 'Replace'", testCase{
+			Entry("With an invalid namingconventions configuration with a replacement when operation is not 'replacement'", testCase{
 				config: &namingconventions.Config{
 					Conventions: []namingconventions.Convention{
 						{
 							Name:             "nothing",
 							ViolationMatcher: "(?i)thing",
 							Operation:        namingconventions.OperationDrop,
 							Message:          "no fields should have any variations of the word 'thing' in their name",
-							Replace:          "item",
+							Replacement:      "item",
 						},
 					},
 				},
-				expectedErr: "namingconventions.conventions[0].replace: Invalid value: \"item\": replace must be specified when operation is 'Replace' and is forbidden otherwise",
+				expectedErr: "namingconventions.conventions[0].replacement: Invalid value: \"item\": replacement must be specified when operation is 'Replacement' and is forbidden otherwise",
 			}),
-			Entry("With an invalid namingconventions configuration with no replace when operation is 'Replace'", testCase{
+			Entry("With an invalid namingconventions configuration with no replacement when operation is 'replacement'", testCase{
 				config: &namingconventions.Config{
 					Conventions: []namingconventions.Convention{
 						{
 							Name:             "nothing",
 							ViolationMatcher: "(?i)thing",
-							Operation:        namingconventions.OperationReplace,
+							Operation:        namingconventions.OperationReplacement,
 							Message:          "no fields should have any variations of the word 'thing' in their name",
 						},
 					},
 				},
-				expectedErr: "namingconventions.conventions[0].replace: Invalid value: \"\": replace must be specified when operation is 'Replace' and is forbidden otherwise",
+				expectedErr: "namingconventions.conventions[0].replacement: Invalid value: \"\": replacement must be specified when operation is 'Replacement' and is forbidden otherwise",
 			}),
 			Entry("With an invalid namingconventions configuration where replacement string matches violationMatcher", testCase{
 				config: &namingconventions.Config{
 					Conventions: []namingconventions.Convention{
 						{
 							Name:             "nothing",
 							ViolationMatcher: "(?i)thing",
-							Operation:        namingconventions.OperationReplace,
+							Operation:        namingconventions.OperationReplacement,
 							Message:          "no fields should have any variations of the word 'thing' in their name",
-							Replace:          "anotherthing",
+							Replacement:      "anotherthing",
 						},
 					},
 				},
-				expectedErr: "namingconventions.conventions[0].replace: Invalid value: \"anotherthing\": replace must not match violationMatcher",
+				expectedErr: "namingconventions.conventions[0].replacement: Invalid value: \"anotherthing\": replacement must not be matched by violationMatcher",
 			}),
 		)
 	})

pkg/analysis/namingconventions/namingconventions_suite_test.go

@@ -22,7 +22,7 @@ import (
 	. "github.com/onsi/gomega"
 )
 
-func TestConditions(t *testing.T) {
+func TestNamingConventions(t *testing.T) {
 	RegisterFailHandler(Fail)
 	RunSpecs(t, "namingconventions")
 }

pkg/analysis/namingconventions/testdata/src/a/a.go

@@ -2,12 +2,12 @@ package a
 
 // Shouldn't care about Go types
 type BasketOfFruit struct {
-	RedFruit    []string `json:"redFruit,omitempty"`    // want `naming convention "nofruit": no fields should contain any variations of the word 'fruit' in their name`
-	OrangeFruit []string `json:"orangeFruit,omitempty"` // want `naming convention "nofruit": no fields should contain any variations of the word 'fruit' in their name`
-	GreenFruit  []string `json:"greenFruit,omitempty"`  // want `naming convention "nofruit": no fields should contain any variations of the word 'fruit' in their name`
-	FruitBlue   []string `json:"fruitBlue,omitempty"`   // want `naming convention "nofruit": no fields should contain any variations of the word 'fruit' in their name`
-	Fruit       []string `json:"fruit,omitempty"`       // want `naming convention "nofruit": no fields should contain any variations of the word 'fruit' in their name`
-	AFruit      string   `json:"aFruit,omitempty"`      // want `naming convention "nofruit": no fields should contain any variations of the word 'fruit' in their name`
+	RedFruit    []string `json:"redFruit,omitempty"`    // want `naming convention "nofruit": field RedFruit: no fields should contain any variations of the word 'fruit' in their name`
+	OrangeFruit []string `json:"orangeFruit,omitempty"` // want `naming convention "nofruit": field OrangeFruit: no fields should contain any variations of the word 'fruit' in their name`
+	GreenFruit  []string `json:"greenFruit,omitempty"`  // want `naming convention "nofruit": field GreenFruit: no fields should contain any variations of the word 'fruit' in their name`
+	FruitBlue   []string `json:"fruitBlue,omitempty"`   // want `naming convention "nofruit": field FruitBlue: no fields should contain any variations of the word 'fruit' in their name`
+	Fruit       []string `json:"fruit,omitempty"`       // want `naming convention "nofruit": field Fruit: no fields should contain any variations of the word 'fruit' in their name`
+	AFruit      string   `json:"aFruit,omitempty"`      // want `naming convention "nofruit": field AFruit: no fields should contain any variations of the word 'fruit' in their name`
 }
 
 // Shouldn't care about methods
@@ -21,16 +21,16 @@ func IsFruit(in string) {
 }
 
 type SpecialBehaviors struct {
-	SomethingBehavior string `json:"somethingBehavior,omitempty"` // want `naming convention "preferbehaviour": prefer the use of the word 'behaviour' instead of 'behavior'.`
-	BehaviorCrazy     bool   `json:"behaviorCrazy,omitempty"`     // want `naming convention "preferbehaviour": prefer the use of the word 'behaviour' instead of 'behavior'.`
+	SomethingBehavior string `json:"somethingBehavior,omitempty"` // want `naming convention "preferbehaviour": field SomethingBehavior: prefer the use of the word 'behaviour' instead of 'behavior'.`
+	BehaviorCrazy     bool   `json:"behaviorCrazy,omitempty"`     // want `naming convention "preferbehaviour": field BehaviorCrazy: prefer the use of the word 'behaviour' instead of 'behavior'.`
 }
 
 type Configurations struct {
 	SomeSupportedThingy string `json:"someSupportedThingy,omitempty"`
-	UnsupportedThingy   string `json:"unsupportedThingy,omitempty"` // want `naming convention "nounsupported": no fields allowing for unsupported behaviors allowed`
+	UnsupportedThingy   string `json:"unsupportedThingy,omitempty"` // want `naming convention "nounsupported": field UnsupportedThingy: no fields allowing for unsupported behaviors allowed`
 }
 
 type TestSet struct {
-	TestName string `json:"testName,omitempty"`  // want `naming convention "notest": no temporary test fields`
-	Other    string `json:"otherTest,omitempty"` // want `naming convention "notest": no temporary test fields`
+	TestName string `json:"testName,omitempty"`  // want `naming convention "notest": field TestName: no temporary test fields`
+	Other    string `json:"otherTest,omitempty"` // want `naming convention "notest": field Other: no temporary test fields`
 }