Skip to content

unify marker definitions to markers package #74

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 7, 2025
Merged

unify marker definitions to markers package #74

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 28 additions & 37 deletions pkg/analysis/maxlength/analyzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,22 +23,13 @@ import (
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"
markershelper "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/markers"
)

const (
name = "maxlength"

kubebuilderMaxLength = "kubebuilder:validation:MaxLength"
kubebuilderEnum = "kubebuilder:validation:Enum"
kubebuilderFormat = "kubebuilder:validation:Format"

kubebuilderItemsMaxLength = "kubebuilder:validation:items:MaxLength"
kubebuilderItemsEnum = "kubebuilder:validation:items:Enum"
kubebuilderItemsFormat = "kubebuilder:validation:items:Format"

kubebuilderMaxItems = "kubebuilder:validation:MaxItems"
)

// Analyzer is the analyzer for the maxlength package.
Expand All @@ -56,25 +47,25 @@ func run(pass *analysis.Pass) (any, error) {
return nil, kalerrors.ErrCouldNotGetInspector
}

inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markershelper.Markers) {
checkField(pass, field, markersAccess)
})

return nil, nil //nolint:nilnil
}

func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markers.Markers) {
func checkField(pass *analysis.Pass, field *ast.Field, markersAccess markershelper.Markers) {
if len(field.Names) == 0 || field.Names[0] == nil {
return
}

fieldName := field.Names[0].Name
prefix := fmt.Sprintf("field %s", fieldName)

checkTypeExpr(pass, field.Type, field, nil, markersAccess, prefix, kubebuilderMaxLength, needsStringMaxLength)
checkTypeExpr(pass, field.Type, field, nil, markersAccess, prefix, markers.KubebuilderMaxLengthMarker, needsStringMaxLength)
}

func checkIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {
func checkIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markershelper.Markers, prefix, marker string, needsMaxLength func(markershelper.MarkerSet) bool) {
if utils.IsBasicType(pass, ident) { // Built-in type
checkString(pass, ident, node, aliases, markersAccess, prefix, marker, needsMaxLength)

Expand All @@ -89,7 +80,7 @@ func checkIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []
checkTypeSpec(pass, tSpec, node, append(aliases, tSpec), markersAccess, fmt.Sprintf("%s type", prefix), marker, needsMaxLength)
}

func checkString(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {
func checkString(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markershelper.Markers, prefix, marker string, needsMaxLength func(markershelper.MarkerSet) bool) {
if ident.Name != "string" {
return
}
Expand All @@ -101,7 +92,7 @@ func checkString(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases [
}
}

func checkTypeSpec(pass *analysis.Pass, tSpec *ast.TypeSpec, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {
func checkTypeSpec(pass *analysis.Pass, tSpec *ast.TypeSpec, node ast.Node, aliases []*ast.TypeSpec, markersAccess markershelper.Markers, prefix, marker string, needsMaxLength func(markershelper.MarkerSet) bool) {
if tSpec.Name == nil {
return
}
Expand All @@ -112,7 +103,7 @@ func checkTypeSpec(pass *analysis.Pass, tSpec *ast.TypeSpec, node ast.Node, alia
checkTypeExpr(pass, tSpec.Type, node, aliases, markersAccess, prefix, marker, needsMaxLength)
}

func checkTypeExpr(pass *analysis.Pass, typeExpr ast.Expr, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix, marker string, needsMaxLength func(markers.MarkerSet) bool) {
func checkTypeExpr(pass *analysis.Pass, typeExpr ast.Expr, node ast.Node, aliases []*ast.TypeSpec, markersAccess markershelper.Markers, prefix, marker string, needsMaxLength func(markershelper.MarkerSet) bool) {
switch typ := typeExpr.(type) {
case *ast.Ident:
checkIdent(pass, typ, node, aliases, markersAccess, prefix, marker, needsMaxLength)
Expand All @@ -123,7 +114,7 @@ func checkTypeExpr(pass *analysis.Pass, typeExpr ast.Expr, node ast.Node, aliase
}
}

func checkArrayType(pass *analysis.Pass, arrayType *ast.ArrayType, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix string) {
func checkArrayType(pass *analysis.Pass, arrayType *ast.ArrayType, node ast.Node, aliases []*ast.TypeSpec, markersAccess markershelper.Markers, prefix string) {
if arrayType.Elt != nil {
if ident, ok := arrayType.Elt.(*ast.Ident); ok {
if ident.Name == "byte" {
Expand All @@ -133,7 +124,7 @@ func checkArrayType(pass *analysis.Pass, arrayType *ast.ArrayType, node ast.Node
NamePos: ident.NamePos,
Name: "string",
}
checkString(pass, i, node, aliases, markersAccess, prefix, kubebuilderMaxLength, needsStringMaxLength)
checkString(pass, i, node, aliases, markersAccess, prefix, markers.KubebuilderMaxLengthMarker, needsStringMaxLength)

return
}
Expand All @@ -142,16 +133,16 @@ func checkArrayType(pass *analysis.Pass, arrayType *ast.ArrayType, node ast.Node
}
}

markers := getCombinedMarkers(markersAccess, node, aliases)
markerSet := getCombinedMarkers(markersAccess, node, aliases)

if !markers.Has(kubebuilderMaxItems) {
pass.Reportf(node.Pos(), "%s must have a maximum items, add %s marker", prefix, kubebuilderMaxItems)
if !markerSet.Has(markers.KubebuilderMaxItemsMarker) {
pass.Reportf(node.Pos(), "%s must have a maximum items, add %s marker", prefix, markers.KubebuilderMaxItemsMarker)
}
}

func checkArrayElementIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markers.Markers, prefix string) {
func checkArrayElementIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node, aliases []*ast.TypeSpec, markersAccess markershelper.Markers, prefix string) {
if ident.Obj == nil { // Built-in type
checkString(pass, ident, node, aliases, markersAccess, prefix, kubebuilderItemsMaxLength, needsItemsMaxLength)
checkString(pass, ident, node, aliases, markersAccess, prefix, markers.KubebuilderItemsMaxLengthMarker, needsItemsMaxLength)

return
}
Expand All @@ -163,13 +154,13 @@ func checkArrayElementIdent(pass *analysis.Pass, ident *ast.Ident, node ast.Node

// If the array element wasn't directly a string, allow a string alias to be used
// with either the items style markers or the on alias style markers.
checkTypeSpec(pass, tSpec, node, append(aliases, tSpec), markersAccess, fmt.Sprintf("%s type", prefix), kubebuilderMaxLength, func(ms markers.MarkerSet) bool {
checkTypeSpec(pass, tSpec, node, append(aliases, tSpec), markersAccess, fmt.Sprintf("%s type", prefix), markers.KubebuilderMaxLengthMarker, func(ms markershelper.MarkerSet) bool {
return needsStringMaxLength(ms) && needsItemsMaxLength(ms)
})
}

func getCombinedMarkers(markersAccess markers.Markers, node ast.Node, aliases []*ast.TypeSpec) markers.MarkerSet {
base := markers.NewMarkerSet(getMarkers(markersAccess, node).UnsortedList()...)
func getCombinedMarkers(markersAccess markershelper.Markers, node ast.Node, aliases []*ast.TypeSpec) markershelper.MarkerSet {
base := markershelper.NewMarkerSet(getMarkers(markersAccess, node).UnsortedList()...)

for _, a := range aliases {
base.Insert(getMarkers(markersAccess, a).UnsortedList()...)
Expand All @@ -178,7 +169,7 @@ func getCombinedMarkers(markersAccess markers.Markers, node ast.Node, aliases []
return base
}

func getMarkers(markersAccess markers.Markers, node ast.Node) markers.MarkerSet {
func getMarkers(markersAccess markershelper.Markers, node ast.Node) markershelper.MarkerSet {
switch t := node.(type) {
case *ast.Field:
return markersAccess.FieldMarkers(t)
Expand All @@ -192,10 +183,10 @@ func getMarkers(markersAccess markers.Markers, node ast.Node) markers.MarkerSet
// needsMaxLength returns true if the field needs a maximum length.
// Fields do not need a maximum length if they are already marked with a maximum length,
// or if they are an enum, or if they are a date, date-time or duration.
func needsStringMaxLength(markerSet markers.MarkerSet) bool {
func needsStringMaxLength(markerSet markershelper.MarkerSet) bool {
switch {
case markerSet.Has(kubebuilderMaxLength),
markerSet.Has(kubebuilderEnum),
case markerSet.Has(markers.KubebuilderMaxLengthMarker),
markerSet.Has(markers.KubebuilderEnumMarker),
markerSet.HasWithValue(kubebuilderFormatWithValue("date")),
markerSet.HasWithValue(kubebuilderFormatWithValue("date-time")),
markerSet.HasWithValue(kubebuilderFormatWithValue("duration")):
Expand All @@ -205,10 +196,10 @@ func needsStringMaxLength(markerSet markers.MarkerSet) bool {
return true
}

func needsItemsMaxLength(markerSet markers.MarkerSet) bool {
func needsItemsMaxLength(markerSet markershelper.MarkerSet) bool {
switch {
case markerSet.Has(kubebuilderItemsMaxLength),
markerSet.Has(kubebuilderItemsEnum),
case markerSet.Has(markers.KubebuilderItemsMaxLengthMarker),
markerSet.Has(markers.KubebuilderItemsEnumMarker),
markerSet.HasWithValue(kubebuilderItemsFormatWithValue("date")),
markerSet.HasWithValue(kubebuilderItemsFormatWithValue("date-time")),
markerSet.HasWithValue(kubebuilderItemsFormatWithValue("duration")):
Expand All @@ -219,9 +210,9 @@ func needsItemsMaxLength(markerSet markers.MarkerSet) bool {
}

func kubebuilderFormatWithValue(value string) string {
return fmt.Sprintf("%s:=%s", kubebuilderFormat, value)
return fmt.Sprintf("%s:=%s", markers.KubebuilderFormatMarker, value)
}

func kubebuilderItemsFormatWithValue(value string) string {
return fmt.Sprintf("%s:=%s", kubebuilderItemsFormat, value)
return fmt.Sprintf("%s:=%s", markers.KubebuilderItemsFormatMarker, value)
}
89 changes: 36 additions & 53 deletions pkg/analysis/optionalorrequired/analyzer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,40 +23,23 @@ import (
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"
markershelper "sigs.k8s.io/kube-api-linter/pkg/analysis/helpers/markers"
"sigs.k8s.io/kube-api-linter/pkg/config"
"sigs.k8s.io/kube-api-linter/pkg/markers"
)

const (
name = "optionalorrequired"

// OptionalMarker is the marker that indicates that a field is optional.
OptionalMarker = "optional"

// RequiredMarker is the marker that indicates that a field is required.
RequiredMarker = "required"

// KubebuilderOptionalMarker is the marker that indicates that a field is optional in kubebuilder.
KubebuilderOptionalMarker = "kubebuilder:validation:Optional"

// KubebuilderRequiredMarker is the marker that indicates that a field is required in kubebuilder.
KubebuilderRequiredMarker = "kubebuilder:validation:Required"

// K8sOptionalMarker is the marker that indicates that a field is optional in k8s declarative validation.
K8sOptionalMarker = "k8s:optional"

// K8sRequiredMarker is the marker that indicates that a field is required in k8s declarative validation.
K8sRequiredMarker = "k8s:required"
)

func init() {
markers.DefaultRegistry().Register(
OptionalMarker,
RequiredMarker,
KubebuilderOptionalMarker,
KubebuilderRequiredMarker,
K8sOptionalMarker,
K8sRequiredMarker,
markershelper.DefaultRegistry().Register(
markers.OptionalMarker,
markers.RequiredMarker,
markers.KubebuilderOptionalMarker,
markers.KubebuilderRequiredMarker,
markers.K8sOptionalMarker,
markers.K8sRequiredMarker,
)
}

Expand All @@ -75,21 +58,21 @@ func newAnalyzer(cfg config.OptionalOrRequiredConfig) *analysis.Analyzer {
a := &analyzer{}

switch cfg.PreferredOptionalMarker {
case OptionalMarker:
a.primaryOptionalMarker = OptionalMarker
a.secondaryOptionalMarker = KubebuilderOptionalMarker
case KubebuilderOptionalMarker:
a.primaryOptionalMarker = KubebuilderOptionalMarker
a.secondaryOptionalMarker = OptionalMarker
case markers.OptionalMarker:
a.primaryOptionalMarker = markers.OptionalMarker
a.secondaryOptionalMarker = markers.KubebuilderOptionalMarker
case markers.KubebuilderOptionalMarker:
a.primaryOptionalMarker = markers.KubebuilderOptionalMarker
a.secondaryOptionalMarker = markers.OptionalMarker
}

switch cfg.PreferredRequiredMarker {
case RequiredMarker:
a.primaryRequiredMarker = RequiredMarker
a.secondaryRequiredMarker = KubebuilderRequiredMarker
case KubebuilderRequiredMarker:
a.primaryRequiredMarker = KubebuilderRequiredMarker
a.secondaryRequiredMarker = RequiredMarker
case markers.RequiredMarker:
a.primaryRequiredMarker = markers.RequiredMarker
a.secondaryRequiredMarker = markers.KubebuilderRequiredMarker
case markers.KubebuilderRequiredMarker:
a.primaryRequiredMarker = markers.KubebuilderRequiredMarker
a.secondaryRequiredMarker = markers.RequiredMarker
}

return &analysis.Analyzer{
Expand All @@ -106,19 +89,19 @@ func (a *analyzer) run(pass *analysis.Pass) (any, error) {
return nil, kalerrors.ErrCouldNotGetInspector
}

inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markers.Markers) {
inspect.InspectFields(func(field *ast.Field, stack []ast.Node, jsonTagInfo extractjsontags.FieldTagInfo, markersAccess markershelper.Markers) {
a.checkField(pass, field, markersAccess.FieldMarkers(field), jsonTagInfo)
})

inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
inspect.InspectTypeSpec(func(typeSpec *ast.TypeSpec, markersAccess markershelper.Markers) {
a.checkTypeSpec(pass, typeSpec, markersAccess)
})

return nil, nil //nolint:nilnil
}

//nolint:cyclop
func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, fieldMarkers markers.MarkerSet, fieldTagInfo extractjsontags.FieldTagInfo) {
func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, fieldMarkers markershelper.MarkerSet, fieldTagInfo extractjsontags.FieldTagInfo) {
if fieldTagInfo.Inline {
// Inline fields would have no effect if they were marked as optional/required.
return
Expand Down Expand Up @@ -169,24 +152,24 @@ func (a *analyzer) checkField(pass *analysis.Pass, field *ast.Field, fieldMarker
}
}

func (a *analyzer) checkK8sMarkers(pass *analysis.Pass, field *ast.Field, fieldMarkers markers.MarkerSet, prefix string, hasEitherOptional, hasEitherRequired bool) {
hasK8sOptional := fieldMarkers.Has(K8sOptionalMarker)
hasK8sRequired := fieldMarkers.Has(K8sRequiredMarker)
func (a *analyzer) checkK8sMarkers(pass *analysis.Pass, field *ast.Field, fieldMarkers markershelper.MarkerSet, prefix string, hasEitherOptional, hasEitherRequired bool) {
hasK8sOptional := fieldMarkers.Has(markers.K8sOptionalMarker)
hasK8sRequired := fieldMarkers.Has(markers.K8sRequiredMarker)

if hasK8sOptional && hasK8sRequired {
pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, K8sOptionalMarker, K8sRequiredMarker)
pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, markers.K8sOptionalMarker, markers.K8sRequiredMarker)
}

if hasK8sOptional && hasEitherRequired {
pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, K8sOptionalMarker, RequiredMarker)
pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, markers.K8sOptionalMarker, markers.RequiredMarker)
}

if hasK8sRequired && hasEitherOptional {
pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, OptionalMarker, K8sRequiredMarker)
pass.Reportf(field.Pos(), "%s must not be marked as both %s and %s", prefix, markers.OptionalMarker, markers.K8sRequiredMarker)
}
}

func reportShouldReplaceSecondaryMarker(field *ast.Field, marker []markers.Marker, primaryMarker, secondaryMarker, prefix string) analysis.Diagnostic {
func reportShouldReplaceSecondaryMarker(field *ast.Field, marker []markershelper.Marker, primaryMarker, secondaryMarker, prefix string) analysis.Diagnostic {
textEdits := make([]analysis.TextEdit, len(marker))

for i, m := range marker {
Expand Down Expand Up @@ -219,7 +202,7 @@ func reportShouldReplaceSecondaryMarker(field *ast.Field, marker []markers.Marke
}
}

func reportShouldRemoveSecondaryMarker(field *ast.Field, marker []markers.Marker, primaryMarker, secondaryMarker, prefix string) analysis.Diagnostic {
func reportShouldRemoveSecondaryMarker(field *ast.Field, marker []markershelper.Marker, primaryMarker, secondaryMarker, prefix string) analysis.Diagnostic {
textEdits := make([]analysis.TextEdit, len(marker))

for i, m := range marker {
Expand All @@ -242,13 +225,13 @@ func reportShouldRemoveSecondaryMarker(field *ast.Field, marker []markers.Marker
}
}

func (a *analyzer) checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markers.Markers) {
func (a *analyzer) checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, markersAccess markershelper.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, K8sOptionalMarker, K8sRequiredMarker:
case a.primaryOptionalMarker, a.secondaryOptionalMarker, a.primaryRequiredMarker, a.secondaryRequiredMarker, markers.K8sOptionalMarker, markers.K8sRequiredMarker:
pass.Report(analysis.Diagnostic{
Pos: typeSpec.Pos(),
Message: fmt.Sprintf("type %s should not be marked as %s", name, marker.String()),
Expand All @@ -271,10 +254,10 @@ func (a *analyzer) checkTypeSpec(pass *analysis.Pass, typeSpec *ast.TypeSpec, ma

func defaultConfig(cfg *config.OptionalOrRequiredConfig) {
if cfg.PreferredOptionalMarker == "" {
cfg.PreferredOptionalMarker = OptionalMarker
cfg.PreferredOptionalMarker = markers.OptionalMarker
}

if cfg.PreferredRequiredMarker == "" {
cfg.PreferredRequiredMarker = RequiredMarker
cfg.PreferredRequiredMarker = markers.RequiredMarker
}
}
5 changes: 3 additions & 2 deletions pkg/analysis/optionalorrequired/analyzer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"golang.org/x/tools/go/analysis/analysistest"
"sigs.k8s.io/kube-api-linter/pkg/analysis/optionalorrequired"
"sigs.k8s.io/kube-api-linter/pkg/config"
"sigs.k8s.io/kube-api-linter/pkg/markers"
)

func TestDefaultConfiguration(t *testing.T) {
Expand All @@ -39,8 +40,8 @@ func TestSwappedMarkerPriority(t *testing.T) {

a, err := optionalorrequired.Initializer().Init(config.LintersConfig{
OptionalOrRequired: config.OptionalOrRequiredConfig{
PreferredOptionalMarker: optionalorrequired.KubebuilderOptionalMarker,
PreferredRequiredMarker: optionalorrequired.KubebuilderRequiredMarker,
PreferredOptionalMarker: markers.KubebuilderOptionalMarker,
PreferredRequiredMarker: markers.KubebuilderRequiredMarker,
},
})
if err != nil {
Expand Down
Loading