wip: support for attribute and value constraints

Signed-off-by: Bryce Palmer <[email protected]>

Author: everettraven

pkg/analysis/forbiddenmarkers/analyzer.go

@@ -31,22 +31,46 @@ import (
 const name = "forbiddenmarkers"
 
 type analyzer struct {
-	forbiddenMarkers []string
+	forbiddenMarkers []config.ForbiddenMarker
+}
+
+// ForbiddenMarkersOptions is a function that configures the
+// forbiddenmarkers analysis.Analyzer
+type ForbiddenMarkersOption func(a *analysis.Analyzer)
+
+// WithName sets the name of the forbiddenmarkers analysis.Analyzer
+func WithName(name string) ForbiddenMarkersOption {
+	return func(a *analysis.Analyzer) {
+		a.Name = name
+	}
+}
+
+// WithDoc sets the doc string of the forbiddenmarkers analysis.Analyzer
+func WithDoc(doc string) ForbiddenMarkersOption {
+	return func(a *analysis.Analyzer) {
+		a.Doc = doc
+	}
 }
 
 // NewAnalyzer creates a new analysis.Analyzer for the forbiddenmarkers
 // linter based on the provided config.ForbiddenMarkersConfig.
-func NewAnalyzer(cfg config.ForbiddenMarkersConfig) *analysis.Analyzer {
+func NewAnalyzer(cfg config.ForbiddenMarkersConfig, opts ...ForbiddenMarkersOption) *analysis.Analyzer {
 	a := &analyzer{
 		forbiddenMarkers: cfg.Markers,
 	}
 
-	return &analysis.Analyzer{
+	analyzer := &analysis.Analyzer{
 		Name:     name,
 		Doc:      "Check that no forbidden markers are present on types and fields.",
 		Run:      a.run,
 		Requires: []*analysis.Analyzer{inspector.Analyzer},
 	}
+
+	for _, opt := range opts {
+		opt(analyzer)
+	}
+
+	return analyzer
 }
 
 func (a *analyzer) run(pass *analysis.Pass) (any, error) {
@@ -66,7 +90,7 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) {
 	return nil, nil //nolint:nilnil
 }
 
-func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, forbiddenMarkers []string) {
+func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers, forbiddenMarkers []config.ForbiddenMarker) {
 	if field == nil || len(field.Names) == 0 {
 		return
 	}
@@ -75,7 +99,7 @@ func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Mar
 	check(markers, forbiddenMarkers, reportField(pass, field))
 }
 
-func checkType(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers, forbiddenMarkers []string) {
+func checkType(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers, forbiddenMarkers []config.ForbiddenMarker) {
 	if typeSpec == nil {
 		return
 	}
@@ -84,15 +108,52 @@ func checkType(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess marker
 	check(markers, forbiddenMarkers, reportType(pass, typeSpec))
 }
 
-func check(markerSet markers.MarkerSet, forbiddenMarkers []string, reportFunc func(marker markers.Marker)) {
+func check(markerSet markers.MarkerSet, forbiddenMarkers []config.ForbiddenMarker, reportFunc func(marker markers.Marker)) {
 	for _, marker := range forbiddenMarkers {
-		marks := markerSet.Get(marker)
+		marks := markerSet.Get(marker.Identifier)
 		for _, mark := range marks {
-			reportFunc(mark)
+			if markerMatchesAttributeRules(mark, marker.Attributes...) {
+				reportFunc(mark)
+			}
 		}
 	}
 }
 
+// TODO: this should probably return some representation of the marker that is failing the
+// attribute rules so that it can be returned to users helpfully.
+func markerMatchesAttributeRules(marker markers.Marker, attrRules ...config.ForbiddenMarkerAttribute) bool {
+	matchesAll := true
+	for _, attrRule := range attrRules {
+		if val, ok := marker.Expressions[attrRule.Attribute]; ok {
+			// if no values are specified, that means the existence match is enough
+			// and we can continue to the next rule
+			if len(attrRule.Values) == 0 {
+				continue
+			}
+
+			// if the value doesn't match one of the forbidden ones, this marker is not forbidden
+			matchesOneValue := false
+			for _, value := range attrRule.Values {
+				if val == value {
+					matchesOneValue = true
+					break
+				}
+			}
+
+			if !matchesOneValue {
+				matchesAll = false
+				break
+			}
+		}
+		// if the marker doesn't contain the attribute for a specified rule it fails the AND
+		// operation.
+		matchesAll = false
+		break
+	}
+
+	return matchesAll
+}
+
 func reportField(pass *analysis.Pass, field *ast.Field) func(marker markers.Marker) {
 	return func(marker markers.Marker) {
 		pass.Report(analysis.Diagnostic{

pkg/analysis/forbiddenmarkers/analyzer_test.go

@@ -26,8 +26,63 @@ import (
 func TestWithConfiguration(t *testing.T) {
 	testdata := analysistest.TestData()
 	analysistest.RunWithSuggestedFixes(t, testdata, forbiddenmarkers.NewAnalyzer(config.ForbiddenMarkersConfig{
-		Markers: []string{
-			"forbidden",
+		Markers: []config.ForbiddenMarker{
+			{
+				Identifier: "custom:forbidden",
+			},
+			{
+				Identifier: "custom:AttrNoValues",
+				Attributes: []config.ForbiddenMarkerAttribute{
+					{
+						Attribute: "fruit",
+					},
+				},
+			},
+			{
+				Identifier: "custom:AttrValues",
+				Attributes: []config.ForbiddenMarkerAttribute{
+					{
+						Attribute: "fruit",
+						Values: []string{
+							"apple",
+							"orange",
+							"banana",
+						},
+					},
+				},
+			},
+			{
+				Identifier: "custom:AttrsNoValues",
+				Attributes: []config.ForbiddenMarkerAttribute{
+					{
+						Attribute: "fruit",
+					},
+					{
+						Attribute: "color",
+					},
+				},
+			},
+			{
+				Identifier: "custom:AttrsValues",
+				Attributes: []config.ForbiddenMarkerAttribute{
+					{
+						Attribute: "fruit",
+						Values: []string{
+							"apple",
+							"orange",
+							"banana",
+						},
+					},
+					{
+						Attribute: "color",
+						Values: []string{
+							"red",
+							"blue",
+							"green",
+						},
+					},
+				},
+			},
 		},
 	}), "a/...")
 }

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

@@ -1,14 +1,20 @@
 package a
 
-// +forbidden
-type ForbiddenMarkerType string // want `type ForbiddenMarkerType has forbidden marker "forbidden"`
+// +custom:forbidden
+type ForbiddenMarkerType string // want `type ForbiddenMarkerType has forbidden marker "custom:forbidden"`
+
+// +custom:AttrNoValues:fruit=apple
+type ForbiddenMarkerWithAttrType string // want `type ForbiddenMarkerWithAttrType has forbidden marker "forbidden:AttrNoValues"`
 
 // +allowed
 type AllowedMarkerType string
 
+// +custom:AttrNoValues:color=blue
+type AllowedMarkerWithAttrType string
+
 type Test struct {
-	// +forbidden
-	ForbiddenMarkerField string `json:"forbiddenMarkerField"`// want `field ForbiddenMarkerField has forbidden marker "forbidden"`
+	// +custom:forbidden
+	ForbiddenMarkerField string `json:"forbiddenMarkerField"`// want `field ForbiddenMarkerField has forbidden marker "custom:forbidden"`
 
 	ForbiddenMarkerFieldTypeAlias ForbiddenMarkerType `json:"forbiddenMarkerFieldTypeAlias"` // want `field ForbiddenMarkerFieldTypeAlias has forbidden marker "forbidden"`
 
@@ -17,4 +23,3 @@ type Test struct {
 
 	AllowedMarkerFieldTypeAlias AllowedMarkerType `json:"AllowedMarkerFieldTypeAlias"`
 }
-

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

@@ -1,12 +1,17 @@
 package a
 
-type ForbiddenMarkerType string // want `type ForbiddenMarkerType has forbidden marker "forbidden"`
+type ForbiddenMarkerType string // want `type ForbiddenMarkerType has forbidden marker "custom:forbidden"`
+
+type ForbiddenMarkerWithAttrType string // want `type ForbiddenMarkerWithAttrType has forbidden marker "forbidden:AttrNoValues"`
 
 // +allowed
 type AllowedMarkerType string
 
+// +custom:AttrNoValues:color=blue
+type AllowedMarkerWithAttrType string
+
 type Test struct {
-	ForbiddenMarkerField string `json:"forbiddenMarkerField"`// want `field ForbiddenMarkerField has forbidden marker "forbidden"`
+	ForbiddenMarkerField string `json:"forbiddenMarkerField"`// want `field ForbiddenMarkerField has forbidden marker "custom:forbidden"`
 
 	ForbiddenMarkerFieldTypeAlias ForbiddenMarkerType `json:"forbiddenMarkerFieldTypeAlias"` // want `field ForbiddenMarkerFieldTypeAlias has forbidden marker "forbidden"`
 

pkg/analysis/nonullable/analyzer.go

@@ -22,16 +22,16 @@ import (
 )
 
 const name = "nonullable"
+const doc = "Check that nullable marker is not present on any types or fields."
 
 func newAnalyzer() *analysis.Analyzer {
 	analyzer := forbiddenmarkers.NewAnalyzer(config.ForbiddenMarkersConfig{
-		Markers: []string{
-			"nullable",
+		Markers: []config.ForbiddenMarker{
+			{
+				Identifier: "nullable",
+			},
 		},
-	})
-
-	analyzer.Name = name
-	analyzer.Doc = "Check that nullable marker is not present on any types or fields."
+	}, forbiddenmarkers.WithName(name), forbiddenmarkers.WithDoc(doc))
 
 	return analyzer
 }