amadan

carlos / platform Public

copy gate fix round 4: no error text in a placeholder, field-read limits propagate, multi-value field writes, closure aliases, shadowed receivers

The five routes the round-3 re-review left open (ruling T15-scope: a lint
against drift, the realistic routes closed, the rest documented at the top
of the analyser):

- A format argument or concatenation operand that is an error's text is a
  hard violation even when the format matches a row: a call to Error(), a
  value typed error or implementing it, an untyped value named like an
  error, a local or helper parameter holding one, and any %w argument.
- A Reason field read as a placeholder keeps its writes' hard problems.
- A field write that takes one result of a multi-value expression is a
  non-sentence write.
- A closure configured in movesLocalSinks is a sentence-flow function, so
  an alias of it (send := fail) is refused.
- A method call whose receiver is declared more than once in the function
  is refused.

Each route has a negative fixture. The real tree passes with no code change.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude f7a2453097cc8d43223de92247e20b9d8af816c7 parent e742cd3
11 files changed, +609 −22
  • approved_moves_copy_scan_test.go +361 −22
  • approved_moves_copy_test.go +56 −0
  • testdata/moves-copy/sinks/closure-alias/emit.go +20 −0
  • testdata/moves-copy/sinks/error-text/emit.go +56 −0
  • testdata/moves-copy/sinks/error-text/record.md +11 −0
  • testdata/moves-copy/sinks/field-limit/emit.go +27 −0
  • testdata/moves-copy/sinks/field-multi/emit.go +25 −0
  • testdata/moves-copy/sinks/ok/emit.go +26 −0
  • testdata/moves-copy/sinks/ok/record.md +3 −0
  • testdata/moves-copy/sinks/shadow-receiver/emit.go +21 −0
  • testdata/moves-copy/sinks/shadow-receiver/record.md +3 −0
