amadan

carlos / platform Public

copy gate fix round 2: a local is a sentence only if every assignment is, helpers and parameters are checked to their returns and call sites, mapping helpers, notes out of the row text

Negative fixtures: one of two assignments an error's text, a helper with one bad
return, a parameter with one bad call site, a mapping helper with an unguarded
echo. MV-N130 and MV-N131 keep their sentence only; their notes move into the
File cell after the path. Both rows stay pending.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
claude 0e26010fe6ce623296a1c2e1c678ecf929669110 parent 8358c0a
7 files changed, +651 −188
  • approved_moves_copy_test.go +533 −172
  • docs/superpowers/plans/2026-10-06-moves-phase1-copy.md +2 −2
  • testdata/moves-copy/sinks/helper-bad-return/emit.go +15 −0
  • testdata/moves-copy/sinks/local-variable/emit.go +18 −14
  • testdata/moves-copy/sinks/mapping/emit.go +36 −0
  • testdata/moves-copy/sinks/ok/emit.go +35 −0
  • testdata/moves-copy/sinks/param/emit.go +12 −0
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go
index 71275867a..cbd54f492 100644
--- a/approved_moves_copy_test.go
+++ b/approved_moves_copy_test.go
@@ -67,11 +67,21 @@ var movesCopyIDs = []string{
"MV-N151", "MV-N152",
}
-// copyRow is one row of the record.
+// copyRow is one row of the record. The File cell is the file the sentence is
+// emitted from, and may carry a note after the path in parentheses ("path (a
+// T3 note, not a failure)"); the Text cell is the sentence and nothing else.
type copyRow struct {
ID, File, Text, Status string
}
+// Path is the File cell without its note.
+func (r copyRow) Path() string {
+ if i := strings.Index(r.File, " ("); i > 0 && strings.HasSuffix(r.File, ")") {
+ return r.File[:i]
+ }
+ return r.File
+}
+
var movesCopyIDPattern = regexp.MustCompile(`^(M-C|MV-N)[0-9]+$`)
// parseMovesCopyRecord reads the record's tables. A data row is a line that
@@ -190,13 +200,13 @@ func checkMovesApplied(rows []copyRow, root string) []string {
if r.Status != "approved" {
continue
}
- body, err := os.ReadFile(filepath.Join(root, r.File))
+ body, err := os.ReadFile(filepath.Join(root, r.Path()))
if err != nil {
out = append(out, fmt.Sprintf("%s: %v", r.ID, err))
continue
}
- if !movesTextApplied(r.File, r.Text, body) {
- out = append(out, fmt.Sprintf("%s: %s does not contain the approved text %q", r.ID, r.File, r.Text))
+ if !movesTextApplied(r.Path(), r.Text, body) {
+ out = append(out, fmt.Sprintf("%s: %s does not contain the approved text %q", r.ID, r.Path(), r.Text))
}
}
return out
@@ -388,6 +398,19 @@ func TestParseMovesCopyTable(t *testing.T) {
}
}
+// A note after the path stays out of the sentence and out of the file name.
+func TestCopyRowPath(t *testing.T) {
+ rows, err := parseMovesCopyTable("| id | file | text | status |\n|---|---|---|---|\n"+
+ "| MV-N1 | a/b.go (a T3 note, not a failure) | <NAME> (<kind>) | pending |\n"+
+ "| MV-N2 | a/b.go | a sentence | pending |\n", "x.md")
+ if err != nil {
+ t.Fatal(err)
+ }
+ if rows[0].Path() != "a/b.go" || rows[1].Path() != "a/b.go" || rows[0].Text != "<NAME> (<kind>)" {
+ t.Errorf("rows = %+v", rows)
+ }
+}
+
func TestParseMovesCopyRecordMissingFile(t *testing.T) {
if _, err := parseMovesCopyRecord("testdata/moves-copy/no-such-record.md"); err == nil {
t.Error("a record missing from the tree must fail to parse")
@@ -657,7 +680,11 @@ func formatPattern(f string) string {
type movesPkgIndex struct {
vars map[string]ast.Expr
funcs map[string]*ast.FuncDecl
+ methods map[string][]*ast.FuncDecl
+ decls []*ast.FuncDecl
noCalls bool // read a call to a package function as a wildcard
+ // locals are the assignments to the variables of the function being read.
+ locals map[string][]localAssign
}
var movesPkgCache = map[string]*movesPkgIndex{}
@@ -666,12 +693,38 @@ var movesPkgCache = map[string]*movesPkgIndex{}
// helper in a sibling file can be named file:line.
var movesFset = token.NewFileSet()
+func newPkgIndex() *movesPkgIndex {
+ return &movesPkgIndex{vars: map[string]ast.Expr{}, funcs: map[string]*ast.FuncDecl{}, methods: map[string][]*ast.FuncDecl{}}
+}
+
+func (p *movesPkgIndex) addFile(f *ast.File) {
+ for _, d := range f.Decls {
+ switch d := d.(type) {
+ case *ast.FuncDecl:
+ p.decls = append(p.decls, d)
+ if d.Recv == nil {
+ p.funcs[d.Name.Name] = d
+ } else {
+ p.methods[d.Name.Name] = append(p.methods[d.Name.Name], d)
+ }
+ case *ast.GenDecl:
+ for _, s := range d.Specs {
+ if vs, ok := s.(*ast.ValueSpec); ok && len(vs.Names) == len(vs.Values) {
+ for i, nm := range vs.Names {
+ p.vars[nm.Name] = vs.Values[i]
+ }
+ }
+ }
+ }
+ }
+}
+
func loadMovesPkg(t *testing.T, dir string) *movesPkgIndex {
t.Helper()
if p, ok := movesPkgCache[dir]; ok {
return p
}
- p := &movesPkgIndex{vars: map[string]ast.Expr{}, funcs: map[string]*ast.FuncDecl{}}
+ p := newPkgIndex()
movesPkgCache[dir] = p
ents, err := os.ReadDir(dir)
if err != nil {
@@ -686,31 +739,97 @@ func loadMovesPkg(t *testing.T, dir string) *movesPkgIndex {
if err != nil {
t.Fatal(err)
}
- for _, d := range f.Decls {
- switch d := d.(type) {
- case *ast.FuncDecl:
- if d.Recv == nil {
- p.funcs[d.Name.Name] = d
+ p.addFile(f)
+ }
+ return p
+}
+
+// localAssign is one assignment to a local variable: either an expression
+// (rhs), or position pos of a multi-value call, or a zero value (a declaration
+// with no value), or something the gate cannot read (a range variable).
+type localAssign struct {
+ rhs ast.Expr
+ call *ast.CallExpr
+ pos int
+ zero bool
+ unreadable bool
+}
+
+// localAssigns lists every assignment to every variable of fd, by name.
+func localAssigns(fd *ast.FuncDecl) map[string][]localAssign {
+ out := map[string][]localAssign{}
+ if fd == nil || fd.Body == nil {
+ return out
+ }
+ ast.Inspect(fd.Body, func(n ast.Node) bool {
+ switch n := n.(type) {
+ case *ast.AssignStmt:
+ for i, l := range n.Lhs {
+ id, ok := l.(*ast.Ident)
+ if !ok || id.Name == "_" {
+ continue
}
- case *ast.GenDecl:
- for _, s := range d.Specs {
- if vs, ok := s.(*ast.ValueSpec); ok && len(vs.Names) == len(vs.Values) {
- for i, nm := range vs.Names {
- p.vars[nm.Name] = vs.Values[i]
- }
+ switch {
+ case len(n.Rhs) == len(n.Lhs):
+ out[id.Name] = append(out[id.Name], localAssign{rhs: n.Rhs[i]})
+ case len(n.Rhs) == 1:
+ if c, ok := n.Rhs[0].(*ast.CallExpr); ok {
+ out[id.Name] = append(out[id.Name], localAssign{call: c, pos: i})
+ } else {
+ out[id.Name] = append(out[id.Name], localAssign{unreadable: true})
}
}
}
+ case *ast.ValueSpec:
+ for i, id := range n.Names {
+ switch {
+ case len(n.Values) == len(n.Names):
+ out[id.Name] = append(out[id.Name], localAssign{rhs: n.Values[i]})
+ case len(n.Values) == 0:
+ out[id.Name] = append(out[id.Name], localAssign{zero: true})
+ case len(n.Values) == 1:
+ if c, ok := n.Values[0].(*ast.CallExpr); ok {
+ out[id.Name] = append(out[id.Name], localAssign{call: c, pos: i})
+ }
+ }
+ }
+ case *ast.RangeStmt:
+ for _, x := range []ast.Expr{n.Key, n.Value} {
+ if id, ok := x.(*ast.Ident); ok && id.Name != "_" {
+ out[id.Name] = append(out[id.Name], localAssign{unreadable: true})
+ }
+ }
}
+ return true
+ })
+ return out
+}
+
+// calleeDecls finds the package function or methods a call may reach. A
+// selector is matched by method name only; the gate has no types.
+func calleeDecls(e *ast.CallExpr, pkg *movesPkgIndex) []*ast.FuncDecl {
+ switch fn := e.Fun.(type) {
+ case *ast.Ident:
+ if d, ok := pkg.funcs[fn.Name]; ok {
+ return []*ast.FuncDecl{d}
+ }
+ case *ast.SelectorExpr:
+ if fn.Sel.Name == "Error" {
+ return nil
+ }
+ return pkg.methods[fn.Sel.Name]
}
- return p
+ return nil
}
// patternsOf returns the sentence patterns an expression can produce:
-// literals, concatenations, fmt.Sprintf formats, package constants and
-// package functions that return strings. Anything it cannot read is a
-// wildcard.
+// literals, concatenations, fmt.Sprintf formats, package constants, local
+// variables (through every assignment), and package functions and methods
+// that return strings. Anything it cannot read is a wildcard.
func patternsOf(e ast.Expr, pkg *movesPkgIndex, depth int) []string {
+ if depth > 4 {
+ return []string{wild}
+ }
switch e := e.(type) {
case *ast.BasicLit:
if e.Kind == token.STRING {
@@ -721,7 +840,23 @@ func patternsOf(e ast.Expr, pkg *movesPkgIndex, depth int) []string {
case *ast.ParenExpr:
return patternsOf(e.X, pkg, depth)
case *ast.Ident:
- if x, ok := pkg.vars[e.Name]; ok && depth < 4 {
+ if as, ok := pkg.locals[e.Name]; ok {
+ var out []string
+ for _, a := range as {
+ switch {
+ case a.zero:
+ out = append(out, "")
+ case a.rhs != nil:
+ out = append(out, patternsOf(a.rhs, pkg, depth+1)...)
+ case a.call != nil:
+ out = append(out, callPatterns(a.call, a.pos, pkg, depth+1)...)
+ default:
+ out = append(out, wild)
+ }
+ }
+ return out
+ }
+ if x, ok := pkg.vars[e.Name]; ok {
return patternsOf(x, pkg, depth+1)
}
case *ast.BinaryExpr:
@@ -736,36 +871,70 @@ func patternsOf(e ast.Expr, pkg *movesPkgIndex, depth int) []string {
}
return out
case *ast.CallExpr:
- switch fn := e.Fun.(type) {
- case *ast.SelectorExpr:
- if id, ok := fn.X.(*ast.Ident); ok && len(e.Args) > 0 &&
- ((id.Name == "fmt" && (fn.Sel.Name == "Sprintf" || fn.Sel.Name == "Errorf")) ||
- (id.Name == "errors" && fn.Sel.Name == "New")) {
- var out []string
- for _, f := range patternsOf(e.Args[0], pkg, depth) {
- out = append(out, formatPattern(f))
- }
- return out
+ if isFormatCall(e) {
+ var out []string
+ for _, f := range patternsOf(e.Args[0], pkg, depth) {
+ out = append(out, formatPattern(f))
}
- case *ast.Ident:
- if d, ok := pkg.funcs[fn.Name]; ok && !pkg.noCalls && depth < 4 && returnsOneString(d) {
- var out []string
- ast.Inspect(d.Body, func(n ast.Node) bool {
- switch n := n.(type) {
- case *ast.FuncLit:
- return false
- case *ast.ReturnStmt:
- if len(n.Results) == 1 {
- out = append(out, patternsOf(n.Results[0], pkg, depth+1)...)
- }
+ return out
+ }
+ return callPatterns(e, 0, pkg, depth)
+ }
+ return []string{wild}
+}
+
+// callPatterns reads the sentences result pos of a call to a package function
+// or method can be: the patterns of its returns, read in the callee's own
+// locals.
+func callPatterns(e *ast.CallExpr, pos int, pkg *movesPkgIndex, depth int) []string {
+ if pkg.noCalls || depth > 4 {
+ return []string{wild}
+ }
+ decls := calleeDecls(e, pkg)
+ if len(decls) == 0 {
+ return []string{wild}
+ }
+ var out []string
+ for _, d := range decls {
+ if d.Body == nil || !stringResults(d)[pos] {
+ out = append(out, wild)
+ continue
+ }
+ hp := *pkg
+ hp.locals = localAssigns(d)
+ nres := resultCount(d)
+ ast.Inspect(d.Body, func(n ast.Node) bool {
+ switch n := n.(type) {
+ case *ast.FuncLit:
+ return false
+ case *ast.ReturnStmt:
+ switch {
+ case len(n.Results) == nres && pos < len(n.Results):
+ out = append(out, patternsOf(n.Results[pos], &hp, depth+1)...)
+ case len(n.Results) == 1 && nres > 1:
+ if c, ok := n.Results[0].(*ast.CallExpr); ok {
+ out = append(out, callPatterns(c, pos, &hp, depth+1)...)
}
- return true
- })
- return out
+ }
}
+ return true
+ })
+ }
+ return out
+}
+
+func resultCount(d *ast.FuncDecl) int {
+ if d.Type.Results == nil {
+ return 0
+ }
+ n := 0
+ for _, f := range d.Type.Results.List {
+ if len(f.Names) == 0 {
+ n++
}
+ n += len(f.Names)
}
- return []string{wild}
+ return n
}
func returnsOneString(d *ast.FuncDecl) bool {
@@ -856,8 +1025,9 @@ func scanMovesFileFull(t *testing.T, root, file string, noCalls bool) ([]movesEm
seenVio := map[string]bool{}
var fn string
var ctx *sinkCtx
+ cur := pkg
emit := func(pos token.Pos, kind string, e ast.Expr, fragment bool) {
- for _, p := range patternsOf(e, pkg, 0) {
+ for _, p := range patternsOf(e, cur, 0) {
if informative(p) {
out = append(out, movesEmission{file, fset.Position(pos).Line, fset.Position(e.End()).Line, fn, kind, p, fragment})
}
@@ -896,7 +1066,10 @@ func scanMovesFileFull(t *testing.T, root, file string, noCalls bool) ([]movesEm
continue
}
fn = fd.Name.Name
- ctx = newSinkCtx(file, fd, pkg)
+ fp := *pkg
+ fp.locals = localAssigns(fd)
+ cur = &fp
+ ctx = newSinkCtx(file, fd, cur)
inTable := tables[fn]
strs := stringResults(fd)
returnsSentence := movesReturnedSentences[file][fn] && len(strs) > 0
@@ -974,6 +1147,7 @@ func scanMovesFileFull(t *testing.T, root, file string, noCalls bool) ([]movesEm
}
}
case *ast.ReturnStmt:
+ ctx.curReturn = n
switch {
case inTable:
for _, r := range n.Results {
@@ -1015,13 +1189,25 @@ var movesFuncSinks = map[string]map[string]int{
// code instead. TestMovesSinkAllowListIsHonest holds the list to this.
var movesSinkAllow = map[string]map[string]string{}
+// movesMappingHelpers are functions that map a refusal from another package
+// onto the sentence an operator sees, echoing the refusal's own text only in a
+// case whose guard tests that text against recorded literals (equality,
+// HasPrefix, HasSuffix or Contains with a literal). A `return x` in such a case
+// is taken; any other return is checked as usual. Anything else answers a fixed
+// sentence.
+var movesMappingHelpers = map[string]map[string]bool{
+ "internal/console/apiinstancemove.go": {"phase1Conflict": true},
+}
+
// sinkCtx classifies what a sink argument is, inside one function.
type sinkCtx struct {
- file string
- fd *ast.FuncDecl
- pkg *movesPkgIndex
- fwd map[string]bool // string parameters whose values are sinks at the call sites
- fwdFn map[string]bool // function parameters that are sentence helpers at the call sites
+ file string
+ fd *ast.FuncDecl
+ pkg *movesPkgIndex
+ fwd map[string]bool // string parameters whose values are sinks at the call sites
+ fwdFn map[string]bool // function parameters that are sentence helpers at the call sites
+ curReturn *ast.ReturnStmt // the return being checked, for a mapping helper
+ visiting map[string]bool // helpers and parameters being read, so a cycle ends
}
func paramNames(ft *ast.FuncType) []string {
@@ -1041,7 +1227,7 @@ func paramNames(ft *ast.FuncType) []string {
}
func newSinkCtx(file string, fd *ast.FuncDecl, pkg *movesPkgIndex) *sinkCtx {
- c := &sinkCtx{file: file, fd: fd, pkg: pkg, fwd: map[string]bool{}, fwdFn: map[string]bool{}}
+ c := &sinkCtx{file: file, fd: fd, pkg: pkg, fwd: map[string]bool{}, fwdFn: map[string]bool{}, visiting: map[string]bool{}}
if fd == nil {
return c
}
@@ -1076,32 +1262,21 @@ func newSinkCtx(file string, fd *ast.FuncDecl, pkg *movesPkgIndex) *sinkCtx {
return c
}
-// isConstLike reports whether an identifier names a package-level string.
-func (c *sinkCtx) pkgString(name string) (ast.Expr, bool) {
- x, ok := c.pkg.vars[name]
- return x, ok
+// sub is a context for another function of the package, sharing the cycle guard.
+func (c *sinkCtx) sub(d *ast.FuncDecl) *sinkCtx {
+ hp := *c.pkg
+ hp.locals = localAssigns(d)
+ n := newSinkCtx(c.file, d, &hp)
+ n.visiting = c.visiting
+ return n
}
-// literalish reports whether e is, on its face, a piece of fixed text: a
-// string literal, a package const, a Sprintf or errors.New with a literal
-// format, or a call to a recognised sentence helper.
-func (c *sinkCtx) literalish(e ast.Expr, depth int) bool {
- switch e := e.(type) {
- case *ast.ParenExpr:
- return c.literalish(e.X, depth)
- case *ast.BasicLit:
- return e.Kind == token.STRING
- case *ast.Ident:
- _, ok := c.pkgString(e.Name)
- return ok
- case *ast.CallExpr:
- return isFormatCall(e) && c.formatIsLiteral(e)
- case *ast.BinaryExpr:
- if e.Op == token.ADD {
- return c.literalish(e.X, depth) || c.literalish(e.Y, depth)
- }
+func (c *sinkCtx) pkgString(name string) (ast.Expr, bool) {
+ if _, shadowed := c.pkg.locals[name]; shadowed {
+ return nil, false
}
- return false
+ x, ok := c.pkg.vars[name]
+ return x, ok
}
func isFormatCall(e *ast.CallExpr) bool {
@@ -1125,61 +1300,26 @@ func (c *sinkCtx) formatIsLiteral(e *ast.CallExpr) bool {
return false
}
-// localSentence reports whether a function assigns a string literal (or a
-// literal concatenation or Sprintf) to the local variable name.
-func (c *sinkCtx) localSentence(name string) bool {
- if c.fd == nil {
- return false
- }
- found := false
- ast.Inspect(c.fd.Body, func(n ast.Node) bool {
- switch n := n.(type) {
- case *ast.AssignStmt:
- for i, l := range n.Lhs {
- if id, ok := l.(*ast.Ident); ok && id.Name == name && i < len(n.Rhs) && isSentenceLiteral(n.Rhs[i]) {
- found = true
- }
- }
- case *ast.ValueSpec:
- for i, id := range n.Names {
- if id.Name == name && i < len(n.Values) && isSentenceLiteral(n.Values[i]) {
- found = true
- }
- }
- }
- return !found
- })
- return found
-}
-
-// isSentenceLiteral is true for text written in place: a string literal, a
-// Sprintf or errors.New, or a concatenation with a string literal in it. A
-// reference to a const is not text written in place.
-func isSentenceLiteral(e ast.Expr) bool {
+// literalish reports whether e is, on its face, a piece of fixed text: a
+// string literal, a package const, a Sprintf or errors.New with a literal
+// format, or a concatenation with such a part.
+func (c *sinkCtx) literalish(e ast.Expr) bool {
switch e := e.(type) {
case *ast.ParenExpr:
- return isSentenceLiteral(e.X)
+ return c.literalish(e.X)
case *ast.BasicLit:
return e.Kind == token.STRING
+ case *ast.Ident:
+ _, ok := c.pkgString(e.Name)
+ return ok
case *ast.CallExpr:
- return isFormatCall(e)
+ return isFormatCall(e) && c.formatIsLiteral(e)
case *ast.BinaryExpr:
- return e.Op == token.ADD && (isSentenceLiteral(e.X) || isSentenceLiteral(e.Y))
+ return e.Op == token.ADD && (c.literalish(e.X) || c.literalish(e.Y))
}
return false
}
-// operandProblem is the one thing a placeholder operand may not be: a local
-// variable that was given a sentence.
-func (c *sinkCtx) operandProblem(e ast.Expr) string {
- if id, ok := e.(*ast.Ident); ok {
- if _, isPkg := c.pkgString(id.Name); !isPkg && c.localSentence(id.Name) {
- return fmt.Sprintf("%s holds a sentence assigned before the sink, so the sentence is never read where it is spoken", id.Name)
- }
- }
- return ""
-}
-
func addOperands(e ast.Expr, out *[]ast.Expr) {
if b, ok := e.(*ast.BinaryExpr); ok && b.Op == token.ADD {
addOperands(b.X, out)
@@ -1218,12 +1358,17 @@ func render(e ast.Expr) string {
//
// Taken: a string literal; a package const; a concatenation with a literal
// part or a Sprintf with a literal format (the other operands are the
-// placeholders the record names); a call to a package function that returns
-// a string and whose own returns are taken; a parameter that is itself a
-// sink at its call sites; a Check's own Reason (every Check is made at a
-// sink). Everything else is refused: a variable, err.Error(), a method's
-// result, a field, an expression the gate cannot read.
+// placeholders the record names); a call to a package function or method
+// whose result is a string and whose every return is itself taken; a local
+// variable whose every assignment is taken; a parameter that is a sink at its
+// call sites, or whose every call site passes a taken argument; a Check's own
+// Reason (every Check is made at a sink); a mapping helper's guarded echo.
+// Everything else is refused: err.Error(), a method's result the gate cannot
+// read, a field, an index, an expression it cannot read.
func (c *sinkCtx) check(e ast.Expr, depth int) string {
+ if depth > 6 {
+ return ""
+ }
switch e := e.(type) {
case *ast.ParenExpr:
return c.check(e.X, depth)
@@ -1233,16 +1378,7 @@ func (c *sinkCtx) check(e ast.Expr, depth int) string {
}
return "not a string literal"
case *ast.Ident:
- if c.fwd[e.Name] {
- return ""
- }
- if x, ok := c.pkgString(e.Name); ok {
- if depth >= 4 {
- return ""
- }
- return (&sinkCtx{pkg: c.pkg, fwd: map[string]bool{}, fwdFn: map[string]bool{}}).check(x, depth+1)
- }
- return fmt.Sprintf("%s is a variable, not a literal, a const or a sentence helper", e.Name)
+ return c.identProblem(e, depth)
case *ast.SelectorExpr:
if e.Sel.Name == "Reason" {
return ""
@@ -1256,13 +1392,11 @@ func (c *sinkCtx) check(e ast.Expr, depth int) string {
addOperands(e, &ops)
lit := false
for _, o := range ops {
- if c.literalish(o, depth) {
+ if c.literalish(o) {
lit = true
- if why := c.check(o, depth); why != "" {
+ if why := c.check(o, depth+1); why != "" {
return why
}
- } else if why := c.operandProblem(o); why != "" {
- return why
}
}
if !lit {
@@ -1274,48 +1408,166 @@ func (c *sinkCtx) check(e ast.Expr, depth int) string {
if !c.formatIsLiteral(e) {
return fmt.Sprintf("%s has a format that is not a literal", render(e))
}
- for _, a := range e.Args[1:] {
- if why := c.operandProblem(a); why != "" {
- return why
+ return ""
+ }
+ if id, ok := e.Fun.(*ast.Ident); ok && c.fwdFn[id.Name] {
+ return ""
+ }
+ return c.callProblem(e, 0, depth)
+ }
+ return fmt.Sprintf("%s is not a sentence the gate can read", render(e))
+}
+
+func (c *sinkCtx) identProblem(id *ast.Ident, depth int) string {
+ if c.fwd[id.Name] {
+ return ""
+ }
+ if c.guardedEcho(id) {
+ return ""
+ }
+ if as, ok := c.pkg.locals[id.Name]; ok {
+ for _, a := range as {
+ switch {
+ case a.zero:
+ case a.rhs != nil:
+ if why := c.check(a.rhs, depth+1); why != "" {
+ return fmt.Sprintf("%s is assigned something that is not a sentence: %s", id.Name, why)
+ }
+ case a.call != nil:
+ if why := c.callProblem(a.call, a.pos, depth+1); why != "" {
+ return fmt.Sprintf("%s is assigned something that is not a sentence: %s", id.Name, why)
}
+ default:
+ return fmt.Sprintf("%s is assigned a value the gate cannot read", id.Name)
}
- return ""
}
- if id, ok := e.Fun.(*ast.Ident); ok {
- if c.fwdFn[id.Name] {
- return ""
+ return ""
+ }
+ if x, ok := c.pkgString(id.Name); ok {
+ return (&sinkCtx{pkg: c.pkg, fwd: map[string]bool{}, fwdFn: map[string]bool{}, visiting: c.visiting}).check(x, depth+1)
+ }
+ if c.fd != nil {
+ for i, n := range paramNames(c.fd.Type) {
+ if n == id.Name {
+ return c.paramProblem(i, id.Name, depth)
}
- return c.helperProblem(id.Name, depth)
}
- return fmt.Sprintf("%s forwards a call's result", render(e))
}
- return fmt.Sprintf("%s is not a sentence the gate can read", render(e))
+ return fmt.Sprintf("%s is a variable, not a literal, a const or a sentence helper", id.Name)
+}
+
+// paramProblem checks a parameter used as a sentence: every call site in the
+// package must pass an argument that is a sentence.
+func (c *sinkCtx) paramProblem(idx int, name string, depth int) string {
+ key := fmt.Sprintf("param %p %d", c.fd, idx)
+ if c.visiting[key] {
+ return ""
+ }
+ c.visiting[key] = true
+ defer delete(c.visiting, key)
+ sites := 0
+ for _, d := range c.pkg.decls {
+ if d.Body == nil {
+ continue
+ }
+ var caller *sinkCtx
+ var why string
+ ast.Inspect(d.Body, func(n ast.Node) bool {
+ call, ok := n.(*ast.CallExpr)
+ if !ok || why != "" {
+ return why == ""
+ }
+ named := ""
+ switch f := call.Fun.(type) {
+ case *ast.Ident:
+ named = f.Name
+ case *ast.SelectorExpr:
+ named = f.Sel.Name
+ }
+ if named != c.fd.Name.Name || idx >= len(call.Args) {
+ return true
+ }
+ sites++
+ if caller == nil {
+ caller = c.sub(d)
+ }
+ if w := caller.check(call.Args[idx], depth+1); w != "" {
+ why = fmt.Sprintf("parameter %s is passed a value that is not a sentence at %s: %s", name, movesFset.Position(call.Pos()), w)
+ }
+ return true
+ })
+ if why != "" {
+ return why
+ }
+ }
+ if sites == 0 {
+ return fmt.Sprintf("parameter %s has no call site in the package, so its value is unknown", name)
+ }
+ return ""
}
-// helperProblem checks a package function used as a sentence helper: it must
-// return one string, and every return must itself be taken.
-func (c *sinkCtx) helperProblem(name string, depth int) string {
- d, ok := c.pkg.funcs[name]
- if !ok || d.Body == nil {
- return fmt.Sprintf("%s(...) is not a sentence helper of this package", name)
+// callProblem checks result pos of a call: the callee must be a package
+// function or method with a string there, and every return of it must be a
+// sentence.
+func (c *sinkCtx) callProblem(e *ast.CallExpr, pos, depth int) string {
+ if sel, ok := e.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "Error" {
+ return fmt.Sprintf("%s forwards an error's text", render(e))
+ }
+ decls := calleeDecls(e, c.pkg)
+ if len(decls) == 0 {
+ return fmt.Sprintf("%s forwards a call's result the gate cannot read", render(e))
}
- if !returnsOneString(d) {
- return fmt.Sprintf("%s(...) does not return a sentence", name)
+ for _, d := range decls {
+ if why := c.helperProblem(d, pos, depth); why != "" {
+ return why
+ }
+ }
+ return ""
+}
+
+// helperProblem checks every return of d at result position pos.
+func (c *sinkCtx) helperProblem(d *ast.FuncDecl, pos, depth int) string {
+ if d.Body == nil || !stringResults(d)[pos] {
+ return fmt.Sprintf("%s(...) does not return a sentence at result %d", d.Name.Name, pos)
}
- if depth >= 4 {
+ key := fmt.Sprintf("helper %p %d", d, pos)
+ if c.visiting[key] {
return ""
}
- hc := &sinkCtx{fd: d, pkg: c.pkg, fwd: map[string]bool{}, fwdFn: map[string]bool{}}
+ c.visiting[key] = true
+ defer delete(c.visiting, key)
+ hc := c.sub(d)
+ nres := resultCount(d)
+ var named []string
+ for _, f := range d.Type.Results.List {
+ for _, n := range f.Names {
+ named = append(named, n.Name)
+ }
+ }
var why string
ast.Inspect(d.Body, func(n ast.Node) bool {
switch n := n.(type) {
case *ast.FuncLit:
return false
case *ast.ReturnStmt:
- if why == "" && len(n.Results) == 1 {
- if w := hc.check(n.Results[0], depth+1); w != "" {
- why = fmt.Sprintf("helper %s, at %s: %s", name, movesFset.Position(n.Pos()), w)
+ if why != "" {
+ return false
+ }
+ hc.curReturn = n
+ var w string
+ switch {
+ case len(n.Results) == nres:
+ w = hc.check(n.Results[pos], depth+1)
+ case len(n.Results) == 1 && nres > 1:
+ if call, ok := n.Results[0].(*ast.CallExpr); ok {
+ w = hc.callProblem(call, pos, depth+1)
}
+ case len(n.Results) == 0 && pos < len(named):
+ w = hc.check(&ast.Ident{Name: named[pos]}, depth+1)
+ }
+ hc.curReturn = nil
+ if w != "" {
+ why = fmt.Sprintf("helper %s, at %s: %s", d.Name.Name, movesFset.Position(n.Pos()), w)
}
}
return why == ""
@@ -1323,12 +1575,95 @@ func (c *sinkCtx) helperProblem(name string, depth int) string {
return why
}
+// guardedEcho reports whether id is the whole of the return being checked,
+// inside a case of a mapping helper whose guard tests id against literals.
+func (c *sinkCtx) guardedEcho(id *ast.Ident) bool {
+ if c.curReturn == nil || c.fd == nil || !movesMappingHelpers[c.file][c.fd.Name.Name] {
+ return false
+ }
+ direct := false
+ for _, r := range c.curReturn.Results {
+ if r == ast.Expr(id) {
+ direct = true
+ }
+ }
+ if !direct {
+ return false
+ }
+ var cc *ast.CaseClause
+ var stack []ast.Node
+ ast.Inspect(c.fd.Body, func(n ast.Node) bool {
+ if n == nil {
+ stack = stack[:len(stack)-1]
+ return true
+ }
+ stack = append(stack, n)
+ if n == ast.Node(c.curReturn) {
+ for i := len(stack) - 1; i >= 0; i-- {
+ if k, ok := stack[i].(*ast.CaseClause); ok {
+ cc = k
+ break
+ }
+ }
+ }
+ return cc == nil
+ })
+ if cc == nil || len(cc.List) == 0 {
+ return false
+ }
+ for _, g := range cc.List {
+ if !guardsLiteral(g, id.Name, c) {
+ return false
+ }
+ }
+ return true
+}
+
+// guardsLiteral reports whether g tests name against literals only: name ==
+// literal, or HasPrefix, HasSuffix or Contains of name and a literal, joined
+// by &&.
+func guardsLiteral(g ast.Expr, name string, c *sinkCtx) bool {
+ switch g := g.(type) {
+ case *ast.ParenExpr:
+ return guardsLiteral(g.X, name, c)
+ case *ast.BinaryExpr:
+ switch g.Op {
+ case token.LAND:
+ return guardsLiteral(g.X, name, c) && guardsLiteral(g.Y, name, c)
+ case token.EQL:
+ l, lok := g.X.(*ast.Ident)
+ r, rok := g.Y.(*ast.Ident)
+ switch {
+ case lok && l.Name == name:
+ return c.literalish(g.Y) && !(rok && r.Name == name)
+ case rok && r.Name == name:
+ return c.literalish(g.X)
+ }
+ }
+ case *ast.CallExpr:
+ sel, ok := g.Fun.(*ast.SelectorExpr)
+ if !ok || len(g.Args) != 2 {
+ return false
+ }
+ x, isID := sel.X.(*ast.Ident)
+ a0, a0ok := g.Args[0].(*ast.Ident)
+ if !isID || x.Name != "strings" || !a0ok || a0.Name != name {
+ return false
+ }
+ switch sel.Sel.Name {
+ case "HasPrefix", "HasSuffix", "Contains":
+ return c.literalish(g.Args[1])
+ }
+ }
+ return false
+}
+
// checkFuncArg checks the function a call hands to a sentence-function
// parameter: a package function that is a sentence helper.
func (c *sinkCtx) checkFuncArg(e ast.Expr) string {
if id, ok := e.(*ast.Ident); ok {
- if _, isFn := c.pkg.funcs[id.Name]; isFn {
- return c.helperProblem(id.Name, 0)
+ if d, isFn := c.pkg.funcs[id.Name]; isFn {
+ return c.helperProblem(d, 0, 0)
}
}
return fmt.Sprintf("%s is passed where a sentence helper is expected", render(e))
@@ -1749,16 +2084,23 @@ func wantViolations(t *testing.T, name string, got []movesViolation, wants ...st
func TestMovesSinkFixtures(t *testing.T) {
wantViolations(t, "a forwarded error", sinkFixture(t, "forwarded-error"),
- "emit.go:7 (jsonError in refuse): err.Error(...) forwards a call's result",
+ "emit.go:7 (jsonError in refuse): err.Error(...) forwards an error's text",
"emit.go:18 (Check.Reason in report): helper why, at ",
)
- wantViolations(t, "a message assigned to a local before the sink", sinkFixture(t, "local-variable"),
- "emit.go:8 (jsonError in refuse): msg is a variable",
- "emit.go:16 (jsonError in refuseLate): text is a variable",
- "emit.go:21 (jsonError in inside): gone holds a sentence assigned before the sink",
+ wantViolations(t, "a local with one assignment that is an error's text", sinkFixture(t, "local-variable"),
+ "emit.go:10 (jsonError in mixed): msg is assigned something that is not a sentence: err.Error(...) forwards an error's text",
+ "emit.go:15 (jsonError in fromMultiValue): text is assigned something that is not a sentence: helper reasonOf, at ",
+ "emit.go:24 (jsonError in rangeVar): n is assigned a value the gate cannot read",
+ )
+ wantViolations(t, "a helper with one bad return", sinkFixture(t, "helper-bad-return"),
+ "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",
+ )
+ wantViolations(t, "a parameter one call site gives an error", sinkFixture(t, "param"),
+ "emit.go:6 (jsonError in refuse): helper wrap, at ",
+ "emit.go:7 (jsonError in refuse): helper wrap, at ",
)
wantViolations(t, "values the gate cannot read", sinkFixture(t, "unresolved"),
- "emit.go:6 (jsonError in methodResult): s.render(...) forwards a call's result",
+ "emit.go:6 (jsonError in methodResult): s.render(...) forwards a call's result the gate cannot read",
"emit.go:10 (jsonError in indexed): names is not a sentence the gate can read",
"emit.go:14 (Check.Reason in bare): a + b is a concatenation with no literal part",
"emit.go:18 (jsonError in dynamicFormat): fmt.Sprintf(...) has a format that is not a literal",
@@ -1767,6 +2109,25 @@ func TestMovesSinkFixtures(t *testing.T) {
wantViolations(t, "sentences the gate can read", sinkFixture(t, "ok"))
}
+// A mapping helper may echo a refusal's text only in a case whose guard tests
+// it against literals.
+func TestMovesMappingHelperFixture(t *testing.T) {
+ file := filepath.Join("testdata", "moves-copy", "sinks", "mapping", "emit.go")
+ movesMappingHelpers[file] = map[string]bool{"mapRefusal": true, "unguarded": true, "badGuard": true}
+ defer delete(movesMappingHelpers, file)
+ _, vios := scanMovesFileFull(t, ".", file, false)
+ wantViolations(t, "a mapping helper", vios,
+ "emit.go:8 (jsonError in refuse): helper unguarded, at ",
+ "emit.go:9 (jsonError in refuse): helper badGuard, at ",
+ )
+ // Without the configuration the same echo is refused.
+ delete(movesMappingHelpers, file)
+ _, vios = scanMovesFileFull(t, ".", file, false)
+ if len(vios) != 3 {
+ t.Errorf("an unconfigured mapping helper: %d violations, want 3: %v", len(vios), vios)
+ }
+}
+
// An allow-list entry exempts a sink, but it needs a reason and must still
// exempt something.
func checkSinkAllow(allow map[string]map[string]string, vios []movesViolation) []string {
diff --git a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
index c0de569b4..c23bf82c7 100644
--- a/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
+++ b/docs/superpowers/plans/2026-10-06-moves-phase1-copy.md
@@ -169,8 +169,8 @@ Columns: id, file, text, status.
| MV-N127 | internal/host/movesreport.go | this box's secrets key is not the console's | pending |
| MV-N128 | internal/host/movesreport.go | this box could not compose the launch environment of the held row | pending |
| MV-N129 | internal/host/movesreport.go | the launch environment differs from the source's: <NAME (kind)>, ... | pending |
-| MV-N130 | internal/host/movesreport.go | <NAME> in the carried .env overrides the rendered configuration (a T3 note, not a failure) | pending |
-| MV-N131 | internal/host/movesreport.go | <NAME> (<kind>) (a T3 note for an advisory variable) | pending |
+| MV-N130 | internal/host/movesreport.go (a T3 note, not a failure) | <NAME> in the carried .env overrides the rendered configuration | pending |
+| MV-N131 | internal/host/movesreport.go (a T3 note for an advisory variable) | <NAME> (<kind>) | pending |
| MV-N132 | internal/host/movesreport.go | the store bound as <PREFIX> did not answer the probe from this box | pending |
| MV-N133 | internal/host/movesreport.go | this box could not read the archive the source published | pending |
| MV-N134 | internal/host/movesreport.go | the archive does not open with this box's key or does not match its manifest | pending |
diff --git a/testdata/moves-copy/sinks/helper-bad-return/emit.go b/testdata/moves-copy/sinks/helper-bad-return/emit.go
new file mode 100644
index 000000000..b15fed26e
--- /dev/null
+++ b/testdata/moves-copy/sinks/helper-bad-return/emit.go
@@ -0,0 +1,15 @@
+package fixture
+
+// Not compiled. A helper with one good return and one bad one is not a
+// sentence helper.
+
+func refuse(w int, n int, err error) {
+ jsonError(w, 400, say(n, err))
+}
+
+func say(n int, err error) string {
+ if n > 1 {
+ return "too many boxes"
+ }
+ return err.Error()
+}
diff --git a/testdata/moves-copy/sinks/local-variable/emit.go b/testdata/moves-copy/sinks/local-variable/emit.go
index 209ab06d0..4548762e8 100644
--- a/testdata/moves-copy/sinks/local-variable/emit.go
+++ b/testdata/moves-copy/sinks/local-variable/emit.go
@@ -1,22 +1,26 @@
package fixture
-// Not compiled. A sentence assigned to a local before the sink is never read
-// where it is spoken.
+// Not compiled. A local is a sentence only if every assignment to it is.
-func refuse(w int) {
- msg := "that region is not armed"
- jsonError(w, 400, msg)
-}
-
-func refuseLate(w int, bad bool) {
- var text string
+func mixed(w int, err error, bad bool) {
+ msg := "the host is already held"
if bad {
- text = "the host is already held"
+ msg = err.Error()
}
- jsonError(w, 409, text)
+ jsonError(w, 409, msg)
+}
+
+func fromMultiValue(w int, err error) {
+ text, _ := reasonOf(err)
+ jsonError(w, 400, text)
}
-func inside(w int) {
- gone := "the host was removed"
- jsonError(w, 404, "the instance says: "+gone)
+func reasonOf(err error) (string, bool) {
+ return err.Error(), true
+}
+
+func rangeVar(w int, names []string) {
+ for _, n := range names {
+ jsonError(w, 400, n)
+ }
}
diff --git a/testdata/moves-copy/sinks/mapping/emit.go b/testdata/moves-copy/sinks/mapping/emit.go
new file mode 100644
index 000000000..a528d6007
--- /dev/null
+++ b/testdata/moves-copy/sinks/mapping/emit.go
@@ -0,0 +1,36 @@
+package fixture
+
+// Not compiled. mapRefusal echoes a refusal only where its case tests the
+// text against literals; the others do not.
+
+func refuse(w int, err error, other string) {
+ jsonError(w, 409, mapRefusal(err))
+ jsonError(w, 409, unguarded(err))
+ jsonError(w, 409, badGuard(err, other))
+}
+
+func mapRefusal(err error) string {
+ msg := err.Error()
+ switch {
+ case msg == "no journaled move in flight", strings.HasPrefix(msg, "a move to ") && strings.Contains(msg, " is already "):
+ return msg
+ }
+ return "this move's state does not allow that"
+}
+
+func unguarded(err error) string {
+ msg := err.Error()
+ if len(msg) > 3 {
+ return msg
+ }
+ return "this move's state does not allow that"
+}
+
+func badGuard(err error, other string) string {
+ msg := err.Error()
+ switch {
+ case msg == other:
+ return msg
+ }
+ return "this move's state does not allow that"
+}
diff --git a/testdata/moves-copy/sinks/ok/emit.go b/testdata/moves-copy/sinks/ok/emit.go
index fe0e5b387..928422261 100644
--- a/testdata/moves-copy/sinks/ok/emit.go
+++ b/testdata/moves-copy/sinks/ok/emit.go
@@ -34,3 +34,38 @@ func tooMany(n int) string {
func checks(c Check) Check {
return Check{Reason: c.Reason}
}
+
+// A local whose every assignment is a sentence.
+func localLiteral(w int, bad bool) {
+ msg := "that region is not armed"
+ if bad {
+ msg = "that region is not hosting"
+ }
+ jsonError(w, 400, msg)
+}
+
+// A multi-value helper whose string results are all sentences.
+func multi(w int) {
+ text, _ := explain(1)
+ jsonError(w, 400, text)
+}
+
+func explain(n int) (string, bool) {
+ if n > 0 {
+ return "the app is not enabled", true
+ }
+ return fmt.Sprintf("%d apps are not enabled", n), false
+}
+
+// A parameter whose every call site passes a sentence.
+func viaParam(w int) {
+ jsonError(w, 400, wrapOK("first"))
+}
+
+func wrapOK(m string) string {
+ return m
+}
+
+func callsAreLiterals() string {
+ return wrapOK("second")
+}
diff --git a/testdata/moves-copy/sinks/param/emit.go b/testdata/moves-copy/sinks/param/emit.go
new file mode 100644
index 000000000..7c15ce2cc
--- /dev/null
+++ b/testdata/moves-copy/sinks/param/emit.go
@@ -0,0 +1,12 @@
+package fixture
+
+// Not compiled. A parameter is a sentence only if every call site passes one.
+
+func refuse(w int, err error) {
+ jsonError(w, 400, wrap("a literal"))
+ jsonError(w, 400, wrap(err.Error()))
+}
+
+func wrap(m string) string {
+ return m
+}