amadan
copy gate fix round 3: the checker fails closed
One evaluator derives the text of every value reaching a sink: parameters are bound to arguments, named results and bare returns are read, fields are sentences only if every write is, callees resolve by declared type, and a value it cannot derive, a pattern with no text of its own, or a limit is a violation and an unrecorded sentence. Function values of sentence-flow functions are refused. Negative fixtures for each counterexample of the re-review.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
12 files changed,
+1966
−1034
approved_moves_copy_scan_test.go+1570 −0approved_moves_copy_test.go+205 −1030testdata/moves-copy/sinks/field-reason/emit.go+23 −0testdata/moves-copy/sinks/funcvalue/emit.go+29 −0testdata/moves-copy/sinks/interface/emit.go+22 −0testdata/moves-copy/sinks/limit/emit.go+36 −0testdata/moves-copy/sinks/named-result/emit.go+12 −0testdata/moves-copy/sinks/ok/emit.go+11 −4testdata/moves-copy/sinks/ok/record.md+15 −0testdata/moves-copy/sinks/shadow/emit.go+13 −0testdata/moves-copy/sinks/sprintf-v/emit.go+18 −0testdata/moves-copy/sinks/substitution/emit.go+12 −0
diff --git a/approved_moves_copy_scan_test.go b/approved_moves_copy_scan_test.go| new file mode 100644 |
| index 000000000..5f6b8bba2 |
| --- /dev/null |
| +++ b/approved_moves_copy_scan_test.go |
| @@ -0,0 +1,1570 @@ |
| +// approved_moves_copy_scan_test.go |
| +package main |
| + |
| +import ( |
| + "fmt" |
| + "go/ast" |
| + "go/parser" |
| + "go/token" |
| + "os" |
| + "path/filepath" |
| + "sort" |
| + "strconv" |
| + "strings" |
| + "testing" |
| +) |
| + |
| +// The analyser behind TestMovesStringsAreRecorded and TestMovesSinksAreSentences. |
| +// |
| +// It fails closed. A value that reaches an operator sink (a jsonError message, |
| +// a Check's Reason, a Phase 1 copy table, a sentence-returning function) must |
| +// be one the analyser can positively derive to text: a literal, a const, a |
| +// literal-format Sprintf or concatenation, a local whose every assignment is |
| +// such text, a call to a package helper whose every return is, a parameter |
| +// bound to such an argument, or a field whose every write in the scanned files |
| +// is. The text it derives is matched against the record. Anything it cannot |
| +// derive is a violation, and a pattern with no text of its own (a bare error |
| +// forwarded through "%v" or "" + err.Error()) is a violation too and an |
| +// 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. |
| + |
| +// 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 |
| +// placeholder is forwarded, not wild, when every write to it is a sentence. |
| +var movesSentenceFields = map[string]bool{"Reason": true} |
| + |
| +// fwdMark stands for a value that is a sink somewhere else (a forwarded |
| +// parameter, a field read): it is checked where it is made, not here. |
| +const fwdMark = "\x01" |
| + |
| +const ( |
| + maxEvalDepth = 10 |
| + maxAlts = 64 |
| + maxEvalSteps = 3000000 |
| +) |
| + |
| +type problem struct { |
| + msg string |
| + hard bool // survives a placeholder: a limit, an ambiguity |
| +} |
| + |
| +func (p problem) String() string { return p.msg } |
| + |
| +// ---- the package index ---- |
| + |
| +type movesPkgIndex struct { |
| + vars map[string]ast.Expr |
| + funcs map[string]*ast.FuncDecl |
| + methods map[string][]*ast.FuncDecl |
| + types map[string]ast.Expr |
| + decls []*ast.FuncDecl |
| +} |
| + |
| +var movesPkgCache = map[string]*movesPkgIndex{} |
| + |
| +// movesFset positions every file the scan parses, so a violation inside a |
| +// 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{}, types: map[string]ast.Expr{}} |
| +} |
| + |
| +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 { |
| + switch s := s.(type) { |
| + case *ast.ValueSpec: |
| + if len(s.Names) == len(s.Values) { |
| + for i, nm := range s.Names { |
| + p.vars[nm.Name] = s.Values[i] |
| + } |
| + } |
| + case *ast.TypeSpec: |
| + p.types[s.Name.Name] = s.Type |
| + } |
| + } |
| + } |
| + } |
| +} |
| + |
| +func loadMovesPkg(t *testing.T, dir string) *movesPkgIndex { |
| + t.Helper() |
| + if p, ok := movesPkgCache[dir]; ok { |
| + return p |
| + } |
| + p := newPkgIndex() |
| + movesPkgCache[dir] = p |
| + ents, err := os.ReadDir(dir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, e := range ents { |
| + n := e.Name() |
| + if e.IsDir() || !strings.HasSuffix(n, ".go") || strings.HasSuffix(n, "_test.go") { |
| + continue |
| + } |
| + f, err := parser.ParseFile(movesFset, filepath.Join(dir, n), nil, 0) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + p.addFile(f) |
| + } |
| + return p |
| +} |
| + |
| +func recvBase(d *ast.FuncDecl) string { |
| + if d.Recv == nil || len(d.Recv.List) == 0 { |
| + return "" |
| + } |
| + return baseTypeName(d.Recv.List[0].Type) |
| +} |
| + |
| +func baseTypeName(e ast.Expr) string { |
| + switch e := e.(type) { |
| + case *ast.Ident: |
| + return e.Name |
| + case *ast.StarExpr: |
| + return baseTypeName(e.X) |
| + case *ast.ParenExpr: |
| + return baseTypeName(e.X) |
| + case *ast.SelectorExpr: |
| + return baseTypeName(e.X) + "." + e.Sel.Name // another package's type: never a package method |
| + } |
| + return "" |
| +} |
| + |
| +func paramNames(ft *ast.FuncType) []string { |
| + var out []string |
| + if ft.Params == nil { |
| + return out |
| + } |
| + for _, f := range ft.Params.List { |
| + if len(f.Names) == 0 { |
| + out = append(out, "_") |
| + } |
| + for _, n := range f.Names { |
| + out = append(out, n.Name) |
| + } |
| + } |
| + 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 n |
| +} |
| + |
| +// stringResults is the set of a function's result positions that are strings. |
| +func stringResults(d *ast.FuncDecl) map[int]bool { |
| + out := map[int]bool{} |
| + if d.Type.Results == nil { |
| + return out |
| + } |
| + i := 0 |
| + for _, f := range d.Type.Results.List { |
| + n := len(f.Names) |
| + if n == 0 { |
| + n = 1 |
| + } |
| + if id, ok := f.Type.(*ast.Ident); ok && id.Name == "string" { |
| + for k := 0; k < n; k++ { |
| + out[i+k] = true |
| + } |
| + } |
| + i += n |
| + } |
| + return out |
| +} |
| + |
| +func namedResults(d *ast.FuncDecl) []string { |
| + var out []string |
| + if d.Type.Results == nil { |
| + return out |
| + } |
| + for _, f := range d.Type.Results.List { |
| + for _, n := range f.Names { |
| + out = append(out, n.Name) |
| + } |
| + } |
| + return out |
| +} |
| + |
| +// ---- local assignments ---- |
| + |
| +// localAssign is one assignment to a variable of a function: an expression, a |
| +// position of a multi-value call, 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 |
| + typ ast.Expr // a declared type, when there is one |
| + typeFrom ast.Expr // for an unreadable value, the expression whose type it has |
| +} |
| + |
| +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 |
| + } |
| + 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 if ix, ok := n.Rhs[0].(*ast.IndexExpr); ok && i == 0 { |
| + out[id.Name] = append(out[id.Name], localAssign{unreadable: true, typeFrom: ix}) |
| + } 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], typ: n.Type}) |
| + case len(n.Values) == 0: |
| + out[id.Name] = append(out[id.Name], localAssign{zero: true, typ: n.Type}) |
| + 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, typ: n.Type}) |
| + } else { |
| + out[id.Name] = append(out[id.Name], localAssign{unreadable: true}) |
| + } |
| + } |
| + } |
| + 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 |
| +} |
| + |
| +// ---- the universe: every struct field and every write to it ---- |
| + |
| +type fieldWrite struct { |
| + file string |
| + fd *ast.FuncDecl |
| + 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 |
| +} |
| + |
| +type fieldResult struct { |
| + done bool |
| + probs []problem |
| +} |
| + |
| +type movesUniverse struct { |
| + t *testing.T |
| + root string |
| + fieldWrites map[string][]*fieldWrite |
| + fieldTypes map[string]map[string]bool |
| + structFields map[string][]string |
| + fieldTypeOf map[string]map[string]ast.Expr |
| + results map[string]*fieldResult |
| + used map[string]bool // package functions evaluated as sentence helpers |
| + paramSinks map[string]bool // functions whose parameter reaches a sink |
| + files []string |
| + steps int |
| +} |
| + |
| +func (u *movesUniverse) at(p string) string { |
| + if u.root == "." { |
| + return p |
| + } |
| + return filepath.Join(u.root, p) |
| +} |
| + |
| +func newUniverse(t *testing.T, root string, files []string) *movesUniverse { |
| + t.Helper() |
| + u := &movesUniverse{t: t, root: root, fieldWrites: map[string][]*fieldWrite{}, fieldTypes: map[string]map[string]bool{}, |
| + structFields: map[string][]string{}, fieldTypeOf: map[string]map[string]ast.Expr{}, results: map[string]*fieldResult{}, used: map[string]bool{}, |
| + paramSinks: map[string]bool{}, files: files} |
| + parsed := map[string]*ast.File{} |
| + for _, f := range files { |
| + af, err := parser.ParseFile(movesFset, u.at(f), nil, 0) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + parsed[f] = af |
| + } |
| + addType := func(field, typ string) { |
| + if u.fieldTypes[field] == nil { |
| + u.fieldTypes[field] = map[string]bool{} |
| + } |
| + u.fieldTypes[field][typ] = true |
| + } |
| + structNames := map[*ast.StructType]string{} |
| + for _, af := range parsed { |
| + ast.Inspect(af, func(n ast.Node) bool { |
| + if ts, ok := n.(*ast.TypeSpec); ok { |
| + if st, ok := ts.Type.(*ast.StructType); ok { |
| + structNames[st] = ts.Name.Name |
| + } |
| + } |
| + return true |
| + }) |
| + } |
| + for _, af := range parsed { |
| + ast.Inspect(af, func(n ast.Node) bool { |
| + st, ok := n.(*ast.StructType) |
| + if !ok { |
| + return true |
| + } |
| + name, named := structNames[st] |
| + if !named { |
| + name = fmt.Sprintf("anonymous struct at %s", movesFset.Position(st.Pos())) |
| + structNames[st] = name |
| + } |
| + var order []string |
| + if u.fieldTypeOf[name] == nil { |
| + u.fieldTypeOf[name] = map[string]ast.Expr{} |
| + } |
| + for _, f := range st.Fields.List { |
| + for _, nm := range f.Names { |
| + order = append(order, nm.Name) |
| + addType(nm.Name, name) |
| + u.fieldTypeOf[name][nm.Name] = f.Type |
| + } |
| + } |
| + u.structFields[name] = order |
| + return true |
| + }) |
| + } |
| + for _, f := range files { |
| + af := parsed[f] |
| + var fd *ast.FuncDecl |
| + var visit func(n ast.Node) bool |
| + visit = func(n ast.Node) bool { |
| + switch n := n.(type) { |
| + case *ast.FuncDecl: |
| + fd = n |
| + if n.Body != nil { |
| + ast.Inspect(n.Body, visit) |
| + } |
| + fd = nil |
| + return false |
| + case *ast.CompositeLit: |
| + var order []string |
| + ctype := "" |
| + switch ty := n.Type.(type) { |
| + case *ast.Ident: |
| + order = u.structFields[ty.Name] |
| + ctype = ty.Name |
| + case *ast.SelectorExpr: |
| + order = u.structFields[ty.Sel.Name] |
| + ctype = ty.Sel.Name |
| + case *ast.StructType: |
| + order = u.structFields[structNames[ty]] |
| + ctype = structNames[ty] |
| + } |
| + 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}) |
| + } |
| + } else if i < len(order) { |
| + u.fieldWrites[order[i]] = append(u.fieldWrites[order[i]], &fieldWrite{f, fd, el, ctype, nil}) |
| + } |
| + } |
| + 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}) |
| + } |
| + } |
| + } |
| + return true |
| + } |
| + ast.Inspect(af, visit) |
| + } |
| + return u |
| +} |
| + |
| +var movesDefaultUniverse *movesUniverse |
| + |
| +// universeFor picks the universe a file is read in: the real tree's scanned |
| +// files, or just the file itself (a fixture, or a base-commit copy). |
| +func universeFor(t *testing.T, root, file string) *movesUniverse { |
| + t.Helper() |
| + if root == "." { |
| + for _, f := range movesScanFiles { |
| + if f == file { |
| + if movesDefaultUniverse == nil { |
| + movesDefaultUniverse = newUniverse(t, ".", movesScanFiles) |
| + } |
| + return movesDefaultUniverse |
| + } |
| + } |
| + } |
| + return newUniverse(t, root, []string{file}) |
| +} |
| + |
| +// ---- evaluation ---- |
| + |
| +type evalCtx struct { |
| + u *movesUniverse |
| + file string |
| + fd *ast.FuncDecl |
| + pkg *movesPkgIndex |
| + locals map[string][]localAssign |
| + bind map[string][]string |
| + fwd map[string]bool |
| + fwdFn map[string]bool |
| + curReturn *ast.ReturnStmt |
| + noCalls bool |
| + visiting map[string]bool |
| +} |
| + |
| +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{}} |
| + c.pkg = loadMovesPkg(u.t, u.at(filepath.Dir(file))) |
| + if fd == nil { |
| + return c |
| + } |
| + if i, ok := movesLocalSinks[file][fd.Name.Name]; ok { |
| + if ps := paramNames(fd.Type); i < len(ps) { |
| + c.fwd[ps[i]] = true |
| + } |
| + } |
| + if i, ok := movesFuncSinks[file][fd.Name.Name]; ok { |
| + if ps := paramNames(fd.Type); i < len(ps) { |
| + c.fwdFn[ps[i]] = true |
| + } |
| + } |
| + // A closure that is a local sink (fail := func(why string)) forwards its parameter. |
| + ast.Inspect(fd.Body, func(n ast.Node) bool { |
| + as, ok := n.(*ast.AssignStmt) |
| + if !ok || len(as.Lhs) != 1 || len(as.Rhs) != 1 { |
| + return true |
| + } |
| + id, ok := as.Lhs[0].(*ast.Ident) |
| + lit, isLit := as.Rhs[0].(*ast.FuncLit) |
| + if !ok || !isLit { |
| + return true |
| + } |
| + if i, ok := movesLocalSinks[file][id.Name]; ok { |
| + if ps := paramNames(lit.Type); i < len(ps) { |
| + c.fwd[ps[i]] = true |
| + } |
| + } |
| + return true |
| + }) |
| + return c |
| +} |
| + |
| +func (c *evalCtx) posOf(n ast.Node) string { return movesFset.Position(n.Pos()).String() } |
| + |
| +func hardOnly(ps []problem) []problem { |
| + var out []problem |
| + for _, p := range ps { |
| + if p.hard { |
| + out = append(out, p) |
| + } |
| + } |
| + return out |
| +} |
| + |
| +func limit(c *evalCtx, n ast.Node, what string) []problem { |
| + return []problem{{fmt.Sprintf("analysis limit reached at %s: %s", c.posOf(n), what), true}} |
| +} |
| + |
| +func cross(l, r []string) ([]string, bool) { |
| + if len(l)*len(r) > maxAlts { |
| + return nil, false |
| + } |
| + out := make([]string, 0, len(l)*len(r)) |
| + for _, a := range l { |
| + for _, b := range r { |
| + out = append(out, a+b) |
| + } |
| + } |
| + return out, true |
| +} |
| + |
| +func dedupe(in []string) []string { |
| + seen := map[string]bool{} |
| + var out []string |
| + for _, s := range in { |
| + if !seen[s] { |
| + seen[s] = true |
| + out = append(out, s) |
| + } |
| + } |
| + return out |
| +} |
| + |
| +// 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. |
| +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. |
| + if movesSentenceFields[sel.Sel.Name] { |
| + if a, p := c.fieldRead(sel); len(p) == 0 { |
| + return a, nil |
| + } |
| + } |
| + return []string{wild}, nil |
| + } |
| + alts, probs := c.eval(e, depth) |
| + if len(probs) == 0 { |
| + return alts, nil |
| + } |
| + return []string{wild}, hardOnly(probs) |
| +} |
| + |
| +// 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 { |
| + return nil, limit(c, e, "expression nested too deeply") |
| + } |
| + if c.u.steps++; c.u.steps > maxEvalSteps { |
| + return nil, limit(c, e, "too much work reading this value") |
| + } |
| + soft := func(format string, a ...any) ([]string, []problem) { |
| + return nil, []problem{{fmt.Sprintf(format, a...), false}} |
| + } |
| + switch e := e.(type) { |
| + case *ast.ParenExpr: |
| + return c.eval(e.X, depth) |
| + case *ast.BasicLit: |
| + if e.Kind != token.STRING { |
| + return soft("%s is not a string", e.Value) |
| + } |
| + v, err := strconv.Unquote(e.Value) |
| + if err != nil { |
| + return soft("%s is not a string literal", e.Value) |
| + } |
| + return []string{v}, nil |
| + case *ast.Ident: |
| + return c.evalIdent(e, depth) |
| + case *ast.BinaryExpr: |
| + 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...) |
| + } |
| + out, ok := cross(l, r) |
| + if !ok { |
| + return nil, limit(c, e, "too many alternative sentences") |
| + } |
| + return dedupe(out), nil |
| + case *ast.SelectorExpr: |
| + if c.noCalls { |
| + return []string{wild}, nil |
| + } |
| + return c.fieldRead(e) |
| + case *ast.CallExpr: |
| + return c.evalCall(e, 0, depth) |
| + } |
| + return soft("%s is not a sentence the gate can read", render(e)) |
| +} |
| + |
| +func (c *evalCtx) evalIdent(id *ast.Ident, depth int) ([]string, []problem) { |
| + if depth > maxEvalDepth { |
| + return nil, limit(c, id, "alias chain too long") |
| + } |
| + if c.guardedEcho(id) { |
| + return []string{fwdMark}, nil |
| + } |
| + var alts []string |
| + var probs []problem |
| + known := false |
| + if b, ok := c.bind[id.Name]; ok { |
| + known = true |
| + alts = append(alts, b...) |
| + } else if c.fwd[id.Name] { |
| + known = true |
| + alts = append(alts, fwdMark) |
| + } |
| + if as, ok := c.locals[id.Name]; ok { |
| + known = true |
| + key := fmt.Sprintf("local %p %s", c.fd, id.Name) |
| + if c.visiting[key] { |
| + return nil, []problem{{fmt.Sprintf("%s at %s is assigned from itself", id.Name, c.posOf(id)), true}} |
| + } |
| + c.visiting[key] = true |
| + for _, a := range as { |
| + switch { |
| + case a.zero: |
| + alts = append(alts, "") |
| + case a.rhs != nil: |
| + x, p := c.eval(a.rhs, depth+1) |
| + alts, probs = append(alts, x...), append(probs, p...) |
| + case a.call != nil: |
| + x, p := c.evalCall(a.call, a.pos, depth+1) |
| + alts, probs = append(alts, x...), append(probs, p...) |
| + default: |
| + probs = append(probs, problem{fmt.Sprintf("%s is assigned a value the gate cannot read (a range variable or a destructured value)", id.Name), false}) |
| + } |
| + } |
| + delete(c.visiting, key) |
| + } |
| + if known { |
| + if len(alts) > maxAlts { |
| + return nil, limit(c, id, "too many alternative sentences") |
| + } |
| + return dedupe(alts), probs |
| + } |
| + if x, ok := c.pkg.vars[id.Name]; ok { |
| + pc := c.u.newCtx(c.file, nil, c.noCalls) |
| + pc.visiting = c.visiting |
| + key := "pkgvar " + id.Name |
| + if c.visiting[key] { |
| + return nil, []problem{{fmt.Sprintf("%s is defined in terms of itself", id.Name), true}} |
| + } |
| + c.visiting[key] = true |
| + defer delete(c.visiting, key) |
| + return pc.eval(x, depth+1) |
| + } |
| + if c.fd != nil { |
| + for _, n := range paramNames(c.fd.Type) { |
| + if n == id.Name { |
| + return nil, []problem{{fmt.Sprintf("parameter %s is not bound to an argument (%s)", id.Name, c.posOf(id)), false}} |
| + } |
| + } |
| + } |
| + return nil, []problem{{fmt.Sprintf("%s is a variable the gate cannot read", id.Name), false}} |
| +} |
| + |
| +// fieldRead reads X.F: a sentence only if every write to a field named F, in |
| +// any struct of the scanned files (a keyed or positional composite literal, or |
| +// an assignment), is. The text of each write is checked where it is written, |
| +// so the read is forwarded. |
| +func (c *evalCtx) fieldRead(e *ast.SelectorExpr) ([]string, []problem) { |
| + f := e.Sel.Name |
| + types := c.u.fieldTypes[f] |
| + if len(types) == 0 { |
| + return nil, []problem{{fmt.Sprintf("%s: no struct in the scanned files declares a field %s", render(e), f), false}} |
| + } |
| + rt := lastName(baseTypeName(c.typeExprOf(e.X, 0))) |
| + key := f + "|" + rt |
| + res := c.u.results[key] |
| + if res == nil { |
| + res = &fieldResult{} |
| + c.u.results[key] = res |
| + n := 0 |
| + for _, w := range c.u.fieldWrites[f] { |
| + wc := c.u.newCtx(w.file, w.fd, c.noCalls) |
| + // A write whose struct type is known and is not the one read is another |
| + // field that happens to share the name. |
| + wt := lastName(w.ctype) |
| + if w.recv != nil { |
| + wt = lastName(baseTypeName(wc.typeExprOf(w.recv, 0))) |
| + } |
| + if rt != "" && wt != "" && wt != rt { |
| + continue |
| + } |
| + n++ |
| + 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}) |
| + } |
| + for _, a := range alts { |
| + if a != "" && !strings.Contains(a, fwdMark) && !informative(a) { |
| + res.probs = append(res.probs, problem{fmt.Sprintf("a write to field %s at %s has no text of its own (%q)", f, wc.posOf(w.expr), strings.ReplaceAll(a, wild, "%")), false}) |
| + } |
| + } |
| + } |
| + if n == 0 { |
| + res.probs = append(res.probs, problem{fmt.Sprintf("field %s is never written in the scanned files", f), false}) |
| + } |
| + } |
| + if len(res.probs) > 0 { |
| + return nil, res.probs |
| + } |
| + return []string{fwdMark}, nil |
| +} |
| + |
| +func lastName(s string) string { |
| + if i := strings.LastIndex(s, "."); i >= 0 { |
| + return s[i+1:] |
| + } |
| + return s |
| +} |
| + |
| +func isFormatCall(e *ast.CallExpr) bool { |
| + sel, ok := e.Fun.(*ast.SelectorExpr) |
| + if !ok || len(e.Args) == 0 { |
| + return false |
| + } |
| + id, ok := sel.X.(*ast.Ident) |
| + return ok && ((id.Name == "fmt" && (sel.Sel.Name == "Sprintf" || sel.Sel.Name == "Errorf")) || |
| + (id.Name == "errors" && sel.Sel.Name == "New")) |
| +} |
| + |
| +// evalCall reads result pos of a call. |
| +func (c *evalCtx) evalCall(e *ast.CallExpr, pos, depth int) ([]string, []problem) { |
| + if depth > maxEvalDepth { |
| + return nil, limit(c, e, "call nesting too deep") |
| + } |
| + soft := func(format string, a ...any) ([]string, []problem) { |
| + return nil, []problem{{fmt.Sprintf(format, a...), false}} |
| + } |
| + if isFormatCall(e) { |
| + fmts, probs := c.eval(e.Args[0], depth+1) |
| + if len(probs) > 0 { |
| + return nil, append([]problem{{fmt.Sprintf("%s has a format that is not a literal", render(e)), false}}, probs...) |
| + } |
| + var out []string |
| + 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)) |
| + } |
| + return dedupe(out), nil |
| + } |
| + if c.noCalls { |
| + return []string{wild}, nil |
| + } |
| + if sel, ok := e.Fun.(*ast.SelectorExpr); ok && len(e.Args) == 2 { |
| + if x, ok := sel.X.(*ast.Ident); ok && x.Name == "strings" && sel.Sel.Name == "TrimPrefix" { |
| + return c.evalTrimPrefix(e, depth) |
| + } |
| + } |
| + decls, prob := c.resolveCallees(e) |
| + if prob != nil { |
| + return nil, []problem{*prob} |
| + } |
| + if len(decls) == 1 && c.fwdFn[renderName(e.Fun)] { |
| + return []string{fwdMark}, nil |
| + } |
| + var alts []string |
| + var probs []problem |
| + for _, d := range decls { |
| + a, p := c.callDecl(d, e, pos, depth) |
| + alts, probs = append(alts, a...), append(probs, p...) |
| + } |
| + if len(alts) > maxAlts { |
| + return nil, limit(c, e, "too many alternative sentences") |
| + } |
| + return dedupe(alts), probs |
| +} |
| + |
| +func renderName(e ast.Expr) string { |
| + if id, ok := e.(*ast.Ident); ok { |
| + return id.Name |
| + } |
| + return "" |
| +} |
| + |
| +// resolveCallees finds the package function or methods a call reaches, by |
| +// name for a function and by the receiver's declared type for a method. |
| +func (c *evalCtx) resolveCallees(e *ast.CallExpr) ([]*ast.FuncDecl, *problem) { |
| + bad := func(format string, a ...any) ([]*ast.FuncDecl, *problem) { |
| + return nil, &problem{fmt.Sprintf(format, a...), false} |
| + } |
| + switch fn := e.Fun.(type) { |
| + case *ast.Ident: |
| + _, isLocal := c.locals[fn.Name] |
| + isParam := false |
| + if c.fd != nil { |
| + for _, p := range paramNames(c.fd.Type) { |
| + if p == fn.Name { |
| + isParam = true |
| + } |
| + } |
| + } |
| + d, isPkg := c.pkg.funcs[fn.Name] |
| + switch { |
| + case (isLocal || isParam) && isPkg: |
| + return bad("%s at %s names both a local function value and a package function", fn.Name, c.posOf(fn)) |
| + case c.fwdFn[fn.Name]: |
| + return []*ast.FuncDecl{{Name: fn}}, nil |
| + case isLocal || isParam: |
| + return bad("%s(...) calls a function value the gate cannot read", fn.Name) |
| + case isPkg: |
| + return []*ast.FuncDecl{d}, nil |
| + } |
| + 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)) |
| + } |
| + tx := c.typeExprOf(fn.X, 0) |
| + t := baseTypeName(tx) |
| + if t == "" { |
| + return bad("%s calls out of the package or on a value the gate cannot type", render(e)) |
| + } |
| + if _, iface := c.pkg.types[t].(*ast.InterfaceType); iface { |
| + return bad("%s at %s is a call through the interface %s, whose implementation the gate cannot see", render(e), c.posOf(e), t) |
| + } |
| + var out []*ast.FuncDecl |
| + for _, d := range c.pkg.methods[fn.Sel.Name] { |
| + if recvBase(d) == t { |
| + out = append(out, d) |
| + } |
| + } |
| + if len(out) == 0 { |
| + return bad("%s: no method %s on %s in this package", render(e), fn.Sel.Name, t) |
| + } |
| + return out, nil |
| + } |
| + return bad("%s is not a call the gate can read", render(e)) |
| +} |
| + |
| +// 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) { |
| + xs, xp := c.eval(e.Args[0], depth+1) |
| + ps, pp := c.eval(e.Args[1], depth+1) |
| + if len(xp)+len(pp) > 0 { |
| + return nil, append(xp, pp...) |
| + } |
| + var out []string |
| + for _, x := range xs { |
| + for _, p := range ps { |
| + if strings.Contains(p, wild) || strings.Contains(p, fwdMark) { |
| + return nil, []problem{{fmt.Sprintf("%s trims a prefix that is not a literal", render(e)), false}} |
| + } |
| + out = append(out, strings.TrimPrefix(x, p)) |
| + } |
| + } |
| + return dedupe(out), nil |
| +} |
| + |
| +func unstar(e ast.Expr) ast.Expr { |
| + for { |
| + switch x := e.(type) { |
| + case *ast.StarExpr: |
| + e = x.X |
| + case *ast.ParenExpr: |
| + e = x.X |
| + default: |
| + return e |
| + } |
| + } |
| +} |
| + |
| +// resultTypeAt is the declared type of result pos of d. |
| +func resultTypeAt(d *ast.FuncDecl, pos int) ast.Expr { |
| + if d.Type.Results == nil { |
| + return nil |
| + } |
| + i := 0 |
| + for _, f := range d.Type.Results.List { |
| + n := len(f.Names) |
| + if n == 0 { |
| + n = 1 |
| + } |
| + if pos < i+n { |
| + return f.Type |
| + } |
| + i += n |
| + } |
| + return nil |
| +} |
| + |
| +// typeExprOf finds the declared type of an expression, as far as the source |
| +// says: a receiver, parameter or typed variable, a composite literal, a field |
| +// of a known struct, an element of a known map or slice, a call's result. |
| +func (c *evalCtx) typeExprOf(e ast.Expr, depth int) ast.Expr { |
| + if depth > 6 { |
| + return nil |
| + } |
| + switch e := e.(type) { |
| + case *ast.ParenExpr: |
| + return c.typeExprOf(e.X, depth+1) |
| + case *ast.UnaryExpr: |
| + if e.Op == token.AND { |
| + return c.typeExprOf(e.X, depth+1) |
| + } |
| + case *ast.CompositeLit: |
| + return e.Type |
| + case *ast.Ident: |
| + if c.fd != nil { |
| + if c.fd.Recv != nil { |
| + for _, f := range c.fd.Recv.List { |
| + for _, n := range f.Names { |
| + if n.Name == e.Name { |
| + return f.Type |
| + } |
| + } |
| + } |
| + } |
| + if c.fd.Type.Params != nil { |
| + for _, f := range c.fd.Type.Params.List { |
| + for _, n := range f.Names { |
| + if n.Name == e.Name { |
| + return f.Type |
| + } |
| + } |
| + } |
| + } |
| + } |
| + for _, a := range c.locals[e.Name] { |
| + switch { |
| + case a.typ != nil: |
| + return a.typ |
| + case a.call != nil: |
| + if decls, p := c.resolveCallees(a.call); p == nil && len(decls) > 0 && decls[0].Type != nil { |
| + if tx := resultTypeAt(decls[0], a.pos); tx != nil { |
| + return tx |
| + } |
| + } |
| + case a.rhs != nil: |
| + if tx := c.typeExprOf(a.rhs, depth+1); tx != nil { |
| + return tx |
| + } |
| + case a.typeFrom != nil: |
| + if tx := c.typeExprOf(a.typeFrom, depth+1); tx != nil { |
| + return tx |
| + } |
| + } |
| + } |
| + case *ast.SelectorExpr: |
| + base := lastName(baseTypeName(c.typeExprOf(e.X, depth+1))) |
| + if ft, ok := c.u.fieldTypeOf[base][e.Sel.Name]; ok { |
| + return ft |
| + } |
| + case *ast.IndexExpr: |
| + switch x := unstar(c.typeExprOf(e.X, depth+1)).(type) { |
| + case *ast.MapType: |
| + return x.Value |
| + case *ast.ArrayType: |
| + return x.Elt |
| + } |
| + case *ast.CallExpr: |
| + if decls, p := c.resolveCallees(e); p == nil && len(decls) > 0 && decls[0].Type != nil { |
| + return resultTypeAt(decls[0], 0) |
| + } |
| + } |
| + return nil |
| +} |
| + |
| +// callDecl evaluates result pos of d called as call: its parameters bound to |
| +// the arguments (read as placeholders), then every return. |
| +func (c *evalCtx) callDecl(d *ast.FuncDecl, call *ast.CallExpr, pos, depth int) ([]string, []problem) { |
| + if d.Body == nil || !stringResults(d)[pos] { |
| + return nil, []problem{{fmt.Sprintf("%s(...) does not return a sentence at result %d", d.Name.Name, pos), false}} |
| + } |
| + key := fmt.Sprintf("helper %p %d", d, pos) |
| + if c.visiting[key] { |
| + return nil, []problem{{fmt.Sprintf("%s calls itself at %s", d.Name.Name, c.posOf(call)), true}} |
| + } |
| + c.visiting[key] = true |
| + defer delete(c.visiting, key) |
| + c.u.used[d.Name.Name] = true |
| + |
| + dfile := strings.TrimPrefix(movesFset.Position(d.Pos()).Filename, c.u.root+"/") |
| + hc := c.u.newCtx(dfile, d, c.noCalls) |
| + hc.visiting = c.visiting |
| + params := paramNames(d.Type) |
| + var probs []problem |
| + for i, p := range params { |
| + if i < len(call.Args) { |
| + a, pr := c.operand(call.Args[i], depth+1) |
| + hc.bind[p] = a |
| + probs = append(probs, pr...) |
| + } else { |
| + hc.bind[p] = []string{wild} |
| + } |
| + } |
| + nres := resultCount(d) |
| + named := namedResults(d) |
| + var alts []string |
| + ast.Inspect(d.Body, func(n ast.Node) bool { |
| + switch n := n.(type) { |
| + case *ast.FuncLit: |
| + return false |
| + case *ast.ReturnStmt: |
| + hc.curReturn = n |
| + var a []string |
| + var p []problem |
| + switch { |
| + case len(n.Results) == nres && pos < len(n.Results): |
| + a, p = hc.eval(n.Results[pos], depth+1) |
| + case len(n.Results) == 1 && nres > 1: |
| + if cl, ok := n.Results[0].(*ast.CallExpr); ok { |
| + a, p = hc.evalCall(cl, pos, depth+1) |
| + } |
| + case len(n.Results) == 0 && pos < len(named): |
| + a, p = hc.evalIdent(&ast.Ident{Name: named[pos], NamePos: n.Pos()}, depth+1) |
| + default: |
| + p = []problem{{fmt.Sprintf("return at %s has a shape the gate cannot read", hc.posOf(n)), false}} |
| + } |
| + hc.curReturn = nil |
| + for _, x := range p { |
| + probs = append(probs, problem{fmt.Sprintf("helper %s, at %s: %s", d.Name.Name, hc.posOf(n), x.msg), x.hard}) |
| + } |
| + alts = append(alts, a...) |
| + } |
| + return true |
| + }) |
| + return alts, probs |
| +} |
| + |
| +// ---- guarded echo (a mapping helper) ---- |
| + |
| +func (c *evalCtx) 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 |
| +} |
| + |
| +func (c *evalCtx) isLiteralText(e ast.Expr) bool { |
| + switch e := e.(type) { |
| + case *ast.ParenExpr: |
| + return c.isLiteralText(e.X) |
| + case *ast.BasicLit: |
| + return e.Kind == token.STRING |
| + case *ast.Ident: |
| + if _, local := c.locals[e.Name]; local { |
| + return false |
| + } |
| + _, ok := c.pkg.vars[e.Name] |
| + return ok |
| + } |
| + return false |
| +} |
| + |
| +func guardsLiteral(g ast.Expr, name string, c *evalCtx) 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.isLiteralText(g.Y) && !(rok && r.Name == name) |
| + case rok && r.Name == name: |
| + return c.isLiteralText(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.isLiteralText(g.Args[1]) |
| + } |
| + } |
| + return false |
| +} |
| + |
| +func render(e ast.Expr) string { |
| + var b strings.Builder |
| + ast.Inspect(e, func(n ast.Node) bool { |
| + switch n := n.(type) { |
| + case *ast.BinaryExpr: |
| + b.WriteString(render(n.X) + " " + n.Op.String() + " " + render(n.Y)) |
| + return false |
| + case *ast.Ident: |
| + b.WriteString(n.Name) |
| + case *ast.SelectorExpr: |
| + b.WriteString(render(n.X) + "." + n.Sel.Name) |
| + return false |
| + case *ast.CallExpr: |
| + b.WriteString(render(n.Fun) + "(...)") |
| + return false |
| + } |
| + return true |
| + }) |
| + return b.String() |
| +} |
| + |
| +// ---- emissions and violations ---- |
| + |
| +type movesEmission struct { |
| + file string |
| + line int // where the call or literal starts |
| + endLine int |
| + fn string // the enclosing function |
| + kind string |
| + pattern string |
| + fragment bool // a piece of a sentence, matched by containment |
| + underived bool // the gate could not derive a sentence: it matches no row |
| +} |
| + |
| +func (e movesEmission) String() string { |
| + if e.underived { |
| + return fmt.Sprintf("%s:%d (%s) a value the gate could not derive to a sentence", e.file, e.line, e.kind) |
| + } |
| + return fmt.Sprintf("%s:%d (%s) %q", e.file, e.line, e.kind, strings.ReplaceAll(e.pattern, wild, "%")) |
| +} |
| + |
| +// informative reports whether a pattern says anything: at least three letters |
| +// outside the parts the code fills in (a bare "a " or "S2: " is a piece of |
| +// a sentence, not one). |
| +func informative(p string) bool { |
| + n := 0 |
| + for _, r := range strings.ReplaceAll(strings.ReplaceAll(p, wild, ""), fwdMark, "") { |
| + if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' { |
| + if n++; n == 3 { |
| + return true |
| + } |
| + } |
| + } |
| + return false |
| +} |
| + |
| +// movesViolation is a sink that does not take a sentence the gate can derive. |
| +type movesViolation struct { |
| + file string |
| + line int |
| + fn string |
| + kind string |
| + why string |
| +} |
| + |
| +func (v movesViolation) String() string { |
| + return fmt.Sprintf("%s:%d (%s in %s): %s", v.file, v.line, v.kind, v.fn, v.why) |
| +} |
| + |
| +// scanMovesFile returns the emissions of one file. |
| +func scanMovesFile(t *testing.T, file string) []movesEmission { |
| + t.Helper() |
| + return scanMovesFileWith(t, ".", file, false) |
| +} |
| + |
| +// scanMovesFileWith is scanMovesFile; noCalls reads calls to package |
| +// functions and field reads as wildcards, so a legacy function's sentence is |
| +// only the text written in that function. |
| +func scanMovesFileWith(t *testing.T, root, file string, noCalls bool) []movesEmission { |
| + t.Helper() |
| + ems, _ := scanMovesFileFull(t, root, file, noCalls) |
| + return ems |
| +} |
| + |
| +func mentions(e ast.Expr, locals map[string][]localAssign, params map[string]bool, out map[string]bool, depth int) { |
| + ast.Inspect(e, func(n ast.Node) bool { |
| + if id, ok := n.(*ast.Ident); ok { |
| + if params[id.Name] { |
| + out[id.Name] = true |
| + } |
| + if depth < 3 { |
| + for _, a := range locals[id.Name] { |
| + if a.rhs != nil { |
| + mentions(a.rhs, locals, params, out, depth+1) |
| + } |
| + if a.call != nil { |
| + mentions(a.call, locals, params, out, depth+1) |
| + } |
| + } |
| + } |
| + } |
| + return true |
| + }) |
| +} |
| + |
| +// scanMovesFileFull returns a file's emissions and its sink violations. |
| +func scanMovesFileFull(t *testing.T, root, file string, noCalls bool) ([]movesEmission, []movesViolation) { |
| + t.Helper() |
| + return scanMovesFileIn(t, universeFor(t, root, file), root, file, noCalls) |
| +} |
| + |
| +// scanMovesFileIn scans a file in the given universe of struct fields. |
| +func scanMovesFileIn(t *testing.T, u *movesUniverse, root, file string, noCalls bool) ([]movesEmission, []movesViolation) { |
| + t.Helper() |
| + at := func(p string) string { |
| + if root == "." { |
| + return p |
| + } |
| + return filepath.Join(root, p) |
| + } |
| + fset := movesFset |
| + f, err := parser.ParseFile(fset, at(file), nil, 0) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + var out []movesEmission |
| + var vios []movesViolation |
| + seenVio := map[string]bool{} |
| + var fn string |
| + var ctx *evalCtx |
| + violate := func(line int, kind, why string) { |
| + v := movesViolation{file, line, fn, kind, why} |
| + if !seenVio[v.String()] { |
| + seenVio[v.String()] = true |
| + vios = append(vios, v) |
| + } |
| + } |
| + emit := func(pos token.Pos, kind string, e ast.Expr, fragment bool) { |
| + line := fset.Position(pos).Line |
| + end := fset.Position(e.End()).Line |
| + alts, probs := ctx.eval(e, 0) |
| + if ctx.fd != nil { |
| + params := map[string]bool{} |
| + if ctx.fd.Type.Params != nil { |
| + for _, f := range ctx.fd.Type.Params.List { |
| + if id, ok := f.Type.(*ast.Ident); ok && (id.Name == "string" || id.Name == "error") { |
| + for _, n := range f.Names { |
| + params[n.Name] = true |
| + } |
| + } |
| + } |
| + } |
| + hit := map[string]bool{} |
| + mentions(e, ctx.locals, params, hit, 0) |
| + if len(hit) > 0 { |
| + u.paramSinks[ctx.fd.Name.Name] = true |
| + } |
| + } |
| + underived := len(probs) > 0 |
| + for _, p := range probs { |
| + violate(line, kind, p.msg) |
| + } |
| + for _, a := range dedupe(alts) { |
| + switch { |
| + case a == "": |
| + case strings.Contains(a, fwdMark) && !informative(a): |
| + // forwarded text with no sentence of its own around it: checked where it is made |
| + case informative(a): |
| + out = append(out, movesEmission{file, line, end, fn, kind, strings.ReplaceAll(a, fwdMark, wild), fragment, false}) |
| + default: |
| + underived = true |
| + if !fragment { |
| + violate(line, kind, fmt.Sprintf("a value with no text of its own reaches this sink (it derives only to %q)", strings.ReplaceAll(a, wild, "%"))) |
| + } |
| + } |
| + } |
| + if underived && !fragment { |
| + out = append(out, movesEmission{file, line, end, fn, kind, "", false, true}) |
| + } |
| + } |
| + isCheck := func(x ast.Expr) bool { |
| + switch x := x.(type) { |
| + case *ast.Ident: |
| + return x.Name == "Check" |
| + case *ast.SelectorExpr: |
| + return x.Sel.Name == "Check" |
| + } |
| + return false |
| + } |
| + tables := map[string]bool{} |
| + for _, n := range movesCopyTables[file] { |
| + tables[n] = true |
| + } |
| + literals := map[string]bool{} |
| + for _, n := range movesLiteralFuncs[file] { |
| + literals[n] = true |
| + } |
| + for _, d := range f.Decls { |
| + fd, ok := d.(*ast.FuncDecl) |
| + if !ok || fd.Body == nil { |
| + continue |
| + } |
| + fn = fd.Name.Name |
| + ctx = u.newCtx(file, fd, noCalls) |
| + inTable := tables[fn] |
| + strs := stringResults(fd) |
| + named := namedResults(fd) |
| + returnsSentence := movesReturnedSentences[file][fn] && len(strs) > 0 |
| + var stack []ast.Node |
| + litDepth := 0 |
| + ast.Inspect(fd.Body, func(n ast.Node) bool { |
| + if n == nil { |
| + if _, ok := stack[len(stack)-1].(*ast.FuncLit); ok { |
| + litDepth-- |
| + } |
| + stack = stack[:len(stack)-1] |
| + return true |
| + } |
| + stack = append(stack, n) |
| + switch n := n.(type) { |
| + case *ast.FuncLit: |
| + litDepth++ |
| + case *ast.BasicLit: |
| + // A literal with an em dash is main's wording, which phase1Conflict |
| + // matches and rewrites; it is never spoken. A literal here is a |
| + // piece of a sentence unless it is compared whole or returned. |
| + if literals[fn] && n.Kind == token.STRING && !strings.Contains(n.Value, `\u2014`) && !strings.ContainsRune(n.Value, '\u2014') { |
| + emit(n.Pos(), fn, n, true) |
| + } |
| + case *ast.CallExpr: |
| + name := "" |
| + switch f := n.Fun.(type) { |
| + case *ast.Ident: |
| + name = f.Name |
| + case *ast.SelectorExpr: |
| + name = f.Sel.Name // a method: only local sinks, below |
| + } |
| + i, sink := movesLocalSinks[file][name] |
| + if id, ok := n.Fun.(*ast.Ident); ok && id.Name == "jsonError" { |
| + i, sink = 2, true |
| + } |
| + if sink && i < len(n.Args) { |
| + emit(n.Pos(), name, n.Args[i], false) |
| + } |
| + if j, fsink := movesFuncSinks[file][name]; fsink && j < len(n.Args) { |
| + if why := ctx.checkFuncArg(n.Args[j]); why != "" { |
| + violate(fset.Position(n.Pos()).Line, name, why) |
| + } |
| + } |
| + case *ast.CompositeLit: |
| + if n.Type != nil && isCheck(n.Type) { |
| + for _, el := range n.Elts { |
| + if kv, ok := el.(*ast.KeyValueExpr); ok { |
| + if k, ok := kv.Key.(*ast.Ident); ok && k.Name == "Reason" { |
| + emit(kv.Pos(), "Check.Reason", kv.Value, false) |
| + } |
| + } |
| + } |
| + } |
| + if inTable { |
| + for _, el := range n.Elts { |
| + if kv, ok := el.(*ast.KeyValueExpr); ok { |
| + switch kv.Value.(type) { |
| + case *ast.FuncLit, *ast.CompositeLit: |
| + // closures are read through their returns; nested tables by descent |
| + default: |
| + emit(kv.Pos(), fn, kv.Value, false) |
| + } |
| + } |
| + } |
| + } |
| + case *ast.AssignStmt: |
| + if len(n.Lhs) == 1 && len(n.Rhs) == 1 { |
| + if s, ok := n.Lhs[0].(*ast.SelectorExpr); ok && s.Sel.Name == "Reason" && isCheck(s.X) { |
| + emit(n.Pos(), "Check.Reason", n.Rhs[0], false) |
| + } |
| + } |
| + case *ast.ReturnStmt: |
| + ctx.curReturn = n |
| + switch { |
| + case inTable: |
| + for _, r := range n.Results { |
| + if _, nested := r.(*ast.CompositeLit); !nested { |
| + emit(r.Pos(), fn, r, false) |
| + } |
| + } |
| + case literals[fn]: |
| + // phase1Conflict's returns are sentences too. |
| + for _, r := range n.Results { |
| + emit(r.Pos(), fn, r, false) |
| + } |
| + case returnsSentence && litDepth == 0: |
| + if len(n.Results) == 0 { |
| + for i, nm := range named { |
| + if strs[i] { |
| + emit(n.Pos(), "returned sentence", &ast.Ident{Name: nm, NamePos: n.Pos()}, false) |
| + } |
| + } |
| + } |
| + for i, r := range n.Results { |
| + if strs[i] { |
| + emit(r.Pos(), "returned sentence", r, false) |
| + } |
| + } |
| + } |
| + ctx.curReturn = nil |
| + } |
| + return true |
| + }) |
| + } |
| + return out, vios |
| +} |
| + |
| +// checkFuncArg checks the function a call hands to a sentence-function |
| +// parameter: a package function that is a sentence helper. |
| +func (c *evalCtx) checkFuncArg(e ast.Expr) string { |
| + id, ok := e.(*ast.Ident) |
| + if !ok { |
| + return fmt.Sprintf("%s is passed where a sentence helper is expected", render(e)) |
| + } |
| + d, isFn := c.pkg.funcs[id.Name] |
| + if !isFn { |
| + return fmt.Sprintf("%s is passed where a sentence helper is expected", render(e)) |
| + } |
| + call := &ast.CallExpr{Fun: id, Args: nil} |
| + _, probs := c.callDecl(d, call, 0, 0) |
| + for _, p := range probs { |
| + return p.msg |
| + } |
| + return "" |
| +} |
| + |
| +// ---- function values ---- |
| + |
| +// movesFuncValueAllow lists functions that take a sentence parameter and are |
| +// referenced other than as a direct call, each with the reason that is safe. |
| +// The file is where the reference is. |
| +var movesFuncValueAllow = map[string]map[string]string{ |
| + "internal/console/apiinstancemove.go": { |
| + "phase1Conflict": "handed only to moveTransitionSaying's conflict parameter; movesFuncSinks makes the gate read every return of it at that call site, and its err parameter only selects among recorded sentences", |
| + }, |
| +} |
| + |
| +type funcRef struct { |
| + file string |
| + name string |
| + msg string |
| +} |
| + |
| +// funcValueRefs finds references to sentence-flow functions that are not |
| +// direct calls: `f := send`, a method value, a function passed as an |
| +// argument. A call through such a value escapes the argument checks. |
| +func funcValueRefs(t *testing.T, u *movesUniverse) []funcRef { |
| + t.Helper() |
| + flow := map[string]bool{} |
| + for n := range u.used { |
| + flow[n] = true |
| + } |
| + for n := range u.paramSinks { |
| + flow[n] = true |
| + } |
| + // Only names that are declared functions or methods of the scanned files. |
| + declared := map[string]bool{} |
| + var out []funcRef |
| + for _, f := range u.files { |
| + af, err := parser.ParseFile(movesFset, u.at(f), nil, 0) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, d := range af.Decls { |
| + fd, ok := d.(*ast.FuncDecl) |
| + if !ok { |
| + continue |
| + } |
| + declared[fd.Name.Name] = true |
| + // The forwarders configured as sinks are sentence-flow functions too. |
| + if _, ok := movesLocalSinks[f][fd.Name.Name]; ok { |
| + flow[fd.Name.Name] = true |
| + } |
| + if _, ok := movesFuncSinks[f][fd.Name.Name]; ok { |
| + flow[fd.Name.Name] = true |
| + } |
| + // Main's own functions are not the series' concern. |
| + if _, ok := movesLegacyFiles[f]; ok { |
| + delete(flow, fd.Name.Name) |
| + } else if _, ok := movesLegacyFuncs[f][fd.Name.Name]; ok { |
| + delete(flow, fd.Name.Name) |
| + } |
| + } |
| + } |
| + for _, f := range u.files { |
| + af, err := parser.ParseFile(movesFset, u.at(f), nil, 0) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + 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.FuncDecl: |
| + skip[n.Name] = true |
| + case *ast.SelectorExpr: |
| + skip[n.Sel] = true |
| + if flow[n.Sel.Name] && declared[n.Sel.Name] && !callFun[n] { |
| + out = append(out, funcRef{f, n.Sel.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()), render(n))}) |
| + } |
| + case *ast.KeyValueExpr: |
| + if id, ok := n.Key.(*ast.Ident); ok { |
| + skip[id] = true |
| + } |
| + case *ast.Ident: |
| + if skip[n] || callFun[n] { |
| + return true |
| + } |
| + if flow[n.Name] && declared[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)}) |
| + } |
| + } |
| + return true |
| + }) |
| + } |
| + sort.Slice(out, func(i, j int) bool { return out[i].msg < out[j].msg }) |
| + return out |
| +} |
diff --git a/approved_moves_copy_test.go b/approved_moves_copy_test.go| index cbd54f492..c104360e8 100644 |
| --- a/approved_moves_copy_test.go |
| +++ b/approved_moves_copy_test.go |
| @@ -234,18 +234,13 @@ func movesTextApplied(file, text string, body []byte) bool { |
| if err != nil { |
| return false |
| } |
| - idx := &movesPkgIndex{vars: map[string]ast.Expr{}, funcs: map[string]*ast.FuncDecl{}} |
| + vars := map[string]ast.Expr{} |
| for _, d := range f.Decls { |
| - switch d := d.(type) { |
| - case *ast.FuncDecl: |
| - if d.Recv == nil { |
| - idx.funcs[d.Name.Name] = d |
| - } |
| - case *ast.GenDecl: |
| - for _, sp := range d.Specs { |
| + if g, ok := d.(*ast.GenDecl); ok { |
| + for _, sp := range g.Specs { |
| if vs, ok := sp.(*ast.ValueSpec); ok && len(vs.Names) == len(vs.Values) { |
| for i, nm := range vs.Names { |
| - idx.vars[nm.Name] = vs.Values[i] |
| + vars[nm.Name] = vs.Values[i] |
| } |
| } |
| } |
| @@ -274,7 +269,7 @@ func movesTextApplied(file, text string, body []byte) bool { |
| if e == nil { |
| return true |
| } |
| - for _, p := range patternsOf(e, idx, 0) { |
| + for _, p := range literalPatterns(e, vars, 0) { |
| if informative(p) && movesRowSpoken(text, p) { |
| found = true |
| } |
| @@ -675,505 +670,6 @@ func formatPattern(f string) string { |
| }) |
| } |
| -// movesPkgIndex is one directory's package-level strings and functions, so a |
| -// Reason that names a constant or a one-line helper is read through. |
| -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{} |
| - |
| -// movesFset positions every file the scan parses, so a violation inside a |
| -// 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 := newPkgIndex() |
| - movesPkgCache[dir] = p |
| - ents, err := os.ReadDir(dir) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - for _, e := range ents { |
| - n := e.Name() |
| - if e.IsDir() || !strings.HasSuffix(n, ".go") || strings.HasSuffix(n, "_test.go") { |
| - continue |
| - } |
| - f, err := parser.ParseFile(movesFset, filepath.Join(dir, n), nil, 0) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - 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 |
| - } |
| - 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 nil |
| -} |
| - |
| -// patternsOf returns the sentence patterns an expression can produce: |
| -// 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 { |
| - if v, err := strconv.Unquote(e.Value); err == nil { |
| - return []string{v} |
| - } |
| - } |
| - case *ast.ParenExpr: |
| - return patternsOf(e.X, pkg, depth) |
| - case *ast.Ident: |
| - 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: |
| - if e.Op != token.ADD { |
| - break |
| - } |
| - var out []string |
| - for _, l := range patternsOf(e.X, pkg, depth) { |
| - for _, r := range patternsOf(e.Y, pkg, depth) { |
| - out = append(out, l+r) |
| - } |
| - } |
| - return out |
| - case *ast.CallExpr: |
| - if isFormatCall(e) { |
| - var out []string |
| - for _, f := range patternsOf(e.Args[0], pkg, depth) { |
| - out = append(out, formatPattern(f)) |
| - } |
| - 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 |
| -} |
| - |
| -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 n |
| -} |
| - |
| -func returnsOneString(d *ast.FuncDecl) bool { |
| - r := d.Type.Results |
| - if r == nil || len(r.List) != 1 || len(r.List[0].Names) > 1 { |
| - return false |
| - } |
| - id, ok := r.List[0].Type.(*ast.Ident) |
| - return ok && id.Name == "string" |
| -} |
| - |
| -type movesEmission struct { |
| - file string |
| - line int // where the call or literal starts |
| - endLine int |
| - fn string // the enclosing function |
| - kind string |
| - pattern string |
| - fragment bool // a piece of a sentence, matched by containment |
| -} |
| - |
| -func (e movesEmission) String() string { |
| - return fmt.Sprintf("%s:%d (%s) %q", e.file, e.line, e.kind, strings.ReplaceAll(e.pattern, wild, "%")) |
| -} |
| - |
| -// informative reports whether a pattern says anything: at least three letters |
| -// outside the parts the code fills in (a bare "a " or "S2: " is a piece of |
| -// a sentence, not one). |
| -func informative(p string) bool { |
| - n := 0 |
| - for _, r := range strings.ReplaceAll(p, wild, "") { |
| - if r >= 'a' && r <= 'z' || r >= 'A' && r <= 'Z' { |
| - if n++; n == 3 { |
| - return true |
| - } |
| - } |
| - } |
| - return false |
| -} |
| - |
| -// scanMovesFile returns the emissions of one file. |
| -func scanMovesFile(t *testing.T, file string) []movesEmission { |
| - t.Helper() |
| - return scanMovesFileWith(t, ".", file, false) |
| -} |
| - |
| -// scanMovesFileWith is scanMovesFile; noCalls reads calls to package |
| -// functions as wildcards, so a legacy function's sentence is only the text |
| -// written in that function. |
| -func scanMovesFileWith(t *testing.T, root, file string, noCalls bool) []movesEmission { |
| - t.Helper() |
| - ems, _ := scanMovesFileFull(t, root, file, noCalls) |
| - return ems |
| -} |
| - |
| -// movesViolation is a sink that does not take a sentence the gate can read. |
| -type movesViolation struct { |
| - file string |
| - line int |
| - fn string |
| - kind string |
| - why string |
| -} |
| - |
| -func (v movesViolation) String() string { |
| - return fmt.Sprintf("%s:%d (%s in %s): %s", v.file, v.line, v.kind, v.fn, v.why) |
| -} |
| - |
| -// scanMovesFileFull returns a file's emissions and its sink violations. |
| -func scanMovesFileFull(t *testing.T, root, file string, noCalls bool) ([]movesEmission, []movesViolation) { |
| - t.Helper() |
| - at := func(p string) string { |
| - if root == "." { |
| - return p |
| - } |
| - return filepath.Join(root, p) |
| - } |
| - fset := movesFset |
| - f, err := parser.ParseFile(fset, at(file), nil, 0) |
| - if err != nil { |
| - t.Fatal(err) |
| - } |
| - idx := *loadMovesPkg(t, at(filepath.Dir(file))) |
| - idx.noCalls = noCalls |
| - pkg := &idx |
| - var out []movesEmission |
| - var vios []movesViolation |
| - 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, cur, 0) { |
| - if informative(p) { |
| - out = append(out, movesEmission{file, fset.Position(pos).Line, fset.Position(e.End()).Line, fn, kind, p, fragment}) |
| - } |
| - } |
| - if fragment { |
| - return |
| - } |
| - if why := ctx.check(e, 0); why != "" { |
| - v := movesViolation{file, fset.Position(pos).Line, fn, kind, why} |
| - if !seenVio[v.String()] { |
| - seenVio[v.String()] = true |
| - vios = append(vios, v) |
| - } |
| - } |
| - } |
| - isCheck := func(x ast.Expr) bool { |
| - switch x := x.(type) { |
| - case *ast.Ident: |
| - return x.Name == "Check" |
| - case *ast.SelectorExpr: |
| - return x.Sel.Name == "Check" |
| - } |
| - return false |
| - } |
| - tables := map[string]bool{} |
| - for _, n := range movesCopyTables[file] { |
| - tables[n] = true |
| - } |
| - literals := map[string]bool{} |
| - for _, n := range movesLiteralFuncs[file] { |
| - literals[n] = true |
| - } |
| - for _, d := range f.Decls { |
| - fd, ok := d.(*ast.FuncDecl) |
| - if !ok || fd.Body == nil { |
| - continue |
| - } |
| - fn = fd.Name.Name |
| - 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 |
| - var stack []ast.Node |
| - litDepth := 0 |
| - ast.Inspect(fd.Body, func(n ast.Node) bool { |
| - if n == nil { |
| - if _, ok := stack[len(stack)-1].(*ast.FuncLit); ok { |
| - litDepth-- |
| - } |
| - stack = stack[:len(stack)-1] |
| - return true |
| - } |
| - stack = append(stack, n) |
| - switch n := n.(type) { |
| - case *ast.FuncLit: |
| - litDepth++ |
| - case *ast.BasicLit: |
| - // A literal with an em dash is main's wording, which phase1Conflict |
| - // matches and rewrites; it is never spoken. A literal here is a |
| - // piece of a sentence unless it is compared whole or returned. |
| - if literals[fn] && n.Kind == token.STRING && !strings.Contains(n.Value, `\u2014`) && !strings.ContainsRune(n.Value, '\u2014') { |
| - emit(n.Pos(), fn, n, true) |
| - } |
| - case *ast.CallExpr: |
| - name := "" |
| - switch fn := n.Fun.(type) { |
| - case *ast.Ident: |
| - name = fn.Name |
| - case *ast.SelectorExpr: |
| - name = fn.Sel.Name // a method: only local sinks, below |
| - } |
| - i, sink := movesLocalSinks[file][name] |
| - if id, ok := n.Fun.(*ast.Ident); ok && id.Name == "jsonError" { |
| - i, sink = 2, true |
| - } |
| - if sink && i < len(n.Args) { |
| - emit(n.Pos(), name, n.Args[i], false) |
| - } |
| - if j, fsink := movesFuncSinks[file][name]; fsink && j < len(n.Args) { |
| - if why := ctx.checkFuncArg(n.Args[j]); why != "" { |
| - v := movesViolation{file, fset.Position(n.Pos()).Line, fn, name, why} |
| - if !seenVio[v.String()] { |
| - seenVio[v.String()] = true |
| - vios = append(vios, v) |
| - } |
| - } |
| - } |
| - case *ast.CompositeLit: |
| - if n.Type != nil && isCheck(n.Type) { |
| - for _, el := range n.Elts { |
| - if kv, ok := el.(*ast.KeyValueExpr); ok { |
| - if k, ok := kv.Key.(*ast.Ident); ok && k.Name == "Reason" { |
| - emit(kv.Pos(), "Check.Reason", kv.Value, false) |
| - } |
| - } |
| - } |
| - } |
| - if inTable { |
| - for _, el := range n.Elts { |
| - if kv, ok := el.(*ast.KeyValueExpr); ok { |
| - switch kv.Value.(type) { |
| - case *ast.FuncLit, *ast.CompositeLit: |
| - // closures are read through their returns; nested tables by descent |
| - default: |
| - emit(kv.Pos(), fn, kv.Value, false) |
| - } |
| - } |
| - } |
| - } |
| - case *ast.AssignStmt: |
| - if len(n.Lhs) == 1 && len(n.Rhs) == 1 { |
| - if s, ok := n.Lhs[0].(*ast.SelectorExpr); ok && s.Sel.Name == "Reason" && isCheck(s.X) { |
| - emit(n.Pos(), "Check.Reason", n.Rhs[0], false) |
| - } |
| - } |
| - case *ast.ReturnStmt: |
| - ctx.curReturn = n |
| - switch { |
| - case inTable: |
| - for _, r := range n.Results { |
| - if _, nested := r.(*ast.CompositeLit); !nested { |
| - emit(r.Pos(), fn, r, false) |
| - } |
| - } |
| - case literals[fn]: |
| - // phase1Conflict's returns are sentences too. |
| - for _, r := range n.Results { |
| - emit(r.Pos(), fn, r, false) |
| - } |
| - case returnsSentence && litDepth == 0: |
| - for i, r := range n.Results { |
| - if strs[i] { |
| - emit(r.Pos(), "returned sentence", r, false) |
| - } |
| - } |
| - } |
| - } |
| - return true |
| - }) |
| - } |
| - return out, vios |
| -} |
| - |
| // ---- sinks must take sentences the gate can read ---- |
| // movesFuncSinks are calls that take a sentence-producing function as an |
| @@ -1199,476 +695,6 @@ 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 |
| - 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 { |
| - var out []string |
| - if ft.Params == nil { |
| - return out |
| - } |
| - for _, f := range ft.Params.List { |
| - if len(f.Names) == 0 { |
| - out = append(out, "_") |
| - } |
| - for _, n := range f.Names { |
| - out = append(out, n.Name) |
| - } |
| - } |
| - return out |
| -} |
| - |
| -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{}, visiting: map[string]bool{}} |
| - if fd == nil { |
| - return c |
| - } |
| - if i, ok := movesLocalSinks[file][fd.Name.Name]; ok { |
| - if ps := paramNames(fd.Type); i < len(ps) { |
| - c.fwd[ps[i]] = true |
| - } |
| - } |
| - if i, ok := movesFuncSinks[file][fd.Name.Name]; ok { |
| - if ps := paramNames(fd.Type); i < len(ps) { |
| - c.fwdFn[ps[i]] = true |
| - } |
| - } |
| - // A closure that is a local sink (fail := func(why string)) forwards its parameter. |
| - ast.Inspect(fd.Body, func(n ast.Node) bool { |
| - as, ok := n.(*ast.AssignStmt) |
| - if !ok || len(as.Lhs) != 1 || len(as.Rhs) != 1 { |
| - return true |
| - } |
| - id, ok := as.Lhs[0].(*ast.Ident) |
| - lit, isLit := as.Rhs[0].(*ast.FuncLit) |
| - if !ok || !isLit { |
| - return true |
| - } |
| - if i, ok := movesLocalSinks[file][id.Name]; ok { |
| - if ps := paramNames(lit.Type); i < len(ps) { |
| - c.fwd[ps[i]] = true |
| - } |
| - } |
| - return true |
| - }) |
| - return c |
| -} |
| - |
| -// 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 |
| -} |
| - |
| -func (c *sinkCtx) pkgString(name string) (ast.Expr, bool) { |
| - if _, shadowed := c.pkg.locals[name]; shadowed { |
| - return nil, false |
| - } |
| - x, ok := c.pkg.vars[name] |
| - return x, ok |
| -} |
| - |
| -func isFormatCall(e *ast.CallExpr) bool { |
| - sel, ok := e.Fun.(*ast.SelectorExpr) |
| - if !ok || len(e.Args) == 0 { |
| - return false |
| - } |
| - id, ok := sel.X.(*ast.Ident) |
| - return ok && ((id.Name == "fmt" && (sel.Sel.Name == "Sprintf" || sel.Sel.Name == "Errorf")) || |
| - (id.Name == "errors" && sel.Sel.Name == "New")) |
| -} |
| - |
| -func (c *sinkCtx) formatIsLiteral(e *ast.CallExpr) bool { |
| - switch a := e.Args[0].(type) { |
| - case *ast.BasicLit: |
| - return a.Kind == token.STRING |
| - case *ast.Ident: |
| - _, ok := c.pkgString(a.Name) |
| - return ok |
| - } |
| - return false |
| -} |
| - |
| -// 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 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) && c.formatIsLiteral(e) |
| - case *ast.BinaryExpr: |
| - return e.Op == token.ADD && (c.literalish(e.X) || c.literalish(e.Y)) |
| - } |
| - return false |
| -} |
| - |
| -func addOperands(e ast.Expr, out *[]ast.Expr) { |
| - if b, ok := e.(*ast.BinaryExpr); ok && b.Op == token.ADD { |
| - addOperands(b.X, out) |
| - addOperands(b.Y, out) |
| - return |
| - } |
| - if p, ok := e.(*ast.ParenExpr); ok { |
| - addOperands(p.X, out) |
| - return |
| - } |
| - *out = append(*out, e) |
| -} |
| - |
| -func render(e ast.Expr) string { |
| - var b strings.Builder |
| - ast.Inspect(e, func(n ast.Node) bool { |
| - switch n := n.(type) { |
| - case *ast.BinaryExpr: |
| - b.WriteString(render(n.X) + " " + n.Op.String() + " " + render(n.Y)) |
| - return false |
| - case *ast.Ident: |
| - b.WriteString(n.Name) |
| - case *ast.SelectorExpr: |
| - b.WriteString(render(n.X) + "." + n.Sel.Name) |
| - return false |
| - case *ast.CallExpr: |
| - b.WriteString(render(n.Fun) + "(...)") |
| - return false |
| - } |
| - return true |
| - }) |
| - return b.String() |
| -} |
| - |
| -// check returns why e is not a sentence the gate can read, or "". |
| -// |
| -// 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 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) |
| - case *ast.BasicLit: |
| - if e.Kind == token.STRING { |
| - return "" |
| - } |
| - return "not a string literal" |
| - case *ast.Ident: |
| - return c.identProblem(e, depth) |
| - case *ast.SelectorExpr: |
| - if e.Sel.Name == "Reason" { |
| - return "" |
| - } |
| - return fmt.Sprintf("%s is a field the gate cannot read", render(e)) |
| - case *ast.BinaryExpr: |
| - if e.Op != token.ADD { |
| - return "an expression that is not a sentence" |
| - } |
| - var ops []ast.Expr |
| - addOperands(e, &ops) |
| - lit := false |
| - for _, o := range ops { |
| - if c.literalish(o) { |
| - lit = true |
| - if why := c.check(o, depth+1); why != "" { |
| - return why |
| - } |
| - } |
| - } |
| - if !lit { |
| - return fmt.Sprintf("%s is a concatenation with no literal part", render(e)) |
| - } |
| - return "" |
| - case *ast.CallExpr: |
| - if isFormatCall(e) { |
| - if !c.formatIsLiteral(e) { |
| - return fmt.Sprintf("%s has a format that is not a literal", render(e)) |
| - } |
| - 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 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 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 "" |
| -} |
| - |
| -// 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)) |
| - } |
| - 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) |
| - } |
| - key := fmt.Sprintf("helper %p %d", d, pos) |
| - if c.visiting[key] { |
| - return "" |
| - } |
| - 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 != "" { |
| - 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 == "" |
| - }) |
| - 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 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)) |
| -} |
| - |
| // diagnosticPrefixes are the ids the readiness code puts before a reason |
| // ("T11: ", "S2: ") and the record leaves on its row or off it. |
| var diagnosticPrefixes = []string{"T11: ", "S1: ", "S2: "} |
| @@ -1790,8 +816,6 @@ func f(w int, c Check) { |
| jsonError(w, 400, "bad region "+c.Region+" given") |
| _ = Check{Reason: noKey} |
| _ = moves.Check{Reason: why(3)} |
| - _ = Check{Reason: "-"} |
| - _ = Check{Reason: c.Other} |
| c.Check.Reason = fmt.Sprintf("took %d of %d", 1, 2) |
| } |
| ` |
| @@ -2009,28 +1033,6 @@ func TestMovesGateBitesOnTheRealRecord(t *testing.T) { |
| } |
| } |
| -// stringResults is the set of a function's result positions that are strings. |
| -func stringResults(d *ast.FuncDecl) map[int]bool { |
| - out := map[int]bool{} |
| - if d.Type.Results == nil { |
| - return out |
| - } |
| - i := 0 |
| - for _, f := range d.Type.Results.List { |
| - n := len(f.Names) |
| - if n == 0 { |
| - n = 1 |
| - } |
| - if id, ok := f.Type.(*ast.Ident); ok && id.Name == "string" { |
| - for k := 0; k < n; k++ { |
| - out[i+k] = true |
| - } |
| - } |
| - i += n |
| - } |
| - return out |
| -} |
| - |
| // movesViolationExempt reports whether a violation is main's own (legacy) or |
| // is allow-listed with a reason. |
| func movesViolationExempt(v movesViolation) bool { |
| @@ -2063,10 +1065,28 @@ func TestMovesSinksAreSentences(t *testing.T) { |
| // ---- negative fixtures for the sink rule, the matcher and the placeholder check ---- |
| -func sinkFixture(t *testing.T, dir string) []movesViolation { |
| +// fixtureScan scans a sink fixture in its own universe, and returns the |
| +// universe too, for the function-value check. |
| +func fixtureScan(t *testing.T, dir string) ([]movesEmission, []movesViolation, *movesUniverse) { |
| + t.Helper() |
| + file := filepath.Join("testdata", "moves-copy", "sinks", dir, "emit.go") |
| + u := newUniverse(t, ".", []string{file}) |
| + ems, vios := scanMovesFileIn(t, u, ".", file, false) |
| + return ems, vios, u |
| +} |
| + |
| +// fixtureRows is the fixture's record.md, or a record with one unrelated row. |
| +func fixtureRows(t *testing.T, dir string) []copyRow { |
| t.Helper() |
| - _, v := scanMovesFileFull(t, ".", filepath.Join("testdata", "moves-copy", "sinks", dir, "emit.go"), false) |
| - return v |
| + path := filepath.Join("testdata", "moves-copy", "sinks", dir, "record.md") |
| + if _, err := os.Stat(path); err == nil { |
| + rows, err := parseMovesCopyRecord(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + return rows |
| + } |
| + return []copyRow{{ID: "MV-N1", File: "x.go", Text: "an unrelated sentence", Status: "pending"}} |
| } |
| func wantViolations(t *testing.T, name string, got []movesViolation, wants ...string) { |
| @@ -2082,31 +1102,108 @@ func wantViolations(t *testing.T, name string, got []movesViolation, wants ...st |
| } |
| } |
| +// Each counterexample fails the sink gate, and, where the value is one the |
| +// record should have seen, fails the record gate too: an underived value matches |
| +// no row, and a substituted or named-result sentence is read as it is spoken. |
| func TestMovesSinkFixtures(t *testing.T) { |
| - wantViolations(t, "a forwarded error", sinkFixture(t, "forwarded-error"), |
| - "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 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 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", |
| - "emit.go:22 (jsonError in field): r.Message is a field the gate cannot read", |
| - ) |
| - wantViolations(t, "sentences the gate can read", sinkFixture(t, "ok")) |
| + cases := []struct { |
| + dir string |
| + vios []string // substrings, in order |
| + unrecorded int // emissions no row of the fixture's record covers |
| + }{ |
| + {"forwarded-error", []string{ |
| + "emit.go:7 (jsonError in refuse): err.Error(...) forwards an error's text", |
| + "emit.go:18 (Check.Reason in report): helper why, at ", |
| + }, 2}, |
| + {"sprintf-v", []string{ |
| + "emit.go:9 (jsonError in viaSprintf): a value with no text of its own", |
| + "emit.go:13 (jsonError in viaConcat): a value with no text of its own", |
| + "emit.go:17 (jsonError in viaVerbs): a value with no text of its own", |
| + }, 3}, |
| + {"field-reason", []string{ |
| + "emit.go:12 (jsonError in positional): a write to field Reason at ", |
| + "emit.go:12 (jsonError in positional): a write to field Reason at ", |
| + "emit.go:12 (jsonError in positional): a write to field Reason at ", |
| + "emit.go:17 (jsonError in keyed): a write to field Reason at ", |
| + "emit.go:17 (jsonError in keyed): a write to field Reason at ", |
| + "emit.go:17 (jsonError in keyed): a write to field Reason at ", |
| + "emit.go:22 (jsonError in assigned): a write to field Reason at ", |
| + }, 4}, |
| + {"local-variable", []string{ |
| + "emit.go:10 (jsonError in mixed): err.Error(...) forwards an error's text", |
| + "emit.go:15 (jsonError in fromMultiValue): helper reasonOf, at ", |
| + "emit.go:24 (jsonError in rangeVar): n is assigned a value the gate cannot read", |
| + }, 4}, |
| + {"helper-bad-return", []string{ |
| + "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): a value with no text of its own", |
| + }, 2}, |
| + {"substitution", nil, 1}, |
| + {"named-result", nil, 1}, |
| + {"shadow", []string{ |
| + "emit.go:12 (jsonError in refuse): say at ", |
| + }, 1}, |
| + {"interface", []string{ |
| + "emit.go:17 (jsonError in viaInterface): m.Message(...) at ", |
| + "emit.go:21 (jsonError in viaUnknown): x.Message(...)", |
| + }, 2}, |
| + {"funcvalue", []string{ |
| + "emit.go:8 (jsonError in send): parameter msg is not bound to an argument", |
| + "emit.go:23 (jsonError in report): parameter msg is not bound to an argument", |
| + }, 2}, |
| + {"limit", []string{ |
| + "emit.go:19 (jsonError in chainLiteral): analysis limit reached at ", |
| + "emit.go:35 (jsonError in chainError): analysis limit reached at ", |
| + }, 2}, |
| + {"unresolved", []string{ |
| + "emit.go:6 (jsonError in methodResult): s.render(...): no method render on server in this package", |
| + "emit.go:10 (jsonError in indexed): names is not a sentence the gate can read", |
| + "emit.go:14 (Check.Reason in bare): a value with no text of its own", |
| + "emit.go:18 (jsonError in dynamicFormat): fmt.Sprintf(...) has a format that is not a literal", |
| + "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}, |
| + {"ok", nil, 0}, |
| + } |
| + for _, c := range cases { |
| + ems, vios, _ := fixtureScan(t, c.dir) |
| + wantViolations(t, c.dir, vios, c.vios...) |
| + if un := findUnrecorded(ems, fixtureRows(t, c.dir)); len(un) != c.unrecorded { |
| + t.Errorf("%s: %d unrecorded %v, want %d", c.dir, len(un), un, c.unrecorded) |
| + } |
| + } |
| + // The sentences that are read as spoken. |
| + for dir, want := range map[string]string{"substitution": "an unrecorded sentence", "named-result": "an unrecorded sentence"} { |
| + ems, _, _ := fixtureScan(t, dir) |
| + if len(ems) != 1 || ems[0].pattern != want { |
| + t.Errorf("%s: emissions = %v, want exactly %q", dir, ems, want) |
| + } |
| + } |
| +} |
| + |
| +// A sentence-flow function referenced other than as a direct call lets a call |
| +// through the value escape the argument checks. |
| +func TestMovesFunctionValueFixture(t *testing.T) { |
| + _, _, u := fixtureScan(t, "funcvalue") |
| + var got []string |
| + for _, r := range funcValueRefs(t, u) { |
| + got = append(got, r.msg) |
| + } |
| + want := []string{"emit.go:16:7: send is used as a value", "emit.go:27:7: s.report is used as a value"} |
| + if len(got) != len(want) { |
| + t.Fatalf("refs = %q, want %d", got, len(want)) |
| + } |
| + for i, w := range want { |
| + if !strings.Contains(got[i], w) { |
| + t.Errorf("ref %d = %q, want one containing %q", i, got[i], w) |
| + } |
| + } |
| + _, _, ok := fixtureScan(t, "ok") |
| + if refs := funcValueRefs(t, ok); len(refs) != 0 { |
| + t.Errorf("the ok fixture: refs = %v", refs) |
| + } |
| } |
| // A mapping helper may echo a refusal's text only in a case whose guard tests |
| @@ -2221,3 +1318,81 @@ func TestMovesApprovedPlaceholderTextIsOneExpression(t *testing.T) { |
| t.Errorf("problems = %q", got) |
| } |
| } |
| + |
| +// literalPatterns reads the texts of an expression written in place: literals, |
| +// concatenations, Sprintf and errors.New formats, and consts of the same file. |
| +// What it cannot read is a wildcard. |
| +func literalPatterns(e ast.Expr, vars map[string]ast.Expr, depth int) []string { |
| + if depth > 6 { |
| + return []string{wild} |
| + } |
| + switch e := e.(type) { |
| + case *ast.BasicLit: |
| + if v, err := strconv.Unquote(e.Value); err == nil && e.Kind == token.STRING { |
| + return []string{v} |
| + } |
| + case *ast.ParenExpr: |
| + return literalPatterns(e.X, vars, depth) |
| + case *ast.Ident: |
| + if x, ok := vars[e.Name]; ok { |
| + return literalPatterns(x, vars, depth+1) |
| + } |
| + case *ast.BinaryExpr: |
| + if e.Op == token.ADD { |
| + var out []string |
| + for _, l := range literalPatterns(e.X, vars, depth) { |
| + for _, r := range literalPatterns(e.Y, vars, depth) { |
| + out = append(out, l+r) |
| + } |
| + } |
| + return out |
| + } |
| + case *ast.CallExpr: |
| + if isFormatCall(e) { |
| + var out []string |
| + for _, f := range literalPatterns(e.Args[0], vars, depth+1) { |
| + out = append(out, formatPattern(f)) |
| + } |
| + return out |
| + } |
| + } |
| + return []string{wild} |
| +} |
| + |
| +// A function that takes a sentence and is referenced other than as a direct |
| +// call (`f := send`, a method value, an argument) lets a call through the |
| +// value escape the argument checks. The real tree has none, bar the allow-list. |
| +func TestMovesFunctionValuesAreNotSinks(t *testing.T) { |
| + u := universeFor(t, ".", movesScanFiles[0]) |
| + for _, f := range movesScanFiles { |
| + scanMovesFileIn(t, u, ".", f, false) |
| + } |
| + refs := funcValueRefs(t, u) |
| + reportAll(t, checkFuncValueAllow(movesFuncValueAllow, refs)) |
| + for _, r := range refs { |
| + if _, ok := movesFuncValueAllow[r.file][r.name]; !ok { |
| + t.Error(r.msg) |
| + } |
| + } |
| +} |
| + |
| +// An allow-list entry needs a real reason and must still exempt a reference. |
| +func checkFuncValueAllow(allow map[string]map[string]string, refs []funcRef) []string { |
| + used := map[string]bool{} |
| + for _, r := range refs { |
| + used[r.file+" "+r.name] = true |
| + } |
| + var out []string |
| + for file, names := range allow { |
| + for name, why := range names { |
| + if len(strings.Fields(why)) < 8 { |
| + out = append(out, fmt.Sprintf("%s %s: the allow-list entry needs a real reason", file, name)) |
| + } |
| + if !used[file+" "+name] { |
| + out = append(out, fmt.Sprintf("%s %s: the allow-list entry exempts nothing; drop it", file, name)) |
| + } |
| + } |
| + } |
| + sort.Strings(out) |
| + return out |
| +} |
diff --git a/testdata/moves-copy/sinks/field-reason/emit.go b/testdata/moves-copy/sinks/field-reason/emit.go| new file mode 100644 |
| index 000000000..1e127527f |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/field-reason/emit.go |
| @@ -0,0 +1,23 @@ |
| +package fixture |
| + |
| +// Not compiled. A field is a sentence only if every write to a field of that |
| +// name is. The anonymous struct writes an error's text, positionally and by key. |
| + |
| +type Check struct{ Reason string } |
| + |
| +func good() Check { return Check{Reason: "this box has no secrets key"} } |
| + |
| +func positional(w int, err error) { |
| + box := struct{ Reason string }{err.Error()} |
| + jsonError(w, 400, box.Reason) |
| +} |
| + |
| +func keyed(w int, err error) { |
| + box := struct{ Reason string }{Reason: err.Error()} |
| + jsonError(w, 400, box.Reason) |
| +} |
| + |
| +func assigned(w int, err error, c Check) { |
| + c.Reason = err.Error() |
| + jsonError(w, 400, c.Reason) |
| +} |
diff --git a/testdata/moves-copy/sinks/funcvalue/emit.go b/testdata/moves-copy/sinks/funcvalue/emit.go| new file mode 100644 |
| index 000000000..6a7291859 |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/funcvalue/emit.go |
| @@ -0,0 +1,29 @@ |
| +package fixture |
| + |
| +// Not compiled. send's parameter reaches a sink. A direct call passes a |
| +// sentence, but a call through a function value passes an error's text and |
| +// escapes the argument checks. |
| + |
| +func send(w int, msg string) { |
| + jsonError(w, 400, msg) |
| +} |
| + |
| +func direct(w int) { |
| + send(w, "invalid region") |
| +} |
| + |
| +func indirect(w int, err error) { |
| + f := send |
| + f(w, err.Error()) |
| +} |
| + |
| +type server struct{} |
| + |
| +func (s server) report(w int, msg string) { |
| + jsonError(w, 400, msg) |
| +} |
| + |
| +func methodValue(w int, s server, err error) { |
| + g := s.report |
| + g(w, err.Error()) |
| +} |
diff --git a/testdata/moves-copy/sinks/interface/emit.go b/testdata/moves-copy/sinks/interface/emit.go| new file mode 100644 |
| index 000000000..543963e31 |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/interface/emit.go |
| @@ -0,0 +1,22 @@ |
| +package fixture |
| + |
| +// Not compiled. The interface's implementation may live in another package and |
| +// say anything. A method selected only by its name is no better. |
| + |
| +type messenger interface { |
| + Message() string |
| +} |
| + |
| +type local struct{} |
| + |
| +func (local) Message() string { |
| + return "a recorded sentence" |
| +} |
| + |
| +func viaInterface(w int, m messenger) { |
| + jsonError(w, 400, m.Message()) |
| +} |
| + |
| +func viaUnknown(w int, x external) { |
| + jsonError(w, 400, x.Message()) |
| +} |
diff --git a/testdata/moves-copy/sinks/limit/emit.go b/testdata/moves-copy/sinks/limit/emit.go| new file mode 100644 |
| index 000000000..dc117b1a5 |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/limit/emit.go |
| @@ -0,0 +1,36 @@ |
| +package fixture |
| + |
| +// Not compiled. A chain of local aliases longer than the gate reads. Running out |
| +// of depth is a violation, not an approval. |
| + |
| +func chainLiteral(w int) { |
| + 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 |
| + jsonError(w, 400, a12) |
| +} |
| + |
| +func chainError(w int, err error) { |
| + b1 := err.Error() |
| + b2 := b1 |
| + b3 := b2 |
| + b4 := b3 |
| + b5 := b4 |
| + b6 := b5 |
| + b7 := b6 |
| + b8 := b7 |
| + b9 := b8 |
| + b10 := b9 |
| + b11 := b10 |
| + b12 := b11 |
| + jsonError(w, 400, b12) |
| +} |
diff --git a/testdata/moves-copy/sinks/named-result/emit.go b/testdata/moves-copy/sinks/named-result/emit.go| new file mode 100644 |
| index 000000000..dbd571078 |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/named-result/emit.go |
| @@ -0,0 +1,12 @@ |
| +package fixture |
| + |
| +// Not compiled. A bare return speaks the named result. |
| + |
| +func refuse(w int) { |
| + jsonError(w, 400, say()) |
| +} |
| + |
| +func say() (msg string) { |
| + msg = "an unrecorded sentence" |
| + return |
| +} |
diff --git a/testdata/moves-copy/sinks/ok/emit.go b/testdata/moves-copy/sinks/ok/emit.go| index 928422261..c1bef0dc2 100644 |
| --- a/testdata/moves-copy/sinks/ok/emit.go |
| +++ b/testdata/moves-copy/sinks/ok/emit.go |
| @@ -31,10 +31,6 @@ func tooMany(n int) string { |
| return "one box is too few" |
| } |
| -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" |
| @@ -69,3 +65,14 @@ func wrapOK(m string) string { |
| func callsAreLiterals() string { |
| return wrapOK("second") |
| } |
| + |
| +// A field whose every write, in the struct that is read, is a sentence. |
| +type Check struct{ Reason string } |
| + |
| +func makeCheck() Check { |
| + return Check{Reason: "this box has no secrets key"} |
| +} |
| + |
| +func viaField(w int, c Check) { |
| + jsonError(w, 400, c.Reason) |
| +} |
diff --git a/testdata/moves-copy/sinks/ok/record.md b/testdata/moves-copy/sinks/ok/record.md| new file mode 100644 |
| index 000000000..07ec468f7 |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/ok/record.md |
| @@ -0,0 +1,15 @@ |
| +# The sentences of the ok fixture, as a record would carry them |
| + |
| +| id | file | text | status | |
| +|---|---|---|---| |
| +| MV-N1 | emit.go | invalid region | pending | |
| +| MV-N2 | emit.go | this box has no secrets key | pending | |
| +| MV-N3 | emit.go | region <to> is not armed | pending | |
| +| MV-N4 | emit.go | <n> boxes publish <region> | pending | |
| +| MV-N5 | emit.go | <n> boxes is too many | pending | |
| +| MV-N6 | emit.go | one box is too few | pending | |
| +| MV-N7 | emit.go | that region is not armed | pending | |
| +| MV-N8 | emit.go | that region is not hosting | pending | |
| +| 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 | |
diff --git a/testdata/moves-copy/sinks/shadow/emit.go b/testdata/moves-copy/sinks/shadow/emit.go| new file mode 100644 |
| index 000000000..882e189d8 |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/shadow/emit.go |
| @@ -0,0 +1,13 @@ |
| +package fixture |
| + |
| +// Not compiled. The closure shadows the package function of the same name, so |
| +// the recorded text of the package function is not what is spoken. |
| + |
| +func say() string { |
| + return "a recorded sentence" |
| +} |
| + |
| +func refuse(w int, err error) { |
| + say := func() string { return err.Error() } |
| + jsonError(w, 400, say()) |
| +} |
diff --git a/testdata/moves-copy/sinks/sprintf-v/emit.go b/testdata/moves-copy/sinks/sprintf-v/emit.go| new file mode 100644 |
| index 000000000..f03cd54d9 |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/sprintf-v/emit.go |
| @@ -0,0 +1,18 @@ |
| +package fixture |
| + |
| +import "fmt" |
| + |
| +// Not compiled. Both forward an error's text with no sentence of their own: |
| +// the pattern derives to a bare placeholder, which no row can vouch for. |
| + |
| +func viaSprintf(w int, err error) { |
| + jsonError(w, 400, fmt.Sprintf("%v", err)) |
| +} |
| + |
| +func viaConcat(w int, err error) { |
| + jsonError(w, 400, ""+err.Error()) |
| +} |
| + |
| +func viaVerbs(w int, err error) { |
| + jsonError(w, 400, fmt.Sprintf("%s%s", "", err)) |
| +} |
diff --git a/testdata/moves-copy/sinks/substitution/emit.go b/testdata/moves-copy/sinks/substitution/emit.go| new file mode 100644 |
| index 000000000..67c7cea37 |
| --- /dev/null |
| +++ b/testdata/moves-copy/sinks/substitution/emit.go |
| @@ -0,0 +1,12 @@ |
| +package fixture |
| + |
| +// Not compiled. The parameter is bound to the argument, so the sentence is |
| +// read as it is spoken: "an unrecorded sentence". |
| + |
| +func refuse(w int) { |
| + jsonError(w, 400, wrap("an unrecorded sentence")) |
| +} |
| + |
| +func wrap(m string) string { |
| + return m |
| +} |