(feature): add nofloats linter

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

Author: everettraven

README.md

@@ -222,6 +222,14 @@ The `nobools` linter checks that fields in the API types do not contain a `bool`
 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.
 
+## NoFloats
+
+The `nofloats` linter checks that fields in the API types do not contain a `float32` or `float64` type.
+
+Floating-point values cannot be reliably round-tripped without changing and have varying precision and representation across languages and architectures.
+Their use should be avoided as much as possible.
+They should never be used in spec.
+
 ## 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/nofloats/analyzer.go

@@ -0,0 +1,58 @@
+package nofloats
+
+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 = "nofloats"
+
+var (
+	errCouldNotGetInspector = errors.New("could not get inspector")
+)
+
+// Analyzer is the analyzer for the nofloats package.
+// It checks that no struct fields are `float`.
+var Analyzer = &analysis.Analyzer{
+	Name:     name,
+	Doc:      "Float values cannot be reliably round-tripped without changing and have varying precisions and representations across languages and architectures.",
+	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 look at fields within structs.
+	// Filter typespecs so that we can look at type aliases.
+	nodeFilter := []ast.Node{
+		(*ast.StructType)(nil),
+		(*ast.TypeSpec)(nil),
+	}
+
+	typeChecker := utils.NewTypeChecker(checkFloat)
+
+	// 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 checkFloat(pass *analysis.Pass, ident *ast.Ident, node ast.Node, prefix string) {
+	if ident.Name == "float32" || ident.Name == "float64" {
+		pass.Reportf(node.Pos(), "%s should not use a float value because they cannot be reliably round-tripped.", prefix)
+	}
+}

pkg/analysis/nofloats/analyzer_test.go

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

pkg/analysis/nofloats/doc.go

@@ -0,0 +1,9 @@
+/*
+nofloats is an analyzer that ensures fields in the API types do not contain a `float32` or `float64` type.
+
+Floating-point values cannot be reliably round-tripped without changing
+and have varying precision and representation across languages and architectures.
+Their use should be avoided as much as possible.
+They should never be used in spec.
+*/
+package nofloats

pkg/analysis/nofloats/initializer.go

@@ -0,0 +1,33 @@
+package nofloats
+
+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 {
+	// Floats avoidance in the Kube conventions is a must for spec fields.
+	// Doesn't hurt to enforce it widely, uses outside of spec fields should be
+	// evaluated on an individual basis to determine if it is reasonable to override.
+	return true
+}

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

