lint: add 7 architectural-sync cops (catches 10 real bugs)

go.mod

@@ -145,7 +145,7 @@ require (
 	github.com/containerd/stargz-snapshotter/estargz v0.18.2 // indirect
 	github.com/cyphar/filepath-securejoin v0.4.1 // indirect
 	github.com/davecgh/go-spew v1.1.1 // indirect
-	github.com/dgageot/rubocop-go v0.0.0-20260323134452-aecdd6345645
+	github.com/dgageot/rubocop-go v0.0.0-20260429095109-a9cea3bf3e72
 	github.com/distribution/reference v0.6.0 // indirect
 	github.com/dlclark/regexp2 v1.11.5 // indirect
 	github.com/docker/distribution v2.8.3+incompatible // indirect

go.sum

@@ -188,8 +188,8 @@ github.com/danieljoos/wincred v1.2.2/go.mod h1:w7w4Utbrz8lqeMbDAK0lkNJUv5sAOkFi7
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/dgageot/rubocop-go v0.0.0-20260323134452-aecdd6345645 h1:7UgEWAo69Dgbtii1j1FLWE88+Rem9Qly4LLrrQhAN0s=
-github.com/dgageot/rubocop-go v0.0.0-20260323134452-aecdd6345645/go.mod h1:r8YOJV5+/30NZ8HW/2NbWUObBGDXGvfHrjgury5YlFI=
+github.com/dgageot/rubocop-go v0.0.0-20260429095109-a9cea3bf3e72 h1:2BdfP/9nnmcCHUBZhOueTb6rQojfZdlh2hI3E65cY2I=
+github.com/dgageot/rubocop-go v0.0.0-20260429095109-a9cea3bf3e72/go.mod h1:r8YOJV5+/30NZ8HW/2NbWUObBGDXGvfHrjgury5YlFI=
 github.com/dgageot/ultraviolet v0.0.0-20260313154905-9451997d56b6 h1:88fWkkjwzuI4tRTqadbJIbA9O+gO67oyu+2OpHHuuT8=
 github.com/dgageot/ultraviolet v0.0.0-20260313154905-9451997d56b6/go.mod h1:SQpCTRNBtzJkwku5ye4S3HEuthAlGy2n9VXZnWkEW98=
 github.com/distribution/reference v0.6.0 h1:0IXCQ5g4/QMHHkarYzh5l+u8T3t73zM5QvfrDyIgxBk=

lint/config_latest_tag_consistency.go

@@ -0,0 +1,147 @@
+package main
+
+import (
+	"go/ast"
+	"reflect"
+	"strconv"
+	"strings"
+
+	"github.com/dgageot/rubocop-go/cop"
+)
+
+// ConfigLatestTagConsistency enforces that struct fields under
+// pkg/config/latest/ keep their `json` and `yaml` struct tags in sync with
+// respect to the omit-when-empty modifier:
+//
+//   - both tags carry `omitempty` (or `omitzero` on the json side), or
+//   - neither tag carries an omit modifier.
+//
+// Mismatched modifiers silently produce different serialisations depending
+// on the format: a value that round-trips through YAML may be lost when the
+// same struct is encoded as JSON (or vice-versa). Because pkg/config/latest
+// is the work-in-progress schema, such drift is almost always a typo, not
+// an intentional design choice.
+//
+// Frozen versioned packages (pkg/config/vN/) are intentionally exempt: they
+// must reproduce the exact wire format that shipped, including any historical
+// quirks.
+//
+// Recognised modifiers:
+//   - json: `omitempty`, `omitzero` (the latter introduced in Go 1.24)
+//   - yaml: `omitempty`
+//
+// Fields without a `yaml` tag are skipped — yaml.v3 derives a default name
+// from the field, which is a separate concern handled elsewhere.
+type ConfigLatestTagConsistency struct {
+	cop.Meta
+}
+
+// NewConfigLatestTagConsistency returns a fully configured
+// ConfigLatestTagConsistency cop.
+func NewConfigLatestTagConsistency() *ConfigLatestTagConsistency {
+	return &ConfigLatestTagConsistency{Meta: cop.Meta{
+		CopName:     "Lint/ConfigLatestTagConsistency",
+		CopDesc:     "json and yaml tags in pkg/config/latest must agree on omitempty/omitzero",
+		CopSeverity: cop.Error,
+	}}
+}
+
+func (c *ConfigLatestTagConsistency) Check(p *cop.Pass) {
+	if configDir(p.Filename()) != "latest" {
+		return
+	}
+	// Black-box test files don't ship struct definitions for the wire format.
+	if p.IsBlackBoxTest() {
+		return
+	}
+
+	ast.Inspect(p.File, func(n ast.Node) bool {
+		field, ok := n.(*ast.Field)
+		if !ok || field.Tag == nil {
+			return true
+		}
+		raw, err := strconv.Unquote(field.Tag.Value)
+		if err != nil {
+			return true
+		}
+		tag := reflect.StructTag(raw)
+
+		jsonTag, hasJSON := tag.Lookup("json")
+		yamlTag, hasYAML := tag.Lookup("yaml")
+		if !hasJSON || !hasYAML {
+			return true
+		}
+		// Embedded fields use ",inline" on both sides; nothing to compare.
+		if isInline(jsonTag) || isInline(yamlTag) {
+			return true
+		}
+
+		jsonOmit, jsonMod := jsonOmitModifier(jsonTag)
+		yamlOmit := hasOption(yamlTag, "omitempty")
+
+		if jsonOmit != yamlOmit {
+			p.Report(field.Tag,
+				"json and yaml tags disagree on omit-when-empty: json has %q, yaml has %q (field %s)",
+				modifierLabel(jsonOmit, jsonMod), modifierLabel(yamlOmit, "omitempty"),
+				fieldNames(field))
+		}
+		return true
+	})
+}
+
+// jsonOmitModifier reports whether the json tag opts out of empty values,
+// returning the specific modifier that triggered the match (omitempty or
+// omitzero). When neither is present, it returns false and "".
+func jsonOmitModifier(tag string) (bool, string) {
+	switch {
+	case hasOption(tag, "omitempty"):
+		return true, "omitempty"
+	case hasOption(tag, "omitzero"):
+		return true, "omitzero"
+	default:
+		return false, ""
+	}
+}
+
+// hasOption reports whether the comma-separated options on tag include opt.
+// The first comma-separated entry is the field name; only later entries are
+// modifiers (`omitempty`, `omitzero`, `inline`, …).
+func hasOption(tag, opt string) bool {
+	for i, part := range strings.Split(tag, ",") {
+		if i == 0 {
+			continue
+		}
+		if part == opt {
+			return true
+		}
+	}
+	return false
+}
+
+// isInline reports whether the tag carries a ",inline" option, which is used
+// by both json/yaml libraries to flatten an embedded struct into the parent.
+func isInline(tag string) bool {
+	return hasOption(tag, "inline")
+}
+
+// modifierLabel renders a human-readable description of the modifier for an
+// error message: the modifier name when present, "<none>" otherwise.
+func modifierLabel(present bool, name string) string {
+	if !present {
+		return "<none>"
+	}
+	return name
+}
+
+// fieldNames returns the comma-separated list of field identifiers for use in
+// diagnostic messages. Anonymous (embedded) fields fall back to "<embedded>".
+func fieldNames(field *ast.Field) string {
+	if len(field.Names) == 0 {
+		return "<embedded>"
+	}
+	names := make([]string, 0, len(field.Names))
+	for _, n := range field.Names {
+		names = append(names, n.Name)
+	}
+	return strings.Join(names, ", ")
+}

lint/config_package_name.go

@@ -1,11 +1,6 @@
 package main
 
 import (
-	"fmt"
-	"go/ast"
-	"go/token"
-	"strings"
-
 	"github.com/dgageot/rubocop-go/cop"
 )
 
@@ -20,32 +15,35 @@ import (
 // is frozen into a numbered vN directory: the package clause is easy to
 // forget, and the broken state remains compilable as long as importers use
 // an explicit alias.
-type ConfigPackageName struct{}
+type ConfigPackageName struct {
+	cop.Meta
+}
 
-func (*ConfigPackageName) Name() string { return "Lint/ConfigPackageName" }
-func (*ConfigPackageName) Description() string {
-	return "Files under pkg/config/<dir>/ must declare package <dir>"
+// NewConfigPackageName returns a fully configured ConfigPackageName cop.
+func NewConfigPackageName() *ConfigPackageName {
+	return &ConfigPackageName{Meta: cop.Meta{
+		CopName:     "Lint/ConfigPackageName",
+		CopDesc:     "Files under pkg/config/<dir>/ must declare package <dir>",
+		CopSeverity: cop.Error,
+	}}
 }
-func (*ConfigPackageName) Severity() cop.Severity { return cop.Error }
 
-func (c *ConfigPackageName) Check(fset *token.FileSet, file *ast.File) []cop.Offense {
-	filename := fset.Position(file.Package).Filename
-	dir := configDir(filename)
+func (c *ConfigPackageName) Check(p *cop.Pass) {
+	dir := configDir(p.Filename())
 	if dir == "" {
-		return nil
+		return
 	}
 
-	got := file.Name.Name
+	got := p.PackageName()
 	switch got {
 	case dir:
-		return nil
+		return
 	case dir + "_test":
 		// Black-box test packages are a legitimate Go convention.
-		if strings.HasSuffix(filename, "_test.go") {
-			return nil
+		if p.IsTestFile() {
+			return
 		}
 	}
 
-	return []cop.Offense{offense(c, fset, file.Name,
-		fmt.Sprintf("file in pkg/config/%s/ must declare package %s, got %s", dir, dir, got))}
+	p.Report(p.File.Name, "file in pkg/config/%s/ must declare package %s, got %s", dir, dir, got)
 }

lint/config_version_constant.go

@@ -1,7 +1,6 @@
 package main
 
 import (
-	"fmt"
 	"go/ast"
 	"go/token"
 	"strconv"
@@ -20,42 +19,40 @@ import (
 //
 // Files under pkg/config/latest/ are intentionally exempt: their `Version`
 // is the next, work-in-progress value (one greater than the highest vN).
-type ConfigVersionConstant struct{}
+type ConfigVersionConstant struct {
+	cop.Meta
+}
 
-func (*ConfigVersionConstant) Name() string { return "Lint/ConfigVersionConstant" }
-func (*ConfigVersionConstant) Description() string {
-	return "Version constant in pkg/config/vN/ must equal \"N\""
+// NewConfigVersionConstant returns a fully configured ConfigVersionConstant cop.
+func NewConfigVersionConstant() *ConfigVersionConstant {
+	return &ConfigVersionConstant{Meta: cop.Meta{
+		CopName:     "Lint/ConfigVersionConstant",
+		CopDesc:     "Version constant in pkg/config/vN/ must equal \"N\"",
+		CopSeverity: cop.Error,
+	}}
 }
-func (*ConfigVersionConstant) Severity() cop.Severity { return cop.Error }
 
-func (c *ConfigVersionConstant) Check(fset *token.FileSet, file *ast.File) []cop.Offense {
-	dirVersion, ok := versionFromDir(configDir(fset.Position(file.Package).Filename))
+func (c *ConfigVersionConstant) Check(p *cop.Pass) {
+	dirVersion, ok := versionFromDir(configDir(p.Filename()))
 	if !ok {
-		return nil
+		return
 	}
 	expected := strconv.Itoa(dirVersion)
 
-	var offenses []cop.Offense
-	for _, lit := range versionStringLiterals(file) {
+	for _, lit := range versionStringLiterals(p) {
 		got, err := strconv.Unquote(lit.Value)
 		if err != nil || got == expected {
 			continue
 		}
-		offenses = append(offenses, offense(c, fset, lit,
-			fmt.Sprintf("Version in pkg/config/v%s/ must be %q, got %q", expected, expected, got)))
+		p.Report(lit, "Version in pkg/config/v%s/ must be %q, got %q", expected, expected, got)
 	}
-	return offenses
 }
 
 // versionStringLiterals returns the value literal of every top-level
-// `const Version = "<string>"` declaration in file.
-func versionStringLiterals(file *ast.File) []*ast.BasicLit {
+// `const Version = "<string>"` declaration in the file under inspection.
+func versionStringLiterals(p *cop.Pass) []*ast.BasicLit {
 	var lits []*ast.BasicLit
-	for _, decl := range file.Decls {
-		gen, ok := decl.(*ast.GenDecl)
-		if !ok || gen.Tok != token.CONST {
-			continue
-		}
+	p.ForEachConst(func(gen *ast.GenDecl) {
 		for _, spec := range gen.Specs {
 			vs, ok := spec.(*ast.ValueSpec)
 			if !ok {
@@ -72,6 +69,6 @@ func versionStringLiterals(file *ast.File) []*ast.BasicLit {
 				lits = append(lits, lit)
 			}
 		}
-	}
+	})
 	return lits
 }

lint/config_version_import.go

@@ -2,8 +2,6 @@ package main
 
 import (
 	"fmt"
-	"go/ast"
-	"go/token"
 	"strings"
 
 	"github.com/dgageot/rubocop-go/cop"
@@ -13,47 +11,49 @@ import (
 // and pkg/config/latest) only import their immediate predecessor and the
 // shared types package, preserving the strict migration chain:
 // v0 → v1 → v2 → … → latest.
-type ConfigVersionImport struct{}
-
-func (*ConfigVersionImport) Name() string { return "Lint/ConfigVersionImport" }
-func (*ConfigVersionImport) Description() string {
-	return "Config version packages must only import their immediate predecessor"
+type ConfigVersionImport struct {
+	cop.Meta
 }
-func (*ConfigVersionImport) Severity() cop.Severity { return cop.Error }
 
-func (c *ConfigVersionImport) Check(fset *token.FileSet, file *ast.File) []cop.Offense {
-	if len(file.Imports) == 0 {
-		return nil
-	}
+// NewConfigVersionImport returns a fully configured ConfigVersionImport cop.
+func NewConfigVersionImport() *ConfigVersionImport {
+	return &ConfigVersionImport{Meta: cop.Meta{
+		CopName:     "Lint/ConfigVersionImport",
+		CopDesc:     "Config version packages must only import their immediate predecessor",
+		CopSeverity: cop.Error,
+	}}
+}
 
-	dir := configDir(fset.Position(file.Package).Filename)
-	if dir == "" {
-		return nil
+func (c *ConfigVersionImport) Check(p *cop.Pass) {
+	if len(p.File.Imports) == 0 {
+		return
 	}
 	// Black-box test files (package <dir>_test) are external to the package
 	// and may import what they please.
-	if strings.HasSuffix(file.Name.Name, "_test") {
-		return nil
+	if p.IsBlackBoxTest() {
+		return
 	}
 
+	dir := configDir(p.Filename())
+	if dir == "" {
+		return
+	}
 	dirVersion, isVersioned := versionFromDir(dir)
 	isLatest := dir == "latest"
 	if !isVersioned && !isLatest {
-		return nil
+		return
 	}
 
-	var offenses []cop.Offense
-	for _, imp := range file.Imports {
-		path := importPath(imp)
+	for _, imp := range p.File.Imports {
+		path := cop.ImportPath(imp)
 
 		if !strings.Contains(path, "pkg/config/") || strings.HasSuffix(path, "pkg/config/types") {
 			continue
 		}
 		if msg := importViolation(path, dirVersion, isLatest); msg != "" {
-			offenses = append(offenses, offense(c, fset, imp.Path, msg))
+			p.Report(imp.Path, "%s", msg)
 		}
 	}
-	return offenses
 }
 
 // importViolation returns a non-empty error message if the given import path