linters: introduce a few linters for catching pathological behavior

Signed-off-by: Xe Iaso <me@xeiaso.net>

9 files changed, 214 insertions, 6 deletions

diff --git a/go.mod b/go.mod
index 5ab726d..1f5da2b 100644
--- a/go.mod
+++ b/go.mod
@@ -156,13 +156,13 @@ require (
 	go4.org/netipx v0.0.0-20230303233057-f1b76eb4bb35 // indirect
 	golang.org/x/exp v0.0.0-20230425010034-47ecfdc1ba53 // indirect
 	golang.org/x/image v0.7.0 // indirect
-	golang.org/x/mod v0.10.0 // indirect
+	golang.org/x/mod v0.11.0 // indirect
 	golang.org/x/net v0.11.0 // indirect
 	golang.org/x/sys v0.9.0 // indirect
 	golang.org/x/term v0.9.0 // indirect
 	golang.org/x/text v0.10.0 // indirect
 	golang.org/x/time v0.3.0 // indirect
-	golang.org/x/tools v0.9.1 // indirect
+	golang.org/x/tools v0.10.0 // indirect
 	golang.zx2c4.com/wintun v0.0.0-20230126152724-0fa3db229ce2 // indirect
 	golang.zx2c4.com/wireguard/windows v0.5.3 // indirect
 	google.golang.org/appengine v1.6.7 // indirect
diff --git a/go.sum b/go.sum
index fd31f37..0d6e941 100644
--- a/go.sum
+++ b/go.sum
@@ -589,6 +589,8 @@ golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91
 golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/mod v0.10.0 h1:lFO9qtOdlre5W1jxS3r/4szv2/6iXxScdzjoBMXNhYk=
 golang.org/x/mod v0.10.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
+golang.org/x/mod v0.11.0 h1:bUO06HqtnRcc/7l71XBe4WcqTZ+3AH1J59zWDDwLKgU=
+golang.org/x/mod v0.11.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/net v0.0.0-20180218175443-cbe0f9307d01/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180811021610-c39426892332/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -837,6 +839,8 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc
 golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
 golang.org/x/tools v0.9.1 h1:8WMNJAz3zrtPmnYC7ISf5dEn3MT0gY7jBJfw27yrrLo=
 golang.org/x/tools v0.9.1/go.mod h1:owI94Op576fPu3cIGQeHs3joujW/2Oc6MtlxbF5dfNc=
+golang.org/x/tools v0.10.0 h1:tvDr/iQoUqNdohiYm0LmmKcBk+q86lb9EprIUFhHHGg=
+golang.org/x/tools v0.10.0/go.mod h1:UJwyiVBsOA2uwvK/e5OY3GTpDUJriEd+/YlqAwLPmyM=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
diff --git a/gomod2nix.toml b/gomod2nix.toml
index c091f9e..7f9dafb 100644
--- a/gomod2nix.toml
+++ b/gomod2nix.toml
@@ -392,8 +392,8 @@ schema = 3
     version = "v0.7.0"
     hash = "sha256-8ymOoG5nFlSCJOgn/apVzP4Zk5jtoYNLwd5TJKlP2X8="
   [mod."golang.org/x/mod"]
-    version = "v0.10.0"
-    hash = "sha256-g0T2wz+K0nhPWdVQJRGGqEqzlTHMBahv+9C3y090eIM="
+    version = "v0.11.0"
+    hash = "sha256-mSiFXtYlketLJEHmaxUHYzkXoR9M7nJPDSbNiBYAyNE="
   [mod."golang.org/x/net"]
     version = "v0.11.0"
     hash = "sha256-9MWuk+lNmiiwRNLswRuIp/hhFRfYXJMrH2/Bs7iRW4M="
@@ -416,8 +416,8 @@ schema = 3
     version = "v0.3.0"
     hash = "sha256-/hmc9skIswMYbivxNS7R8A6vCTUF9k2/7tr/ACkcEaM="
   [mod."golang.org/x/tools"]
-    version = "v0.9.1"
-    hash = "sha256-6jvh0cvdVMvGBceeBxWFj0+bFvboB8wuIYPHabxrST0="
+    version = "v0.10.0"
+    hash = "sha256-9ylXC8l7/hgsIW/WMKq/5PtE6fswd2zp783Z4UO5IT4="
   [mod."golang.zx2c4.com/wintun"]
     version = "v0.0.0-20230126152724-0fa3db229ce2"
     hash = "sha256-cjMLNjKnnupVROWmeASORVieAL9ieYdzX3cFzG8bCpo="
diff --git a/linters/cmd/documentfunctions/documentfunctions.go b/linters/cmd/documentfunctions/documentfunctions.go
new file mode 100644
index 0000000..684db20
--- /dev/null
+++ b/linters/cmd/documentfunctions/documentfunctions.go
@@ -0,0 +1,10 @@
+package main
+
+import (
+	"golang.org/x/tools/go/analysis/singlechecker"
+	"within.website/x/linters/documentfunctions"
+)
+
+func main() {
+	singlechecker.Main(documentfunctions.Analyzer)
+}
diff --git a/linters/cmd/nosleep/nosleep.go b/linters/cmd/nosleep/nosleep.go
new file mode 100644
index 0000000..6150644
--- /dev/null
+++ b/linters/cmd/nosleep/nosleep.go
@@ -0,0 +1,10 @@
+package main
+
+import (
+	"golang.org/x/tools/go/analysis/singlechecker"
+	"within.website/x/linters/nosleep"
+)
+
+func main() {
+	singlechecker.Main(nosleep.Analyzer)
+}
diff --git a/linters/documentfunctions/documentfunctions.go b/linters/documentfunctions/documentfunctions.go
new file mode 100644
index 0000000..0d05410
--- /dev/null
+++ b/linters/documentfunctions/documentfunctions.go
@@ -0,0 +1,61 @@
+// Package documentfunctions implements a linter that forces you to document functions.
+//
+// When you are dealing with code that requires multiple people to collaborate, your
+// code is not going to be "immediately obvious based on the implementation". You need
+// to document all functions that you make. This can help find untested invariants that
+// can lead to customers having a bad time.
+//
+// To disable this behavior, document your code.
+package documentfunctions
+
+import (
+	"go/ast"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+var Analyzer = &analysis.Analyzer{
+	Name: "documentfunctions",
+	Doc:  "Prevent the use of time.Sleep in test functions",
+	Run:  run,
+}
+
+var ignoreFunctions = []string{
+	"main", // main functions are probably okay
+}
+
+// run runs the analysis pass for documentfunctions. It performs a depth-first
+// traversal of all AST nodes in a file, looks for function definitions, and
+// then ensures all of them have comments.
+func run(pass *analysis.Pass) (interface{}, error) {
+	for _, file := range pass.Files {
+		pos := pass.Fset.Position(file.Pos())
+		if !strings.HasSuffix(".go", pos.Filename) {
+			continue
+		}
+		ast.Inspect(file, func(n ast.Node) bool {
+			fe, ok := n.(*ast.FuncDecl)
+			if !ok {
+				return true
+			}
+
+			for _, ignoreName := range ignoreFunctions {
+				if fe.Name.Name == ignoreName {
+					return true
+				}
+			}
+
+			if fe.Doc == nil {
+				if fe.Name.Name == "init" {
+					pass.Reportf(fe.Pos(), "init functions must have side effects documented")
+					return true
+				}
+				pass.Reportf(fe.Pos(), "function %s must have documentation", fe.Name.Name)
+			}
+
+			return true
+		})
+	}
+	return nil, nil
+}
diff --git a/linters/nosleep/nosleep.go b/linters/nosleep/nosleep.go
new file mode 100644
index 0000000..f5f0728
--- /dev/null
+++ b/linters/nosleep/nosleep.go
@@ -0,0 +1,97 @@
+// Package nosleep contains a Go linter that prevents the use of time.Sleep in tests.
+//
+// Time is not a synchronization mechanism. It is not an appropriate thing to use in
+// tests because undoubtedly the thing you are synchronizing on is going to depend on
+// some unspoken assumption that can and will randomly change. Do something else.
+//
+// If god is dead and you need to do this anyways, add a comment that reads:
+//
+//	// nosleep bypass(yournick): put a reason why here
+package nosleep
+
+import (
+	"fmt"
+	"go/ast"
+	"go/token"
+	"regexp"
+	"strings"
+
+	"golang.org/x/tools/go/analysis"
+)
+
+var Analyzer = &analysis.Analyzer{
+	Name: "nosleep",
+	Doc:  "Prevent the use of time.Sleep in test functions",
+	Run:  run,
+}
+
+var bypassRegex = regexp.MustCompile(`^// nosleep bypass\((\w+)\): ([\w \.\!\,]+)`)
+
+// nodeFuncName converts an ast.CallExpr into the human-readable name of
+// the function and name.
+func nodeFuncName(node *ast.CallExpr) string {
+	se, ok := node.Fun.(*ast.SelectorExpr)
+	if !ok {
+		return ""
+	}
+	ie, ok := se.X.(*ast.Ident)
+	if !ok {
+		return ""
+	}
+
+	return fmt.Sprintf("%s.%s", ie.Name, se.Sel.Name)
+}
+
+// run finds all time.Sleep calls and raises a linter warning unless there
+// is an appropriate magic comment.
+func run(pass *analysis.Pass) (interface{}, error) {
+	for _, file := range pass.Files {
+		ignore := findBypassComments(pass.Fset, file.Comments)
+
+		ast.Inspect(file, func(n ast.Node) bool {
+			ce, ok := n.(*ast.CallExpr)
+			if !ok {
+				return true
+			}
+			pos := pass.Fset.Position(ce.Pos())
+			fname := pos.Filename
+			line := pos.Line
+
+			for _, ignoreLine := range ignore {
+				if ignoreLine == line {
+					return true
+				}
+			}
+
+			if strings.HasSuffix(fname, "_test.go") {
+				funcName := nodeFuncName(ce)
+				if funcName == "time.Sleep" {
+					pass.Reportf(ce.Pos(), "use of time.Sleep in testing code")
+				}
+			}
+
+			return true
+		})
+	}
+
+	return nil, nil
+}
+
+// findBypassComments finds all the comments in the file that meet the bypassRegex
+// and then adds it to a slice of lines to ignore.
+func findBypassComments(fset *token.FileSet, cg []*ast.CommentGroup) []int {
+	var result []int
+	for _, g := range cg {
+		for _, c := range g.List {
+			text := bypassRegex.FindString(c.Text)
+			if text == "" {
+				continue
+			}
+
+			lnum := fset.Position(c.Pos()).Line
+			result = append(result, lnum)
+		}
+	}
+
+	return result
+}
diff --git a/linters/nosleep/nosleep_test.go b/linters/nosleep/nosleep_test.go
new file mode 100644
index 0000000..e6b38a0
--- /dev/null
+++ b/linters/nosleep/nosleep_test.go
@@ -0,0 +1,12 @@
+package nosleep
+
+import (
+	"testing"
+
+	"golang.org/x/tools/go/analysis/analysistest"
+)
+
+// TestNosleep ensures that the nosleep analyzer catches use of time.Sleep in testing code.
+func TestNosleep(t *testing.T) {
+	analysistest.Run(t, analysistest.TestData(), Analyzer)
+}
diff --git a/linters/nosleep/testdata/sleep_test.go b/linters/nosleep/testdata/sleep_test.go
new file mode 100644
index 0000000..0d47045
--- /dev/null
+++ b/linters/nosleep/testdata/sleep_test.go
@@ -0,0 +1,14 @@
+package testdata
+
+import (
+	"testing"
+	"time"
+)
+
+func TestNosleep(t *testing.T) {
+	time.Sleep(time.Second) // want "use of time.Sleep in testing code"
+}
+
+func TestNosleepIgnore(t *testing.T) {
+	time.Sleep(time.Second) // nosleep bypass(Mai): This test requires us to use a sleep statement here. I hate it too.
+}