Implement optional enforcement of omitempty on optional fields

Author: JoelSpeed

pkg/analysis/optionalfields/analyzer.go

@@ -35,3 +35,23 @@ func TestWhenRequiredPreferenceConfiguration(t *testing.T) {
 
 	analysistest.RunWithSuggestedFixes(t, testdata, a, "b")
 }
+
+func TestWhenRequiredWithOmitEmptyIgnorePreferenceConfiguration(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	a, err := requiredfields.Initializer().Init(config.LintersConfig{
+		OptionalFields: config.OptionalFieldsConfig{
+			Pointers: config.OptionalFieldsPointers{
+				Preference: config.OptionalFieldsPointerPreferenceWhenRequired,
+			},
+			OmitEmpty: config.OptionalFieldsOmitEmpty{
+				Policy: config.OptionalFieldsOmitEmptyPolicyIgnore,
+			},
+		},
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	analysistest.RunWithSuggestedFixes(t, testdata, a, "c")
+}

pkg/analysis/optionalfields/analyzer_test.go

@@ -21,14 +21,26 @@ type A struct {
 	// +optional
 	String string `json:"string,omitempty"` // want "field String is optional and should be a pointer"
 
+	// NonOmittedString is a string field without omitempty
+	// +optional
+	NonOmittedString string `json:"nonOmittedString"` // want "field NonOmittedString is optional and should be a pointer" "field NonOmittedString is optional and should be omitempty"
+
 	// int is an int field.
 	// +optional
 	Int int `json:"int,omitempty"` // want "field Int is optional and should be a pointer"
 
+	// nonOmittedInt is an int field without omitempty
+	// +optional
+	NonOmittedInt int `json:"nonOmittedInt"` // want "field NonOmittedInt is optional and should be a pointer" "field NonOmittedInt is optional and should be omitempty"
+
 	// struct is a struct field.
 	// +optional
 	Struct B `json:"struct,omitempty"` // want "field Struct is optional and should be a pointer"
 
+	// nonOmittedStruct is a struct field without omitempty.
+	// +optional
+	NonOmittedStruct B `json:"nonOmittedStruct"` // want "field NonOmittedStruct is optional and should be a pointer" "field NonOmittedStruct is optional and should be omitempty"
+
 	// slice is a slice field.
 	// +optional
 	Slice []string `json:"slice,omitempty"`

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

@@ -21,14 +21,26 @@ type A struct {
 	// +optional
 	String *string `json:"string,omitempty"` // want "field String is optional and should be a pointer"
 
+	// NonOmittedString is a string field without omitempty
+	// +optional
+	NonOmittedString *string `json:"nonOmittedString,omitempty"` // want "field NonOmittedString is optional and should be a pointer" "field NonOmittedString is optional and should be omitempty"
+
 	// int is an int field.
 	// +optional
 	Int *int `json:"int,omitempty"` // want "field Int is optional and should be a pointer"
 
+	// nonOmittedInt is an int field without omitempty
+	// +optional
+	NonOmittedInt *int `json:"nonOmittedInt,omitempty"` // want "field NonOmittedInt is optional and should be a pointer" "field NonOmittedInt is optional and should be omitempty"
+
 	// struct is a struct field.
 	// +optional
 	Struct *B `json:"struct,omitempty"` // want "field Struct is optional and should be a pointer"
 
+    // nonOmittedStruct is a struct field without omitempty.
+	// +optional
+	NonOmittedStruct *B `json:"nonOmittedStruct,omitempty"` // want "field NonOmittedStruct is optional and should be a pointer" "field NonOmittedStruct is optional and should be omitempty"
+
 	// slice is a slice field.
 	// +optional
 	Slice []string `json:"slice,omitempty"`

pkg/analysis/optionalfields/testdata/src/a/a.go.golden

@@ -151,6 +151,11 @@ type OptionalFieldsConfig struct {
 	// This defines how the linter should handle optional fields, and whether they should be pointers or not.
 	// By default, all fields will be expected to be pointers, and the linter will suggest fixes if they are not.
 	Pointers OptionalFieldsPointers `json:"pointers"`
+
+	// omitempty is the policy for the `omitempty` tag within the json tag for fields.
+	// This defines how the linter should handle optional fields, and whether they should have the omitempty tag or not.
+	// By default, all fields will be expected to have the `omitempty` tag.
+	OmitEmpty OptionalFieldsOmitEmpty `json:"omitempty"`
 }
 
 type OptionalFieldsPointers struct {
@@ -170,6 +175,15 @@ type OptionalFieldsPointers struct {
 	Policy OptionalFieldsPointerPolicy `json:"policy"`
 }
 
+type OptionalFieldsOmitEmpty struct {
+	// policy determines whether the linter should require omitempty for all optional fields.
+	// Valid values are "SuggestFix" and "Ignore".
+	// When set to "SuggestFix", the linter will suggest adding the `omitempty` tag when an optional field does not have it.
+	// When set to "Ignore", and optional field missing the `omitempty` tag will be ignored.
+	// Note, when set to "Ignore", and a field does not have the `omitempty` tag, this may affect whether the field should be a pointer or not.
+	Policy OptionalFieldsOmitEmptyPolicy `json:"policy"`
+}
+
 // OptionalFieldsPointerPreference is the preference for pointers in optional fields.
 type OptionalFieldsPointerPreference string
 
@@ -192,6 +206,17 @@ const (
 	OptionalFieldsPointerPolicyWarn OptionalFieldsPointerPolicy = "Warn"
 )
 
+// OptionalFieldsOmitEmptyPolicy is the policy for the omitempty tag on optional fields.
+type OptionalFieldsOmitEmptyPolicy string
+
+const (
+	// OptionalFieldsOmitEmptyPolicySuggestFix indicates that the linter will emit a warning if the field does not have omitempty, and suggest a fix.
+	OptionalFieldsOmitEmptyPolicySuggestFix OptionalFieldsOmitEmptyPolicy = "SuggestFix"
+
+	// OptionalFieldsOmitEmptyPolicyIgnore indicates that the linter will ignore any field missing the omitempty tag.
+	OptionalFieldsOmitEmptyPolicyIgnore OptionalFieldsOmitEmptyPolicy = "Ignore"
+)
+
 // OptionalOrRequiredConfig contains configuration for the optionalorrequired linter.
 type OptionalOrRequiredConfig struct {
 	// preferredOptionalMarker is the preferred marker to use for optional fields.