Merge pull request #62 from sivchari/optionalorrequired-diagnostic

check if type spec has optional or required marker

Author: k8s-ci-robot

README.md

@@ -240,6 +240,8 @@ It also supports the `// +kubebuilder:validation:Optional` and `// +kubebuilder:
 
 If you prefer to use the Kubebuilder markers instead, you can change the preference in the configuration.
 
+The `optionalorrequired` linter also checks for the presence of optional or required markers on type declarations, and forbids this pattern.
+
 ### Configuration
 
 ```yaml

pkg/analysis/helpers/inspector/inspector.go

@@ -28,6 +28,9 @@ import (
 type Inspector interface {
 	// InspectFields is a function that iterates over fields in structs.
 	InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers))
+
+	// InspectTypeSpec is a function that inspects the type spec and calls the provided inspectTypeSpec function.
+	InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers))
 }
 
 // inspector implements the Inspector interface.
@@ -100,3 +103,19 @@ func (i *inspector) InspectFields(inspectField func(field *ast.Field, stack []as
 		return true
 	})
 }
+
+// InspectTypeSpec inspects the type spec and calls the provided inspectTypeSpec function.
+func (i *inspector) InspectTypeSpec(inspectTypeSpec func(typeSpec *ast.TypeSpec, markersAccess markers.Markers)) {
+	nodeFilter := []ast.Node{
+		(*ast.TypeSpec)(nil),
+	}
+
+	i.inspector.Preorder(nodeFilter, func(n ast.Node) {
+		typeSpec, ok := n.(*ast.TypeSpec)
+		if !ok {
+			return
+		}
+
+		inspectTypeSpec(typeSpec, i.markers)
+	})
+}

pkg/analysis/optionalorrequired/analyzer.go

@@ -102,6 +102,10 @@ func (a *analyzer) run(pass *analysis.Pass) (interface{}, error) {
 		a.checkField(pass, field, markersAccess.FieldMarkers(field), jsonTagInfo)
 	})
 
+	inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
+		a.checkTypeSpec(pass, typeSpec, markersAccess)
+	})
+
 	return nil, nil //nolint:nilnil
 }
 
@@ -211,6 +215,33 @@ func reportShouldRemoveSecondaryMarker(field *ast.Field, marker []markers.Marker
 	}
 }
 
+func (a *analyzer) checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
+	name := typeSpec.Name.Name
+	set := markersAccess.TypeMarkers(typeSpec)
+
+	for _, marker := range set.UnsortedList() {
+		switch marker.Identifier {
+		case a.primaryOptionalMarker, a.secondaryOptionalMarker, a.primaryRequiredMarker, a.secondaryRequiredMarker:
+			pass.Report(analysis.Diagnostic{
+				Pos:     typeSpec.Pos(),
+				Message: fmt.Sprintf("type %s should not be marked as %s", name, marker.String()),
+				SuggestedFixes: []analysis.SuggestedFix{
+					{
+						Message: fmt.Sprintf("should remove `// +%s`", marker.String()),
+						TextEdits: []analysis.TextEdit{
+							{
+								Pos:     marker.Pos,
+								End:     marker.End,
+								NewText: nil,
+							},
+						},
+					},
+				},
+			})
+		}
+	}
+}
+
 func defaultConfig(cfg *config.OptionalOrRequiredConfig) {
 	if cfg.PreferredOptionalMarker == "" {
 		cfg.PreferredOptionalMarker = OptionalMarker

pkg/analysis/optionalorrequired/analyzer_test.go

@@ -49,3 +49,14 @@ func TestSwappedMarkerPriority(t *testing.T) {
 
 	analysistest.RunWithSuggestedFixes(t, testdata, a, "b")
 }
+
+func TestTypeSpec(t *testing.T) {
+	testdata := analysistest.TestData()
+
+	a, err := optionalorrequired.Initializer().Init(config.LintersConfig{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	analysistest.RunWithSuggestedFixes(t, testdata, a, "c")
+}

pkg/analysis/optionalorrequired/doc.go

@@ -16,6 +16,8 @@ limitations under the License.
 
 /*
 optionalorrequired is a linter to ensure that all fields are marked as either optional or required.
+It also checks for the presence of optional or required markers on type declarations, and forbids this pattern.
+
 By default, it searches for the `+optional` and `+required` markers, and ensures that all fields are marked
 with at least one of these markers.
 

pkg/analysis/optionalorrequired/testdata/src/c/c.go

@@ -0,0 +1,27 @@
+package c
+
+type OptionalOrRequiredTestStruct struct {
+	RequiredEnumField RequiredEnum // want "field RequiredEnumField must be marked as optional or required"
+
+	KubebuilderRequiredEnumField KubeBuilderRequiredEnum // want "field KubebuilderRequiredEnumField must be marked as optional or required"
+
+	OptionalEnumField OptionalEnum // want "field OptionalEnumField must be marked as optional or required"
+
+	KubebuilderOptionalEnumField KubeBuilderOptionalEnum // want "field KubebuilderOptionalEnumField must be marked as optional or required"
+}
+
+// +required
+// +kubebuilder:validation:Enum=Foo;Bar;Baz
+type RequiredEnum string // want "type RequiredEnum should not be marked as required"
+
+// +kubebuilder:validation:Required
+// +kubebuilder:validation:Enum=Foo;Bar;Baz
+type KubeBuilderRequiredEnum string // want "type KubeBuilderRequiredEnum should not be marked as kubebuilder:validation:Required"
+
+// +optional
+// +kubebuilder:validation:Enum=Foo;Bar;Baz
+type OptionalEnum string // want "type OptionalEnum should not be marked as optional"
+
+// +kubebuilder:validation:Optional
+// +kubebuilder:validation:Enum=Foo;Bar;Baz
+type KubeBuilderOptionalEnum string // want "type KubeBuilderOptionalEnum should not be marked as kubebuilder:validation:Optional"

pkg/analysis/optionalorrequired/testdata/src/c/c.go.golden

@@ -0,0 +1,23 @@
+package c
+
+type OptionalOrRequiredTestStruct struct {
+	RequiredEnumField RequiredEnum // want "field RequiredEnumField must be marked as optional or required"
+
+	KubebuilderRequiredEnumField KubeBuilderRequiredEnum // want "field KubebuilderRequiredEnumField must be marked as optional or required"
+
+	OptionalEnumField OptionalEnum // want "field OptionalEnumField must be marked as optional or required"
+
+	KubebuilderOptionalEnumField KubeBuilderOptionalEnum // want "field KubebuilderOptionalEnumField must be marked as optional or required"
+}
+
+// +kubebuilder:validation:Enum=Foo;Bar;Baz
+type RequiredEnum string // want "type RequiredEnum should not be marked as required"
+
+// +kubebuilder:validation:Enum=Foo;Bar;Baz
+type KubeBuilderRequiredEnum string // want "type KubeBuilderRequiredEnum should not be marked as kubebuilder:validation:Required"
+
+// +kubebuilder:validation:Enum=Foo;Bar;Baz
+type OptionalEnum string // want "type OptionalEnum should not be marked as optional"
+
+// +kubebuilder:validation:Enum=Foo;Bar;Baz
+type KubeBuilderOptionalEnum string // want "type KubeBuilderOptionalEnum should not be marked as kubebuilder:validation:Optional"