Merge pull request #94 from everettraven/feature/uniquemarkers

add uniquemarkers linter

Author: k8s-ci-robot

README.md

@@ -415,6 +415,36 @@ This linter is not enabled by default as it is only applicable to CustomResource
 In the case where there is a status field present but no `kubebuilder:subresource:status` marker, the
 linter will suggest adding the comment `// +kubebuilder:subresource:status` above the struct.
 
+## UniqueMarkers
+
+The `uniquemarkers` linter ensures that types and fields do not contain more than a single definition of a marker that should only be present once.
+
+Because this linter has no way of determining which marker definition was intended it does not suggest any fixes 
+
+### Configuration
+It can configured to include a set of custom markers in the analysis by setting:
+```yaml
+lintersConfig:
+  uniqueMarkers:
+    customMarkers:
+      - identifier: custom:SomeCustomMarker
+        attributes:
+          - fruit
+```
+
+For each custom marker, it must specify an `identifier` and optionally some `attributes`.
+As an example, take the marker definition `kubebuilder:validation:XValidation:rule='has(self.foo)',message='should have foo',fieldPath='.foo'`.
+The identifier for the marker is `kubebuilder:validation:XValidation` and its attributes are `rule`, `message`, and `fieldPath`.
+
+When specifying `attributes`, those attributes are included in the uniqueness identification of a marker definition.
+
+Taking the example configuration from above:
+
+- Marker definitions of `custom:SomeCustomMarker:fruit=apple,color=red` and `custom:SomeCustomMarker:fruit=apple,color=green` would violate the uniqueness requirement and be flagged.
+- Marker definitions of `custom:SomeCustomMarker:fruit=apple,color=red` and `custom:SomeCustomMarker:fruit=orange,color=red` would _not_ violate the uniqueness requirement.
+
+Each entry in `customMarkers` must have a unique `identifier`.
+
 # Contributing
 
 New linters can be added by following the [New Linter][new-linter] guide.

pkg/analysis/duplicatemarkers/analyzer.go

@@ -19,7 +19,6 @@ import (
 	"fmt"
 	"go/ast"
 	"go/token"
-	"go/types"
 
 	"golang.org/x/tools/go/analysis"
 
@@ -65,7 +64,7 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Mar
 		return
 	}
 
-	markerSet := collectMarkers(pass, markersAccess, field)
+	markerSet := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
 
 	seen := markers.NewMarkerSet()
 
@@ -79,32 +78,6 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Mar
 	}
 }
 