@@ -0,0 +1,93 @@
+package a
+
+type Floats struct {
+	ValidString string
+
+	ValidMap map[string]string
+
+	ValidInt32 int32
+
+	ValidInt64 int64
+
+	InvalidFloat32 float32 // want "field InvalidFloat32 should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat64 float64 // want "field InvalidFloat64 should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32Ptr *float32 // want "field InvalidFloat32Ptr pointer should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat64Ptr *float64 // want "field InvalidFloat64Ptr pointer should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32Slice []float32 // want "field InvalidFloat32Slice array element should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat64Slice []float64 // want "field InvalidFloat64Slice array element should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32PtrSlice []*float32 // want "field InvalidFloat32PtrSlice array element pointer should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat64PtrSlice []*float64 // want "field InvalidFloat64PtrSlice array element pointer should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32Alias Float32Alias // want "field InvalidFloat32Alias type Float32Alias should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat64Alias Float64Alias // want "field InvalidFloat64Alias type Float64Alias should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32PtrAlias *Float32Alias // want "field InvalidFloat32PtrAlias pointer type Float32Alias should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat64PtrAlias *Float64Alias // want "field InvalidFloat64PtrAlias pointer type Float64Alias should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32SliceAlias []Float32Alias // want "field InvalidFloat32SliceAlias array element type Float32Alias should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat64SliceAlias []Float64Alias // want "field InvalidFloat64SliceAlias array element type Float64Alias should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat32PtrSliceAlias []*Float32Alias // want "field InvalidFloat32PtrSliceAlias array element pointer type Float32Alias should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidFloat64PtrSliceAlias []*Float64Alias // want "field InvalidFloat64PtrSliceAlias array element pointer type Float64Alias should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidMapStringToFloat32 map[string]float32 // want "field InvalidMapStringToFloat32 map value should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidMapStringToFloat64 map[string]float64 // want "field InvalidMapStringToFloat64 map value should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidMapStringToFloat32Ptr map[string]*float32 // want "field InvalidMapStringToFloat32Ptr map value pointer should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidMapStringToFloat64Ptr map[string]*float64 // want "field InvalidMapStringToFloat64Ptr map value pointer should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidMapFloat32ToString map[float32]string // want "field InvalidMapFloat32ToString map key should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidMapFloat64ToString map[float64]string // want "field InvalidMapFloat64ToString map key should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidMapFloat32PtrToString map[*float32]string // want "field InvalidMapFloat32PtrToString map key pointer should not use a float value because they cannot be reliably round-tripped."
+
+	InvalidMapFloat64PtrToString map[*float64]string // want "field InvalidMapFloat64PtrToString map key pointer should not use a float value because they cannot be reliably round-tripped."
+}
+
+// DoNothingFloat32 is used to check that the analyser doesn't report on methods.
+func (Floats) DoNothingFloat32(a float32) float32 {
+	return a
+}
+
+// DoNothingFloat64 is used to check that the analyser doesn't report on methods.
+func (Floats) DoNothingFloat64(a float64) float64 {
+	return a
+}
+
+type Float32Alias float32 // want "type Float32Alias should not use a float value because they cannot be reliably round-tripped."
+
+type Float64Alias float64 // want "type Float64Alias should not use a float value because they cannot be reliably round-tripped."
+
+type Float32AliasPtr *float32 // want "type Float32AliasPtr pointer should not use a float value because they cannot be reliably round-tripped."
+
+type Float64AliasPtr *float64 // want "type Float64AliasPtr pointer should not use a float value because they cannot be reliably round-tripped."
+
+type Float32AliasSlice []float32 // want "type Float32AliasSlice array element should not use a float value because they cannot be reliably round-tripped."
+
+type Float64AliasSlice []float64 // want "type Float64AliasSlice array element should not use a float value because they cannot be reliably round-tripped."
+
+type Float32AliasPtrSlice []*float32 // want "type Float32AliasPtrSlice array element pointer should not use a float value because they cannot be reliably round-tripped."
+
+type Float64AliasPtrSlice []*float64 // want "type Float64AliasPtrSlice array element pointer should not use a float value because they cannot be reliably round-tripped."
+
+type MapStringToFloat32Alias map[string]float32 // want "type MapStringToFloat32Alias map value should not use a float value because they cannot be reliably round-tripped."
+
+type MapStringToFloat64Alias map[string]float64 // want "type MapStringToFloat64Alias map value should not use a float value because they cannot be reliably round-tripped."
+
+type MapStringToFloat32PtrAlias map[string]*float32 // want "type MapStringToFloat32PtrAlias map value pointer should not use a float value because they cannot be reliably round-tripped."
+
+type MapStringToFloat64PtrAlias map[string]*float64 // want "type MapStringToFloat64PtrAlias map value pointer should not use a float value because they cannot be reliably round-tripped."

pkg/analysis/registry.go

@@ -9,6 +9,7 @@ import (
 	"github.com/JoelSpeed/kal/pkg/analysis/jsontags"
 	"github.com/JoelSpeed/kal/pkg/analysis/maxlength"
 	"github.com/JoelSpeed/kal/pkg/analysis/nobools"
+	"github.com/JoelSpeed/kal/pkg/analysis/nofloats"
 	"github.com/JoelSpeed/kal/pkg/analysis/nophase"
 	"github.com/JoelSpeed/kal/pkg/analysis/optionalorrequired"
 	"github.com/JoelSpeed/kal/pkg/analysis/requiredfields"
@@ -61,6 +62,7 @@ func NewRegistry() Registry {
 			jsontags.Initializer(),
 			maxlength.Initializer(),
 			nobools.Initializer(),
+			nofloats.Initializer(),
 			nophase.Initializer(),
 			optionalorrequired.Initializer(),
 			requiredfields.Initializer(),

pkg/analysis/registry_test.go

@@ -20,6 +20,7 @@ var _ = Describe("Registry", func() {
 				"commentstart",
 				"integers",
 				"jsontags",
+				"nofloats",
 				"nophase",
 				"optionalorrequired",
 				"requiredfields",
@@ -37,6 +38,7 @@ var _ = Describe("Registry", func() {
 				"jsontags",
 				"maxlength",
 				"nobools",
+				"nofloats",
 				"nophase",
 				"optionalorrequired",
 				"requiredfields",