Act under a lease, keep accounts by order, one writer at composition (hq to-be 45 Phase 2)
Two controllers could both act (issue 204), a reconcile's report could overtake the apply after it and the digest decided (issue 267), and a grant could make a second writer of a machine's report. - The lease (internal/lease, ADR 0229): mesh-controller_lease key `holder`, 15 s age, renewed every 5 s by compare-and-set; the epoch is the revision it was taken at. The gate is the clock (stops 3 s before expiry); a refused renewal is a loss and the process exits; a holder that stops gives it back. serve takes it before asserting the bus. Epochs kept in the store (migration 0068 controller_epoch) as a floor: a bucket raised from nothing is compacted past it. Unleased (no epoch, S12 urgent) only when nobody holds it and the bus will not let it be written. A shell command acts under the holder's epoch, or its own lease when none. - Declarations carry `epoch` inside the signed envelope, only to a machine whose latest account carried a report_sequence (mesh-host #35); would-send is composed with the epoch last sent. Allot and the send both pass the gate. - Reports: contract in internal/link/order.go (epoch, sequence, report_sequence, older_than, refused_older). Accounts kept by epoch, then sequence, then report sequence; older refused, counted; unordered reports keep the digest rule. Plans by compare-and-set on a revision, with epoch. Conditions and calls carry the epoch and are not written off the lease. - S12 and S13 (naming the writer by epoch) watched, D5 run; reset of the bucket said. Writers table compiled in and enforced in PermissionsFor; the controller no longer publishes mesh.control.>. A contract per consumed kind, and the empty-on-error lint over the repository. - mesh-host pinned to its main with the epoch in the validator (D1 validates the envelope as sent). Needs mesh-host's genesis lock with the lease grant (mesh-host PR) for TestTheInstallersFirstUserListIsWhatTheControllerWouldCompose.
This commit is contained in:
@@ -0,0 +1,237 @@
|
||||
// Package lint holds the checks this repository runs over its own source (novox/hq to-be 45 Phase 2).
|
||||
//
|
||||
// **Empty on error** (ADR 0227 rule 4, "nothing is dropped silently"): a reader that cannot read and
|
||||
// answers an empty collection is a reader whose caller cannot tell *nothing is there* from *I could not
|
||||
// tell* — issue 241's unreadable contributions file read as "nobody asks", and seven databases were
|
||||
// dropped. EmptyOnError finds every error branch that answers an empty collection with no error; the
|
||||
// test beside it fails the build on each one that does not say, where it does it, why the empty answer
|
||||
// is the truth (a `// empty-on-error: <why>` comment on the return or the line above).
|
||||
package lint
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/parser"
|
||||
"go/token"
|
||||
"io/fs"
|
||||
"path/filepath"
|
||||
"sort"
|
||||
"strings"
|
||||
)
|
||||
|
||||
// Allow is the comment that says, at the return, why an empty answer to an error is the truth.
|
||||
const Allow = "empty-on-error:"
|
||||
|
||||
// Finding is one error branch answering an empty collection.
|
||||
type Finding struct {
|
||||
File string
|
||||
Line int
|
||||
Func string
|
||||
// Allowed is the reason given where it was allowed; empty for a finding that is a failure.
|
||||
Allowed string
|
||||
}
|
||||
|
||||
func (f Finding) String() string { return fmt.Sprintf("%s:%d (%s)", f.File, f.Line, f.Func) }
|
||||
|
||||
// EmptyOnError walks every Go file under root but tests, vendor/ and testdata/, and answers every
|
||||
// return inside an `if <err> != nil` branch that answers an empty collection — nil, or a literal with no
|
||||
// elements, where the function answers a slice or a map — and no error: a nil error, or none declared.
|
||||
func EmptyOnError(root string) ([]Finding, error) {
|
||||
var out []Finding
|
||||
fset := token.NewFileSet()
|
||||
err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if d.IsDir() {
|
||||
switch d.Name() {
|
||||
case "vendor", "testdata", ".git", "node_modules":
|
||||
return filepath.SkipDir
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
|
||||
return nil
|
||||
}
|
||||
file, err := parser.ParseFile(fset, path, nil, parser.ParseComments)
|
||||
if err != nil {
|
||||
return fmt.Errorf("%s cannot be read: %w", path, err)
|
||||
}
|
||||
rel, _ := filepath.Rel(root, path)
|
||||
out = append(out, inFile(fset, file, rel)...)
|
||||
return nil
|
||||
})
|
||||
sort.Slice(out, func(i, j int) bool {
|
||||
if out[i].File != out[j].File {
|
||||
return out[i].File < out[j].File
|
||||
}
|
||||
return out[i].Line < out[j].Line
|
||||
})
|
||||
return out, err
|
||||
}
|
||||
|
||||
// inFile is every finding in one file.
|
||||
func inFile(fset *token.FileSet, file *ast.File, rel string) []Finding {
|
||||
// Every comment's text by the line it ends on, so an allowance is read on the return's line or the
|
||||
// line above it.
|
||||
comments := map[int]string{}
|
||||
for _, group := range file.Comments {
|
||||
for _, c := range group.List {
|
||||
comments[fset.Position(c.End()).Line] += c.Text
|
||||
}
|
||||
}
|
||||
var out []Finding
|
||||
var walk func(name string, results *ast.FieldList, body *ast.BlockStmt)
|
||||
walk = func(name string, results *ast.FieldList, body *ast.BlockStmt) {
|
||||
if body == nil {
|
||||
return
|
||||
}
|
||||
ast.Inspect(body, func(n ast.Node) bool {
|
||||
switch n := n.(type) {
|
||||
case *ast.FuncLit:
|
||||
// Its returns are its own, judged against its own results.
|
||||
walk(name+" (a function inside it)", n.Type.Results, n.Body)
|
||||
return false
|
||||
case *ast.IfStmt:
|
||||
if !errNotNil(n.Cond) {
|
||||
return true
|
||||
}
|
||||
for _, ret := range returnsIn(n.Body) {
|
||||
if !emptyOnError(results, ret) {
|
||||
continue
|
||||
}
|
||||
line := fset.Position(ret.Pos()).Line
|
||||
f := Finding{File: rel, Line: line, Func: name}
|
||||
for _, l := range []int{line, line - 1} {
|
||||
if text, ok := comments[l]; ok {
|
||||
if _, why, found := strings.Cut(text, Allow); found && strings.TrimSpace(why) != "" {
|
||||
f.Allowed = strings.TrimSpace(why)
|
||||
}
|
||||
}
|
||||
}
|
||||
out = append(out, f)
|
||||
}
|
||||
}
|
||||
return true
|
||||
})
|
||||
}
|
||||
for _, decl := range file.Decls {
|
||||
fn, ok := decl.(*ast.FuncDecl)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
name := fn.Name.Name
|
||||
if fn.Recv != nil && len(fn.Recv.List) > 0 {
|
||||
name = "(" + exprString(fn.Recv.List[0].Type) + ")." + name
|
||||
}
|
||||
walk(name, fn.Type.Results, fn.Body)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
// errNotNil is a condition `<x> != nil` where x is an error by its name: err, or one ending in Err.
|
||||
func errNotNil(cond ast.Expr) bool {
|
||||
found := false
|
||||
ast.Inspect(cond, func(n ast.Node) bool {
|
||||
b, ok := n.(*ast.BinaryExpr)
|
||||
if !ok || b.Op != token.NEQ {
|
||||
return true
|
||||
}
|
||||
x, xok := b.X.(*ast.Ident)
|
||||
y, yok := b.Y.(*ast.Ident)
|
||||
if xok && yok && y.Name == "nil" && (x.Name == "err" || strings.HasSuffix(x.Name, "Err")) {
|
||||
found = true
|
||||
}
|
||||
return !found
|
||||
})
|
||||
return found
|
||||
}
|
||||
|
||||
// returnsIn is the returns of a branch, not those of a function literal inside it.
|
||||
func returnsIn(body *ast.BlockStmt) []*ast.ReturnStmt {
|
||||
var out []*ast.ReturnStmt
|
||||
ast.Inspect(body, func(n ast.Node) bool {
|
||||
switch n := n.(type) {
|
||||
case *ast.FuncLit:
|
||||
return false
|
||||
case *ast.ReturnStmt:
|
||||
out = append(out, n)
|
||||
}
|
||||
return true
|
||||
})
|
||||
return out
|
||||
}
|
||||
|
||||
// emptyOnError says a return answers an empty collection and no error, for a function's results.
|
||||
func emptyOnError(results *ast.FieldList, ret *ast.ReturnStmt) bool {
|
||||
if results == nil {
|
||||
return false
|
||||
}
|
||||
var types []ast.Expr
|
||||
for _, f := range results.List {
|
||||
n := len(f.Names)
|
||||
if n == 0 {
|
||||
n = 1
|
||||
}
|
||||
for i := 0; i < n; i++ {
|
||||
types = append(types, f.Type)
|
||||
}
|
||||
}
|
||||
if len(ret.Results) != len(types) {
|
||||
return false // a bare return of named results, or a call answering them: not judged here
|
||||
}
|
||||
empty := false
|
||||
for i, t := range types {
|
||||
value := ret.Results[i]
|
||||
if isError(t) {
|
||||
if !isNil(value) {
|
||||
return false // an error is answered: said, not dropped
|
||||
}
|
||||
continue
|
||||
}
|
||||
if isCollection(t) && isEmpty(value) {
|
||||
empty = true
|
||||
}
|
||||
}
|
||||
return empty
|
||||
}
|
||||
|
||||
func isError(t ast.Expr) bool {
|
||||
id, ok := t.(*ast.Ident)
|
||||
return ok && id.Name == "error"
|
||||
}
|
||||
|
||||
func isCollection(t ast.Expr) bool {
|
||||
switch t := t.(type) {
|
||||
case *ast.ArrayType:
|
||||
return t.Len == nil
|
||||
case *ast.MapType:
|
||||
return true
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func isNil(e ast.Expr) bool {
|
||||
id, ok := e.(*ast.Ident)
|
||||
return ok && id.Name == "nil"
|
||||
}
|
||||
|
||||
func isEmpty(e ast.Expr) bool {
|
||||
if isNil(e) {
|
||||
return true
|
||||
}
|
||||
lit, ok := e.(*ast.CompositeLit)
|
||||
return ok && len(lit.Elts) == 0
|
||||
}
|
||||
|
||||
func exprString(e ast.Expr) string {
|
||||
switch e := e.(type) {
|
||||
case *ast.StarExpr:
|
||||
return "*" + exprString(e.X)
|
||||
case *ast.Ident:
|
||||
return e.Name
|
||||
case *ast.IndexExpr:
|
||||
return exprString(e.X)
|
||||
}
|
||||
return "?"
|
||||
}
|
||||
Reference in New Issue
Block a user