-func collectMarkers(pass *analysis.Pass, markersAccess markers.Markers, field *ast.Field) markers.MarkerSet {
-	markers := markersAccess.FieldMarkers(field)
-
-	if _, ok := pass.TypesInfo.TypeOf(field.Type).(*types.Basic); ok {
-		return markers
-	}
-
-	ident, ok := field.Type.(*ast.Ident)
-	if !ok {
-		return markers
-	}
-
-	typeSpec, ok := utils.LookupTypeSpec(pass, ident)
-	if !ok {
-		return markers
-	}
-
-	typeMarkers := markersAccess.TypeMarkers(typeSpec)
-
-	// duplicatemarkers removes duplicate markers without the first line.
-	// By inserting the field markers after the type markers, it allows the markers to be removed from the field.
-	typeMarkers.Insert(markers.UnsortedList()...)
-
-	return typeMarkers
-}
-
 func checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
 	if typeSpec == nil {
 		return

pkg/analysis/optionalfields/util.go

@@ -193,7 +193,7 @@ func isStructZeroValueValid(pass *analysis.Pass, field *ast.Field, structType *a
 
 	zeroValueValid, nonOmittedFields := areStructFieldZeroValuesValid(pass, structType, markersAccess, jsonTagInfo)
 
-	markerSet := combinedMarkers(markersAccess, field, structType)
+	markerSet := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
 
 	minProperties, err := getMarkerNumericValueByName[int](markerSet, markers.KubebuilderMinPropertiesMarker)
 	if err != nil && !errors.Is(err, errMarkerMissingValue) {
@@ -332,15 +332,6 @@ func enumFieldAllowsEmpty(fieldMarkers markershelper.MarkerSet) bool {
 	return false
 }
 
-func combinedMarkers(markersAccess markershelper.Markers, field *ast.Field, structType *ast.StructType) markershelper.MarkerSet {
-	markers := markersAccess.FieldMarkers(field)
-	structMarkers := markersAccess.StructMarkers(structType)
-
-	markers.Insert(structMarkers.UnsortedList()...)
-
-	return markers
-}
-
 // number is a type constraint for numeric types.
 // It allows us to create a generic extraction function for numeric values from markers.
 type number interface {

pkg/analysis/uniquemarkers/analyzer.go

@@ -0,0 +1,281 @@
+/*
+Copyright 2025 The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+	http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+package uniquemarkers
+
+import (
+	"fmt"
+	"go/ast"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+	"k8s.io/apimachinery/pkg/util/sets"
+	kalerrors "sigs.k8s.io/kube-api-linter/pkg/analysis/errors"
+	"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/extractjsontags"
+	"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/inspector"
+	"sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
+	"sigs.k8s.io/kube-api-linter/pkg/analysis/utils"
+	"sigs.k8s.io/kube-api-linter/pkg/config"
+	markersconsts "sigs.k8s.io/kube-api-linter/pkg/markers"
+)
+
+const name = "uniquemarkers"
+
+func init() {
+	for _, uniqueMarker := range defaultUniqueMarkers() {
+		markers.DefaultRegistry().Register(uniqueMarker.Identifier)
+	}
+}
+
+type analyzer struct {
+	uniqueMarkers []config.UniqueMarker
+}
+
+func newAnalyzer(cfg config.UniqueMarkersConfig) *analysis.Analyzer {
+	a := &analyzer{
+		uniqueMarkers: append(defaultUniqueMarkers(), cfg.CustomMarkers...),
+	}
+
+	return &analysis.Analyzer{
+		Name:     name,
+		Doc:      "Check that all markers that should be unique on a field/type are only present once",
+		Run:      a.run,
+		Requires: []*analysis.Analyzer{inspector.Analyzer},
+	}
+}
+
+func (a *analyzer) run(pass *analysis.Pass) (any, error) {
+	inspect, ok := pass.ResultOf[inspector.Analyzer].(inspector.Inspector)
+	if !ok {
+		return nil, kalerrors.ErrCouldNotGetInspector
+	}
+
+	inspect.InspectFields(func(field *ast.Field, stack []ast.Node, _ extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
+		checkField(pass, field, markersAccess, a.uniqueMarkers)
+	})
+
+	inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
+		checkType(pass, typeSpec, markersAccess, a.uniqueMarkers)
+	})
+
+	return nil, nil //nolint:nilnil
+}
+
+func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, uniqueMarkers []config.UniqueMarker) {
+	if field == nil || len(field.Names) == 0 {
+		return
+	}
+
+	markers := utils.TypeAwareMarkerCollectionForField(pass, markersAccess, field)
+	check(markers, uniqueMarkers, reportField(pass, field))
+}
+
+func checkType(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers, uniqueMarkers []config.UniqueMarker) {
+	if typeSpec == nil {
+		return
+	}
+
+	markers := markersAccess.TypeMarkers(typeSpec)
+	check(markers, uniqueMarkers, reportType(pass, typeSpec))
+}
+
+func check(markerSet markers.MarkerSet, uniqueMarkers []config.UniqueMarker, reportFunc func(id string)) {
+	for _, marker := range uniqueMarkers {
+		marks := markerSet.Get(marker.Identifier)
+		markSet := sets.New[string]()
+
+		for _, mark := range marks {
+			id := constructIdentifier(mark, marker.Attributes...)
+
+			if markSet.Has(id) {
+				reportFunc(id)
+				continue
+			}
+
+			markSet.Insert(id)
+		}
+	}
+}
+
+// constructIdentifier returns a string that serves as a unique identifier for a
+// marker based on the provided attributes that should be unique for the marker.
+func constructIdentifier(marker markers.Marker, attributes ...string) string {
+	// if there are no unique attributes, the unique identifier is just
+	// the base marker identifier
+	if len(attributes) == 0 {
+		return marker.Identifier
+	}
+
+	// If a marker doesn't specify the attribute, we should assume it is equivalent
+	// to the empty string ("") so that we can still key on uniqueness of other attributes
+	// effectively.
+	id := fmt.Sprintf("%s:", marker.Identifier)
+	for _, attr := range attributes {
+		id += fmt.Sprintf("%s=%s,", attr, marker.Expressions[attr])
+	}
+
+	id = strings.TrimSuffix(id, ",")
+
+	return id
+}
+
+func reportField(pass *analysis.Pass, field *ast.Field) func(id string) {
+	return func(id string) {
+		pass.Report(analysis.Diagnostic{
+			Pos:     field.Pos(),
+			Message: fmt.Sprintf("field %s has multiple definitions of marker %s when only a single definition should exist", field.Names[0].Name, id),
+		})
+	}
+}
+
+func reportType(pass *analysis.Pass, typeSpec *ast.TypeSpec) func(id string) {
+	return func(id string) {
+		pass.Report(analysis.Diagnostic{
+			Pos:     typeSpec.Pos(),
+			Message: fmt.Sprintf("type %s has multiple definitions of marker %s when only a single definition should exist", typeSpec.Name, id),
+		})
+	}
+}
+
+//nolint:funlen
+func defaultUniqueMarkers() []config.UniqueMarker {
+	return []config.UniqueMarker{
+		// Basic unique markers
+		// ------
+		{
+			Identifier: markersconsts.DefaultMarker,
+		},
+		// ------
+
+		// Kubebuilder-specific unique markers
+		// ------
+		{
+			Identifier: markersconsts.KubebuilderDefaultMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderExampleMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderEnumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderExclusiveMaximumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderExclusiveMinimumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderFormatMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMaxItemsMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMaxLengthMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMaxPropertiesMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMaximumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMinItemsMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMinLengthMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMinPropertiesMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMinimumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderMultipleOfMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderPatternMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderTypeMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderUniqueItemsMarker,
+		},
+
+		{
+			Identifier: markersconsts.KubebuilderItemsEnumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsFormatMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMaxLengthMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMaxItemsMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMaxPropertiesMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMaximumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMinLengthMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMinItemsMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMinPropertiesMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMinimumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsExclusiveMaximumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsExclusiveMinimumMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsMultipleOfMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsPatternMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsTypeMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsUniqueItemsMarker,
+		},
+		{
+			Identifier: markersconsts.KubebuilderXValidationMarker,
+			Attributes: []string{
+				"rule",
+			},
+		},
+		{
+			Identifier: markersconsts.KubebuilderItemsXValidationMarker,
+			Attributes: []string{
+				"rule",
+			},
+		},
+		// ------
+	}
+}