Add NoBools analyzer to prevent the usage of booleans

Author: JoelSpeed

README.md

@@ -194,6 +194,13 @@ lintersConfig:
     jsonTagRegex: "^[a-z][a-z0-9]*(?:[A-Z][a-z0-9]*)*$" # Provide a custom regex, which the json tag must match.
 ```
 
+## NoBools
+
+The `nobools` linter checks that fields in the API types do not contain a `bool` type.
+
+Booleans are limited and do not evolve well over time.
+It is recommended instead to create a string alias with meaningful values, as an enum.
+
 ## Nophase
 
 The `nophase` linter checks that the fields in the API types don't contain a 'Phase', or any field which contains 'Phase' as a substring, e.g MachinePhase.

pkg/analysis/nobools/analyzer.go

@@ -0,0 +1,58 @@
+package nobools
+
+import (
+	"errors"
+	"go/ast"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/utils"
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+const name = "nobools"
+
+var (
+	errCouldNotGetInspector = errors.New("could not get inspector")
+)
+
+// Analyzer is the analyzer for the nobools package.
+// It checks that no struct fields are `bool`.
+var Analyzer = &analysis.Analyzer{
+	Name:     name,
+	Doc:      "Boolean values cannot evolve over time, use an enum with meaningful values instead",
+	Run:      run,
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+}
+
+func run(pass *analysis.Pass) (interface{}, error) {
+	inspect, ok := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	if !ok {
+		return nil, errCouldNotGetInspector
+	}
+
+	// Filter to structs so that we can iterate over fields in a struct.
+	// Filter typespecs so that we can look at type aliases.
+	nodeFilter := []ast.Node{
+		(*ast.StructType)(nil),
+		(*ast.TypeSpec)(nil),
+	}
+
+	typeChecker := utils.NewTypeChecker(checkBool)
+
+	// Preorder visits all the nodes of the AST in depth-first order. It calls
+	// f(n) for each node n before it visits n's children.
+	//
+	// We use the filter defined above, ensuring we only look at struct fields and type declarations.
+	inspect.Preorder(nodeFilter, func(n ast.Node) {
+		typeChecker.CheckNode(pass, n)
+	})
+
+	return nil, nil //nolint:nilnil
+}
+
+func checkBool(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) {
+	if ident.Name == "bool" {
+		pass.Reportf(node.Pos(), "%s should not use a bool. Use a string type with meaningful constant values as an enum.", prefix)
+	}
+}

pkg/analysis/nobools/analyzer_test.go

@@ -0,0 +1,13 @@
+package nobools_test
+
+import (
+	"testing"
+
+	"github.com/JoelSpeed/kal/pkg/analysis/nobools"
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+func Test(t *testing.T) {
+	testdata := analysistest.TestData()
+	analysistest.Run(t, testdata, nobools.Analyzer, "a")
+}

pkg/analysis/nobools/doc.go

@@ -0,0 +1,15 @@
+/*
+nobools is an analyzer that checks for usage of bool types.
+
+Boolean values can only ever have 2 states, true or false.
+Over time, needs may change, and with a bool type, there is no way to add additional states.
+This problem then leads to pairs of bools, where values of one are only valid given the value of another.
+This is confusing and error-prone.
+
+It is recommended instead to use a string type with a set of constants to represent the different states,
+creating an enum.
+
+By using an enum, not only can you provide meaningul names for the various states of the API,
+but you can also add additional states in the future without breaking the API.
+*/
+package nobools

pkg/analysis/nobools/initializer.go

@@ -0,0 +1,32 @@
+package nobools
+
+import (
+	"github.com/JoelSpeed/kal/pkg/config"
+	"golang.org/x/tools/go/analysis"
+)
+
+// Initializer returns the AnalyzerInitializer for this
+// Analyzer so that it can be added to the registry.
+func Initializer() initializer {
+	return initializer{}
+}
+
+// intializer implements the AnalyzerInitializer interface.
+type initializer struct{}
+
+// Name returns the name of the Analyzer.
+func (initializer) Name() string {
+	return name
+}
+
+// Init returns the intialized Analyzer.
+func (initializer) Init(cfg config.LintersConfig) (*analysis.Analyzer, error) {
+	return Analyzer, nil
+}
+
+// Default determines whether this Analyzer is on by default, or not.
+func (initializer) Default() bool {
+	// Bools avoidance in the Kube conventions is not a must.
+	// Make this opt in depending on the projects own preference.
+	return false
+}

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

@@ -0,0 +1,35 @@
+package a
+
+type Integers struct {
+	ValidString string
+
+	ValidMap map[string]string
+
+	ValidInt32 int32
+
+	ValidInt64 int64
+
+	InvalidBool bool // want "field InvalidBool should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolPtr *bool // want "field InvalidBoolPtr pointer should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolSlice []bool // want "field InvalidBoolSlice array element should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolPtrSlice []*bool // want "field InvalidBoolPtrSlice array element pointer should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolAlias BoolAlias // want "field InvalidBoolAlias type BoolAlias should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolPtrAlias *BoolAlias // want "field InvalidBoolPtrAlias pointer type BoolAlias should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolSliceAlias []BoolAlias // want "field InvalidBoolSliceAlias array element type BoolAlias should not use a bool. Use a string type with meaningful constant values as an enum."
+
+	InvalidBoolPtrSliceAlias []*BoolAlias // want "field InvalidBoolPtrSliceAlias array element pointer type BoolAlias should not use a bool. Use a string type with meaningful constant values as an enum."
+}
+
+type BoolAlias bool // want "type BoolAlias should not use a bool. Use a string type with meaningful constant values as an enum."
+
+type BoolAliasPtr *bool // want "type BoolAliasPtr pointer should not use a bool. Use a string type with meaningful constant values as an enum."
+
+type BoolAliasSlice []bool // want "type BoolAliasSlice array element should not use a bool. Use a string type with meaningful constant values as an enum."
+
+type BoolAliasPtrSlice []*bool // want "type BoolAliasPtrSlice array element pointer should not use a bool. Use a string type with meaningful constant values as an enum."

pkg/analysis/registry.go

@@ -7,6 +7,7 @@ import (
 	"github.com/JoelSpeed/kal/pkg/analysis/conditions"
 	"github.com/JoelSpeed/kal/pkg/analysis/integers"
 	"github.com/JoelSpeed/kal/pkg/analysis/jsontags"
+	"github.com/JoelSpeed/kal/pkg/analysis/nobools"
 	"github.com/JoelSpeed/kal/pkg/analysis/nophase"
 	"github.com/JoelSpeed/kal/pkg/analysis/optionalorrequired"
 	"github.com/JoelSpeed/kal/pkg/analysis/requiredfields"
@@ -56,6 +57,7 @@ func NewRegistry() Registry {
 			commentstart.Initializer(),
 			integers.Initializer(),
 			jsontags.Initializer(),
+			nobools.Initializer(),
 			nophase.Initializer(),
 			optionalorrequired.Initializer(),
 			requiredfields.Initializer(),

pkg/analysis/registry_test.go

@@ -35,6 +35,7 @@ var _ = Describe("Registry", func() {
 				"commentstart",
 				"integers",
 				"jsontags",
+				"nobools",
 				"nophase",
 				"optionalorrequired",
 				"requiredfields",