linters: introduce a few linters for catching pathological behavior

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

6 files changed, 204 insertions, 0 deletions

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.
+}