diff --git a/approved_moves_copy_scan_test.go b/approved_moves_copy_scan_test.go
index 5f6b8bba2..fde592418 100644
--- a/approved_moves_copy_scan_test.go
+++ b/approved_moves_copy_scan_test.go
@@ -28,10 +28,45 @@ import (
// unrecorded sentence: it is never silently dropped. Hitting a limit is a
// violation that names file:line.
//
-// The one place it does not look is a placeholder: an operand of a
-// concatenation or an argument of a Sprintf may be any value. The row it is
-// matched against must carry a <placeholder> at that spot, and the record is
-// where a human vouches for it.
+// A placeholder (an operand of a concatenation, an argument of a Sprintf) may
+// be data, such as a host name: the row it is matched against must carry a
+// <placeholder> at that spot, and the record is where a human vouches for it.
+// But a placeholder never carries an error's text. A call to Error(), a value
+// whose declared type is error or implements it, a value the gate cannot type
+// that is named err or ends in Err or err, a local or helper parameter that
+// holds any of these, and the argument of a %w verb are all violations there,
+// even when the format matches a row.
+//
+// What it is (ruling T15-scope): a lint against accidental drift into
+// operator copy, not a boundary against code written to evade it. The
+// whole-branch review and code review cover deliberate constructions. These
+// are its known limits:
+//
+// - Types are syntactic and same-package. It does not follow generics,
+// embedded structs, type aliases, or methods promoted through embedding.
+// A value of another package's type is an error only by its type's name
+// (one ending in Error) or, when its type is unknown, by its own name.
+// An error held in a variable of an unrelated name and an unknown type
+// (x := pkg.Do(); Sprintf("%v", x)) is read as data.
+// - Receivers are not lexically scoped. A method call whose receiver
+// identifier is declared more than once in the function is refused; a
+// receiver declared once is resolved by its one declaration.
+// - Struct fields written by JSON decoding, reflection, through a pointer
+// to the field (p := &x.Reason; *p = ...), with +=, or in files outside
+// movesScanFiles are not seen. A field read reflects the writes it can
+// see. A write that takes one result of a multi-value expression is a
+// non-sentence write.
+// - A Reason read as a placeholder whose writes fail only softly (a
+// variable the gate cannot read, not an error's text or a limit) is a
+// wildcard, as any other placeholder is.
+// - Function values: a direct reference to a sentence-flow function (a
+// package function, a method, or a closure configured in movesLocalSinks)
+// that is not a call is refused. A closure that is not configured is not
+// tracked as a value; its own parameters are unbound at its sinks, which
+// refuses them there.
+// - Anything that leaves the scanned files (a map or channel carrying text
+// from elsewhere, a global set at init) is read as far as the files say
+// and no further.
// movesSentenceFields are the struct fields that carry a sentence from the
// function that makes it to the one that speaks it. A read of one as a
@@ -290,6 +325,7 @@ type fieldWrite struct {
expr ast.Expr
ctype string // the composite literal's struct type, when it names one
recv ast.Expr // for an assignment, the expression whose field is written
+ multi bool // one result of a multi-value expression: not provable
}
type fieldResult struct {
@@ -404,16 +440,25 @@ func newUniverse(t *testing.T, root string, files []string) *movesUniverse {
for i, el := range n.Elts {
if kv, ok := el.(*ast.KeyValueExpr); ok {
if k, ok := kv.Key.(*ast.Ident); ok {
- u.fieldWrites[k.Name] = append(u.fieldWrites[k.Name], &fieldWrite{f, fd, kv.Value, ctype, nil})
+ u.fieldWrites[k.Name] = append(u.fieldWrites[k.Name], &fieldWrite{file: f, fd: fd, expr: kv.Value, ctype: ctype})
}
} else if i < len(order) {
- u.fieldWrites[order[i]] = append(u.fieldWrites[order[i]], &fieldWrite{f, fd, el, ctype, nil})
+ u.fieldWrites[order[i]] = append(u.fieldWrites[order[i]], &fieldWrite{file: f, fd: fd, expr: el, ctype: ctype})
}
}
case *ast.AssignStmt:
for i, l := range n.Lhs {
- if s, ok := l.(*ast.SelectorExpr); ok && i < len(n.Rhs) && len(n.Rhs) == len(n.Lhs) {
- u.fieldWrites[s.Sel.Name] = append(u.fieldWrites[s.Sel.Name], &fieldWrite{f, fd, n.Rhs[i], "", s.X})
+ s, ok := l.(*ast.SelectorExpr)
+ if !ok {
+ continue
+ }
+ if len(n.Rhs) == len(n.Lhs) {
+ u.fieldWrites[s.Sel.Name] = append(u.fieldWrites[s.Sel.Name], &fieldWrite{file: f, fd: fd, expr: n.Rhs[i], recv: s.X})
+ } else {
+ // box.Reason, _ = detail(err) or box.Reason, ok = m[k]: one
+ // result of a multi-value expression, which the gate cannot
+ // prove is a sentence. The expression is the field written.
+ u.fieldWrites[s.Sel.Name] = append(u.fieldWrites[s.Sel.Name], &fieldWrite{file: f, fd: fd, expr: s, recv: s.X, multi: true})
}
}
}
@@ -454,6 +499,7 @@ type evalCtx struct {
bind map[string][]string
fwd map[string]bool
fwdFn map[string]bool
+ errParams map[string]string // a helper's parameters bound to an error's text, and why
curReturn *ast.ReturnStmt
noCalls bool
visiting map[string]bool
@@ -461,7 +507,7 @@ type evalCtx struct {
func (u *movesUniverse) newCtx(file string, fd *ast.FuncDecl, noCalls bool) *evalCtx {
c := &evalCtx{u: u, file: file, fd: fd, locals: localAssigns(fd), bind: map[string][]string{},
- fwd: map[string]bool{}, fwdFn: map[string]bool{}, noCalls: noCalls, visiting: map[string]bool{}}
+ fwd: map[string]bool{}, fwdFn: map[string]bool{}, errParams: map[string]string{}, noCalls: noCalls, visiting: map[string]bool{}}
c.pkg = loadMovesPkg(u.t, u.at(filepath.Dir(file)))
if fd == nil {
return c
@@ -540,17 +586,20 @@ func dedupe(in []string) []string {
// operand reads a placeholder: any value is allowed, so what it cannot derive
// becomes one wildcard, which the record must bind to a <placeholder>. Only a
-// limit or a cycle (a hard problem) still counts.
+// hard problem (a limit, a cycle, an error's text) still counts.
func (c *evalCtx) operand(e ast.Expr, depth int) ([]string, []problem) {
if sel, isField := e.(*ast.SelectorExpr); isField && !c.noCalls {
// A field read as a placeholder is a wildcard for the record's
// <placeholder>, except a field that carries a sentence from one function
// to another (a Check's Reason): that is forwarded when every write to it
- // is a sentence.
+ // is a sentence. A write's hard problem (a limit, an error's text, a
+ // multi-value write) is never covered by the wildcard.
if movesSentenceFields[sel.Sel.Name] {
- if a, p := c.fieldRead(sel); len(p) == 0 {
+ a, p := c.fieldRead(sel)
+ if len(p) == 0 {
return a, nil
}
+ return []string{wild}, hardOnly(p)
}
return []string{wild}, nil
}
@@ -561,6 +610,166 @@ func (c *evalCtx) operand(e ast.Expr, depth int) ([]string, []problem) {
return []string{wild}, hardOnly(probs)
}
+// concatOperand reads one side of a +: a placeholder, unless it is an error's
+// text. A side that is itself a concatenation is checked by its own eval.
+func (c *evalCtx) concatOperand(e ast.Expr, depth int) ([]string, []problem) {
+ if !isConcat(e) {
+ if why := c.errorText(e, 0); why != "" {
+ return []string{wild}, []problem{errorTextProblem(e, why)}
+ }
+ }
+ return c.operand(e, depth)
+}
+
+func isConcat(e ast.Expr) bool {
+ switch e := e.(type) {
+ case *ast.ParenExpr:
+ return isConcat(e.X)
+ case *ast.BinaryExpr:
+ return e.Op == token.ADD
+ }
+ return false
+}
+
+func errorTextProblem(e ast.Expr, why string) problem {
+ return problem{fmt.Sprintf("%s puts an error's text in a sentence (%s)", render(e), why), true}
+}
+
+// errName is the name an error usually goes by: err, or one ending in Err or
+// err. It is used only when the gate cannot type the value.
+func errName(n string) bool {
+ return strings.HasSuffix(n, "err") || strings.HasSuffix(n, "Err")
+}
+
+// isErrorType reports whether a declared type is error or implements it: the
+// error interface, a package type with an Error method, a package interface
+// that embeds error or declares Error, or another package's type whose name
+// ends in Error.
+func (c *evalCtx) isErrorType(t ast.Expr) bool {
+ switch t := unstar(t).(type) {
+ case *ast.Ident:
+ if t.Name == "error" {
+ return true
+ }
+ for _, d := range c.pkg.methods["Error"] {
+ if recvBase(d) == t.Name {
+ return true
+ }
+ }
+ if it, ok := c.pkg.types[t.Name].(*ast.InterfaceType); ok && it.Methods != nil {
+ for _, m := range it.Methods.List {
+ if id, ok := m.Type.(*ast.Ident); ok && id.Name == "error" && len(m.Names) == 0 {
+ return true
+ }
+ for _, n := range m.Names {
+ if n.Name == "Error" {
+ return true
+ }
+ }
+ }
+ }
+ case *ast.SelectorExpr:
+ return strings.HasSuffix(t.Sel.Name, "Error")
+ }
+ return false
+}
+
+// errorText says why e is an error's text, or "" when the gate has no reason to
+// think so. It reads the expression as written and the locals it names.
+func (c *evalCtx) errorText(e ast.Expr, depth int) string {
+ if depth > maxEvalDepth {
+ return "the gate ran out of depth deciding whether it is an error's text"
+ }
+ switch e := e.(type) {
+ case *ast.ParenExpr:
+ return c.errorText(e.X, depth+1)
+ case *ast.BinaryExpr:
+ if e.Op != token.ADD {
+ return ""
+ }
+ if why := c.errorText(e.X, depth+1); why != "" {
+ return why
+ }
+ return c.errorText(e.Y, depth+1)
+ case *ast.CallExpr:
+ if sel, ok := e.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "Error" && len(e.Args) == 0 {
+ return "it calls Error"
+ }
+ if t := c.typeExprOf(e, 0); t != nil && c.isErrorType(t) {
+ return "its result is an error"
+ }
+ // A call out of the package (fmt.Sprint, strings.TrimSpace) carries the
+ // text of its arguments. A package helper is read by eval instead.
+ if _, p := c.resolveCallees(e); p != nil {
+ for _, a := range e.Args {
+ if why := c.errorText(a, depth+1); why != "" {
+ return fmt.Sprintf("it is made from %s, and %s", render(a), why)
+ }
+ }
+ }
+ case *ast.Ident:
+ if why, ok := c.errParams[e.Name]; ok {
+ return why
+ }
+ t := c.typeExprOf(e, 0)
+ if t != nil && c.isErrorType(t) {
+ return "its type is error"
+ }
+ key := fmt.Sprintf("errtext %p %s", c.fd, e.Name)
+ if !c.visiting[key] {
+ c.visiting[key] = true
+ defer delete(c.visiting, key)
+ for _, a := range c.locals[e.Name] {
+ switch {
+ case a.rhs != nil:
+ if why := c.errorText(a.rhs, depth+1); why != "" {
+ return fmt.Sprintf("it is assigned %s at %s, and %s", render(a.rhs), c.posOf(a.rhs), why)
+ }
+ case a.call != nil:
+ if decls, p := c.resolveCallees(a.call); p == nil && len(decls) > 0 && decls[0].Type != nil {
+ if rt := resultTypeAt(decls[0], a.pos); rt != nil && c.isErrorType(rt) {
+ return fmt.Sprintf("it is assigned an error result at %s", c.posOf(a.call))
+ }
+ }
+ }
+ }
+ }
+ if t == nil && errName(e.Name) {
+ return "the gate cannot type it, and it is named as an error"
+ }
+ case *ast.SelectorExpr:
+ t := c.typeExprOf(e, 0)
+ if t != nil && c.isErrorType(t) {
+ return "its type is error"
+ }
+ if t == nil && errName(e.Sel.Name) {
+ return "the gate cannot type it, and it is named as an error"
+ }
+ }
+ return ""
+}
+
+// wrapArgs returns the argument indexes (into the call's Args, the format at
+// 0) of a format's %w verbs. ok is false when the format's arguments cannot be
+// counted (an explicit argument index).
+func wrapArgs(format string) (idx []int, ok bool) {
+ if strings.Contains(format, "%[") {
+ return nil, !strings.Contains(format, "w")
+ }
+ arg := 1
+ for _, v := range formatVerb.FindAllString(format, -1) {
+ if v == "%%" {
+ continue
+ }
+ arg += strings.Count(v, "*")
+ if strings.HasSuffix(v, "w") {
+ idx = append(idx, arg)
+ }
+ arg++
+ }
+ return idx, true
+}
+
// eval returns the texts e can be and why it cannot be read.
func (c *evalCtx) eval(e ast.Expr, depth int) ([]string, []problem) {
if depth > maxEvalDepth {
@@ -590,16 +799,15 @@ func (c *evalCtx) eval(e ast.Expr, depth int) ([]string, []problem) {
if e.Op != token.ADD {
return soft("%s is not a sentence", render(e))
}
- l, lp := c.operand(e.X, depth+1)
- r, rp := c.operand(e.Y, depth+1)
- if len(lp)+len(rp) > 0 {
- return nil, append(lp, rp...)
- }
+ l, lp := c.concatOperand(e.X, depth+1)
+ r, rp := c.concatOperand(e.Y, depth+1)
out, ok := cross(l, r)
if !ok {
- return nil, limit(c, e, "too many alternative sentences")
+ return nil, append(append(lp, rp...), limit(c, e, "too many alternative sentences")...)
}
- return dedupe(out), nil
+ // The text is kept beside the problems, so a refused sentence is still
+ // named by what it says.
+ return dedupe(out), append(lp, rp...)
case *ast.SelectorExpr:
if c.noCalls {
return []string{wild}, nil
@@ -707,6 +915,10 @@ func (c *evalCtx) fieldRead(e *ast.SelectorExpr) ([]string, []problem) {
continue
}
n++
+ if w.multi {
+ res.probs = append(res.probs, problem{fmt.Sprintf("a write to field %s at %s takes one result of a multi-value expression, which the gate cannot prove is a sentence", f, wc.posOf(w.expr)), true})
+ continue
+ }
alts, probs := wc.eval(w.expr, 0)
for _, p := range probs {
res.probs = append(res.probs, problem{fmt.Sprintf("a write to field %s at %s is not a sentence: %s", f, wc.posOf(w.expr), p.msg), p.hard})
@@ -758,13 +970,34 @@ func (c *evalCtx) evalCall(e *ast.CallExpr, pos, depth int) ([]string, []problem
return nil, append([]problem{{fmt.Sprintf("%s has a format that is not a literal", render(e)), false}}, probs...)
}
var out []string
+ var argProbs []problem
+ flagged := map[int]bool{}
+ for i, a := range e.Args[1:] {
+ if why := c.errorText(a, 0); why != "" {
+ flagged[i+1] = true
+ argProbs = append(argProbs, errorTextProblem(a, why))
+ }
+ }
for _, f := range fmts {
if strings.Contains(f, wild) || strings.Contains(f, fwdMark) {
return soft("%s has a format that is not a literal", render(e))
}
out = append(out, formatPattern(f))
+ idx, ok := wrapArgs(f)
+ if !ok {
+ argProbs = append(argProbs, problem{fmt.Sprintf("%s has a %%w verb whose argument the gate cannot count", render(e)), true})
+ }
+ for _, i := range idx {
+ switch {
+ case i >= len(e.Args):
+ argProbs = append(argProbs, problem{fmt.Sprintf("%s has a %%w verb with no argument", render(e)), true})
+ case !flagged[i]:
+ flagged[i] = true
+ argProbs = append(argProbs, problem{fmt.Sprintf("%s is the argument of a %%w verb, an error", render(e.Args[i])), true})
+ }
+ }
}
- return dedupe(out), nil
+ return dedupe(out), argProbs
}
if c.noCalls {
return []string{wild}, nil
@@ -831,7 +1064,13 @@ func (c *evalCtx) resolveCallees(e *ast.CallExpr) ([]*ast.FuncDecl, *problem) {
return bad("%s(...) is not a function of this package", fn.Name)
case *ast.SelectorExpr:
if fn.Sel.Name == "Error" && len(e.Args) == 0 {
- return bad("%s forwards an error's text", render(e))
+ // Hard: an error's text is never a placeholder either.
+ return nil, &problem{fmt.Sprintf("%s forwards an error's text", render(e)), true}
+ }
+ if x, ok := fn.X.(*ast.Ident); ok && c.fd != nil && declCount(c.fd, x.Name) > 1 {
+ // The gate types a receiver by its first declaration. A receiver
+ // declared again (if m := supplied; ...) may be another type.
+ return nil, &problem{fmt.Sprintf("%s at %s: the receiver %s is declared more than once in %s, so the gate cannot tell which one this is", render(e), c.posOf(e), x.Name, c.fd.Name.Name), true}
}
tx := c.typeExprOf(fn.X, 0)
t := baseTypeName(tx)
@@ -855,6 +1094,61 @@ func (c *evalCtx) resolveCallees(e *ast.CallExpr) ([]*ast.FuncDecl, *problem) {
return bad("%s is not a call the gate can read", render(e))
}
+// declCount counts the declarations of name in fd: its receiver, parameters
+// and named results, and in its body every := , var, range variable and
+// closure parameter or result. A := that redeclares in the same scope counts
+// too; the gate does not scope.
+func declCount(fd *ast.FuncDecl, name string) int {
+ n := 0
+ fields := func(fl *ast.FieldList) {
+ if fl == nil {
+ return
+ }
+ for _, f := range fl.List {
+ for _, id := range f.Names {
+ if id.Name == name {
+ n++
+ }
+ }
+ }
+ }
+ fields(fd.Recv)
+ fields(fd.Type.Params)
+ fields(fd.Type.Results)
+ if fd.Body == nil {
+ return n
+ }
+ ident := func(e ast.Expr) {
+ if id, ok := e.(*ast.Ident); ok && id.Name == name {
+ n++
+ }
+ }
+ ast.Inspect(fd.Body, func(x ast.Node) bool {
+ switch x := x.(type) {
+ case *ast.AssignStmt:
+ if x.Tok == token.DEFINE {
+ for _, l := range x.Lhs {
+ ident(l)
+ }
+ }
+ case *ast.ValueSpec:
+ for _, id := range x.Names {
+ ident(id)
+ }
+ case *ast.RangeStmt:
+ if x.Tok == token.DEFINE {
+ ident(x.Key)
+ ident(x.Value)
+ }
+ case *ast.FuncLit:
+ fields(x.Type.Params)
+ fields(x.Type.Results)
+ }
+ return true
+ })
+ return n
+}
+
// evalTrimPrefix reads strings.TrimPrefix(x, "lit"): the sentences x can be,
// with the prefix taken off those that have it.
func (c *evalCtx) evalTrimPrefix(e *ast.CallExpr, depth int) ([]string, []problem) {
@@ -1008,6 +1302,11 @@ func (c *evalCtx) callDecl(d *ast.FuncDecl, call *ast.CallExpr, pos, depth int)
a, pr := c.operand(call.Args[i], depth+1)
hc.bind[p] = a
probs = append(probs, pr...)
+ // An argument that is an error's text may still only select among
+ // sentences; it is refused where the helper puts it in one.
+ if why := c.errorText(call.Args[i], 0); why != "" {
+ hc.errParams[p] = fmt.Sprintf("it is bound to %s at %s, and %s", render(call.Args[i]), c.posOf(call.Args[i]), why)
+ }
} else {
hc.bind[p] = []string{wild}
}
@@ -1537,12 +1836,52 @@ func funcValueRefs(t *testing.T, u *movesUniverse) []funcRef {
if err != nil {
t.Fatal(err)
}
+ // A closure configured as a local sink (verifyArchive's fail) is a
+ // sentence-flow function of this file: an alias of it (send := fail)
+ // escapes the checks at its call sites.
+ closures := map[string]bool{}
+ ast.Inspect(af, func(n ast.Node) bool {
+ var names []*ast.Ident
+ var values []ast.Expr
+ switch n := n.(type) {
+ case *ast.AssignStmt:
+ for _, l := range n.Lhs {
+ id, _ := l.(*ast.Ident)
+ names = append(names, id)
+ }
+ values = n.Rhs
+ case *ast.ValueSpec:
+ names, values = n.Names, n.Values
+ }
+ if len(names) != len(values) {
+ return true
+ }
+ for i, id := range names {
+ if _, lit := values[i].(*ast.FuncLit); lit && id != nil {
+ if _, ok := movesLocalSinks[f][id.Name]; ok {
+ closures[id.Name] = true
+ }
+ }
+ }
+ return true
+ })
callFun := map[ast.Node]bool{}
skip := map[ast.Node]bool{}
ast.Inspect(af, func(n ast.Node) bool {
switch n := n.(type) {
case *ast.CallExpr:
callFun[n.Fun] = true
+ case *ast.AssignStmt:
+ // The name being defined or assigned is not a use of its value.
+ for _, l := range n.Lhs {
+ if id, ok := l.(*ast.Ident); ok {
+ skip[id] = true
+ }
+ }
+ case *ast.ValueSpec:
+ for _, id := range n.Names {
+ skip[id] = true
+ }
case *ast.FuncDecl:
skip[n.Name] = true
case *ast.SelectorExpr:
@@ -1558,7 +1897,7 @@ func funcValueRefs(t *testing.T, u *movesUniverse) []funcRef {
if skip[n] || callFun[n] {
return true
}
- if flow[n.Name] && declared[n.Name] {
+ if flow[n.Name] && declared[n.Name] || closures[n.Name] {
out = append(out, funcRef{f, n.Name, fmt.Sprintf("%s: %s is used as a value, not called, so a call through it escapes the argument checks", movesFset.Position(n.Pos()), n.Name)})
}
}
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go
index c104360e8..34ca1193a 100644
--- a/approved_moves_copy_test.go
+++ b/approved_moves_copy_test.go
@@ -1116,8 +1116,11 @@ func TestMovesSinkFixtures(t *testing.T) {
"emit.go:18 (Check.Reason in report): helper why, at ",
}, 2},
{"sprintf-v", []string{
+ "emit.go:9 (jsonError in viaSprintf): err puts an error's text in a sentence",
"emit.go:9 (jsonError in viaSprintf): a value with no text of its own",
+ "emit.go:13 (jsonError in viaConcat): err.Error(...) puts an error's text in a sentence",
"emit.go:13 (jsonError in viaConcat): a value with no text of its own",
+ "emit.go:17 (jsonError in viaVerbs): err puts an error's text in a sentence",
"emit.go:17 (jsonError in viaVerbs): a value with no text of its own",
}, 3},
{"field-reason", []string{
@@ -1138,6 +1141,7 @@ func TestMovesSinkFixtures(t *testing.T) {
"emit.go:7 (jsonError in refuse): helper say, at testdata/moves-copy/sinks/helper-bad-return/emit.go:14:2: err.Error(...) forwards an error's text",
}, 2},
{"param", []string{
+ "emit.go:7 (jsonError in refuse): err.Error(...) forwards an error's text",
"emit.go:7 (jsonError in refuse): a value with no text of its own",
}, 2},
{"substitution", nil, 1},
@@ -1165,6 +1169,32 @@ func TestMovesSinkFixtures(t *testing.T) {
"emit.go:18 (jsonError in dynamicFormat): parameter format is not bound to an argument",
"emit.go:22 (jsonError in field): r.Message: no struct in the scanned files declares a field Message",
}, 5},
+ // Fix round 4. Every format in error-text matches a row of its record,
+ // so the only unrecorded emissions are the underived ones: a row never
+ // vouches for an error's text.
+ {"error-text", []string{
+ "emit.go:16 (jsonError in formatArg): err puts an error's text in a sentence",
+ "emit.go:20 (jsonError in concatError): err.Error(...) puts an error's text in a sentence",
+ "emit.go:25 (jsonError in wrapVerb): why is the argument of a %w verb",
+ "emit.go:30 (jsonError in implementsError): r puts an error's text in a sentence",
+ "emit.go:35 (jsonError in unknownErr): lastErr puts an error's text in a sentence",
+ "emit.go:40 (jsonError in errorField): res.Err puts an error's text in a sentence",
+ "emit.go:46 (jsonError in localText): detail puts an error's text in a sentence",
+ "emit.go:51 (jsonError in viaHelper): helper explain, at testdata/moves-copy/sinks/error-text/emit.go:55:2: m puts an error's text in a sentence",
+ }, 8},
+ {"field-limit", []string{
+ "emit.go:26 (jsonError in placeholder): a write to field Reason at testdata/moves-copy/sinks/field-limit/emit.go:22:22 is not a sentence: analysis limit reached at ",
+ }, 2},
+ // Both reads are of refusal.Reason, so each sees both writes.
+ {"field-multi", []string{
+ "emit.go:16 (jsonError in multi): a write to field Reason at testdata/moves-copy/sinks/field-multi/emit.go:15:2 takes one result of a multi-value expression",
+ "emit.go:16 (jsonError in multi): a write to field Reason at testdata/moves-copy/sinks/field-multi/emit.go:22:2 takes one result of a multi-value expression",
+ "emit.go:24 (jsonError in commaOK): a write to field Reason at testdata/moves-copy/sinks/field-multi/emit.go:15:2 takes one result of a multi-value expression",
+ "emit.go:24 (jsonError in commaOK): a write to field Reason at testdata/moves-copy/sinks/field-multi/emit.go:22:2 takes one result of a multi-value expression",
+ }, 2},
+ {"shadow-receiver", []string{
+ "emit.go:15 (jsonError in viaShadow): m.Message(...) at testdata/moves-copy/sinks/shadow-receiver/emit.go:15:21: the receiver m is declared more than once in viaShadow",
+ }, 1},
{"ok", nil, 0},
}
for _, c := range cases {
@@ -1206,6 +1236,32 @@ func TestMovesFunctionValueFixture(t *testing.T) {
}
}
+// A local closure configured as a sink (verifyArchive's fail) is a
+// sentence-flow function too: an alias of it (send := fail) escapes the
+// checks at fail's call sites, so the alias is refused.
+func TestMovesLocalClosureAliasFixture(t *testing.T) {
+ file := filepath.Join("testdata", "moves-copy", "sinks", "closure-alias", "emit.go")
+ movesLocalSinks[file] = map[string]int{"fail": 0}
+ defer delete(movesLocalSinks, file)
+ _, vios, u := fixtureScan(t, "closure-alias")
+ if len(vios) != 0 {
+ t.Errorf("violations = %v, want none: the alias is the function-value check's", vios)
+ }
+ var got []string
+ for _, r := range funcValueRefs(t, u) {
+ got = append(got, r.msg)
+ }
+ if len(got) != 1 || !strings.Contains(got[0], "emit.go:18:10: fail is used as a value") {
+ t.Errorf("refs = %q, want exactly fail at emit.go:18:10", got)
+ }
+ // Without the configuration fail is an ordinary closure, and its own
+ // parameter is unbound at its sink.
+ delete(movesLocalSinks, file)
+ if _, vios, _ := fixtureScan(t, "closure-alias"); len(vios) == 0 {
+ t.Error("an unconfigured closure sink must still be a violation")
+ }
+}
+
// A mapping helper may echo a refusal's text only in a case whose guard tests
// it against literals.
func TestMovesMappingHelperFixture(t *testing.T) {
diff --git a/testdata/moves-copy/sinks/closure-alias/emit.go b/testdata/moves-copy/sinks/closure-alias/emit.go
new file mode 100644
index 000000000..0abd4b1fa
--- /dev/null
+++ b/testdata/moves-copy/sinks/closure-alias/emit.go
@@ -0,0 +1,20 @@
+package fixture
+
+// Not compiled. verifyArchive's shape: fail is a local closure configured as
+// a sink, so every call to it is checked. A call through an alias of it is
+// not, and passes an error's text.
+
+type Check struct{ Reason string }
+
+func verify(err error) Check {
+ var out Check
+ fail := func(reason string) Check {
+ out = Check{Reason: reason}
+ return out
+ }
+ if err == nil {
+ return fail("the archive is not readable")
+ }
+ send := fail
+ return send(err.Error())
+}
diff --git a/testdata/moves-copy/sinks/error-text/emit.go b/testdata/moves-copy/sinks/error-text/emit.go
new file mode 100644
index 000000000..fe18943d2
--- /dev/null
+++ b/testdata/moves-copy/sinks/error-text/emit.go
@@ -0,0 +1,56 @@
+package fixture
+
+import "fmt"
+
+// Not compiled. Each sentence has text of its own, and a row with a
+// <placeholder> would match it, but the placeholder carries an error's text.
+
+type refusal struct{}
+
+func (refusal) Error() string { return "refused" }
+
+type result struct{ Err error }
+
+// MV-N2's format with the error as its list.
+func formatArg(w int, to string, err error) {
+ jsonError(w, 400, fmt.Sprintf("region %s is not one of this app's regions: %v", to, err))
+}
+
+func concatError(w int, err error) {
+ jsonError(w, 400, "the region refused it: "+err.Error())
+}
+
+// A %w argument is an error whatever its declared type.
+func wrapVerb(w int, to string, why string) {
+ jsonError(w, 400, fmt.Sprintf("region %s failed: %w", to, why))
+}
+
+// A value of a type that implements error.
+func implementsError(w int, r refusal) {
+ jsonError(w, 400, fmt.Sprintf("the region refused: %v", r))
+}
+
+// An identifier the gate cannot type, named like an error.
+func unknownErr(w int) {
+ jsonError(w, 400, fmt.Sprintf("the last attempt failed: %v", lastErr))
+}
+
+// A field whose declared type is error.
+func errorField(w int, res result) {
+ jsonError(w, 400, fmt.Sprintf("the copy failed: %s", res.Err))
+}
+
+// A local that holds an error's text.
+func localText(w int, err error) {
+ detail := err.Error()
+ jsonError(w, 400, "the copy failed: "+detail)
+}
+
+// A helper whose string parameter is bound to an error's text.
+func viaHelper(w int, err error) {
+ jsonError(w, 400, explain(fmt.Sprint(err)))
+}
+
+func explain(m string) string {
+ return "the copy failed: " + m
+}
diff --git a/testdata/moves-copy/sinks/error-text/record.md b/testdata/moves-copy/sinks/error-text/record.md
new file mode 100644
index 000000000..6b11370fc
--- /dev/null
+++ b/testdata/moves-copy/sinks/error-text/record.md
@@ -0,0 +1,11 @@
+# Rows that match every format of the error-text fixture. The gate must still
+# refuse each sink: a placeholder never carries an error's text.
+
+| id | file | text | status |
+|---|---|---|---|
+| MV-N1 | emit.go | region <to> is not one of this app's regions: <list> | pending |
+| MV-N2 | emit.go | the region refused it: <detail> | pending |
+| MV-N3 | emit.go | region <to> failed: <why> | pending |
+| MV-N4 | emit.go | the region refused: <why> | pending |
+| MV-N5 | emit.go | the last attempt failed: <why> | pending |
+| MV-N6 | emit.go | the copy failed: <why> | pending |
diff --git a/testdata/moves-copy/sinks/field-limit/emit.go b/testdata/moves-copy/sinks/field-limit/emit.go
new file mode 100644
index 000000000..a20ccad79
--- /dev/null
+++ b/testdata/moves-copy/sinks/field-limit/emit.go
@@ -0,0 +1,27 @@
+package fixture
+
+// Not compiled. A Reason read as a placeholder is forwarded when every write
+// to it is a sentence. Here one write runs past the gate's depth limit; the
+// limit is a violation, never a wildcard.
+
+type note struct{ Reason string }
+
+func deep() note {
+ a1 := "this box has no secrets key"
+ a2 := a1
+ a3 := a2
+ a4 := a3
+ a5 := a4
+ a6 := a5
+ a7 := a6
+ a8 := a7
+ a9 := a8
+ a10 := a9
+ a11 := a10
+ a12 := a11
+ return note{Reason: a12}
+}
+
+func placeholder(w int, n note) {
+ jsonError(w, 400, "the copy stopped: "+n.Reason)
+}
diff --git a/testdata/moves-copy/sinks/field-multi/emit.go b/testdata/moves-copy/sinks/field-multi/emit.go
new file mode 100644
index 000000000..1c84d3f84
--- /dev/null
+++ b/testdata/moves-copy/sinks/field-multi/emit.go
@@ -0,0 +1,25 @@
+package fixture
+
+// Not compiled. The literal writes a recorded sentence, but a later
+// assignment takes one result of a multi-value expression into the same
+// field. The gate cannot prove that write, so the read is a violation.
+
+type refusal struct{ Reason string }
+
+func detail(err error) (string, error) {
+ return err.Error(), nil
+}
+
+func multi(w int, err error) {
+ box := refusal{Reason: "invalid region"}
+ box.Reason, _ = detail(err)
+ jsonError(w, 400, box.Reason)
+}
+
+func commaOK(w int, names map[string]string, k string) {
+ box := refusal{Reason: "invalid region"}
+ var ok bool
+ box.Reason, ok = names[k]
+ _ = ok
+ jsonError(w, 400, box.Reason)
+}
diff --git a/testdata/moves-copy/sinks/ok/emit.go b/testdata/moves-copy/sinks/ok/emit.go
index c1bef0dc2..d18909e2e 100644
--- a/testdata/moves-copy/sinks/ok/emit.go
+++ b/testdata/moves-copy/sinks/ok/emit.go
@@ -76,3 +76,29 @@ func makeCheck() Check {
func viaField(w int, c Check) {
jsonError(w, 400, c.Reason)
}
+
+// An error that only selects among sentences, and is never spoken.
+func selects(w int, err error) {
+ jsonError(w, 400, pick(err))
+}
+
+func pick(err error) string {
+ if err == nil {
+ return "the copy is fine"
+ }
+ return "the copy is not fine"
+}
+
+// A placeholder that is data, not an error.
+func dataPlaceholder(w int, host string) {
+ jsonError(w, 400, fmt.Sprintf("host %s is not armed", host))
+}
+
+// A method on a receiver declared once.
+type local struct{}
+
+func (local) Message() string { return "the app is not enabled" }
+
+func viaMethod(w int, m local) {
+ jsonError(w, 400, m.Message())
+}
diff --git a/testdata/moves-copy/sinks/ok/record.md b/testdata/moves-copy/sinks/ok/record.md
index 07ec468f7..4edbaead7 100644
--- a/testdata/moves-copy/sinks/ok/record.md
+++ b/testdata/moves-copy/sinks/ok/record.md
@@ -13,3 +13,6 @@
| MV-N9 | emit.go | the app is not enabled | pending |
| MV-N10 | emit.go | <n> apps are not enabled | pending |
| MV-N11 | emit.go | first | pending |
+| MV-N12 | emit.go | the copy is fine | pending |
+| MV-N13 | emit.go | the copy is not fine | pending |
+| MV-N14 | emit.go | host <host> is not armed | pending |
diff --git a/testdata/moves-copy/sinks/shadow-receiver/emit.go b/testdata/moves-copy/sinks/shadow-receiver/emit.go
new file mode 100644
index 000000000..85be3bd8b
--- /dev/null
+++ b/testdata/moves-copy/sinks/shadow-receiver/emit.go
@@ -0,0 +1,21 @@
+package fixture
+
+// Not compiled. The inner m shadows the parameter m, whose type has a method
+// that returns a recorded sentence. The inner m's type is another package's,
+// and its Message may return anything.
+
+type local struct{}
+
+func (local) Message() string {
+ return "a recorded sentence"
+}
+
+func viaShadow(w int, m local, supplied external) {
+ if m := supplied; m != nil {
+ jsonError(w, 400, m.Message())
+ }
+}
+
+func direct(w int, m local) {
+ jsonError(w, 400, m.Message())
+}
diff --git a/testdata/moves-copy/sinks/shadow-receiver/record.md b/testdata/moves-copy/sinks/shadow-receiver/record.md
new file mode 100644
index 000000000..12b4144bb
--- /dev/null
+++ b/testdata/moves-copy/sinks/shadow-receiver/record.md
@@ -0,0 +1,3 @@
+| id | file | text | status |
+|---|---|---|---|
+| MV-N1 | emit.go | a recorded sentence | pending |