Skip to content

Commit

Permalink
starlark: add simple check for misspelled attributes
Browse files Browse the repository at this point in the history
Implementations of SetField may return ErrNoSuchField to avail
themselves of a better error message.

Fixes google#135

Change-Id: I5941890b59947e0fc64e2ba932d8876fe5a9c97b
  • Loading branch information
adonovan committed Feb 5, 2019
1 parent c1a3d54 commit 557c1f1
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 4 deletions.
2 changes: 2 additions & 0 deletions resolve/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,8 @@ func (r *resolver) useGlobal(id *syntax.Ident) binding {
}
} else {
scope = Undefined
// TODO(adonovan): implement spell check. Ideally we
// need a way to enumerate universal and predeclared.
r.errorf(id.NamePos, "undefined: %s", id.Name)
}
id.Scope = uint8(scope)
Expand Down
22 changes: 19 additions & 3 deletions starlark/eval.go
Original file line number Diff line number Diff line change
Expand Up @@ -452,22 +452,38 @@ func listExtend(x *List, y Iterable) {

// getAttr implements x.dot.
func getAttr(x Value, name string) (Value, error) {
var hint string

// field or method?
if x, ok := x.(HasAttrs); ok {
if v, err := x.Attr(name); v != nil || err != nil {
return v, err
}

// check spelling
if n := nearest(name, x.AttrNames()); n != "" {
hint = fmt.Sprintf(" (did you mean .%s?)", n)
}
}

return nil, fmt.Errorf("%s has no .%s field or method", x.Type(), name)
return nil, fmt.Errorf("%s has no .%s field or method%s", x.Type(), name, hint)
}

// setField implements x.name = y.
func setField(x Value, name string, y Value) error {
if x, ok := x.(HasSetField); ok {
err := x.SetField(name, y)
return err
if err := x.SetField(name, y); err != ErrNoSuchField {
return err
}

// No such field: check spelling.
var hint string
if n := nearest(name, x.AttrNames()); n != "" {
hint = fmt.Sprintf(" (did you mean .%s?)", n)
}
return fmt.Errorf("%s has no .%s field%s", x.Type(), name, hint)
}

return fmt.Errorf("can't assign to .%s field of %s", name, x.Type())
}

Expand Down
10 changes: 9 additions & 1 deletion starlark/eval_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"math"
"path/filepath"
"sort"
"strings"
"testing"

Expand Down Expand Up @@ -184,7 +185,10 @@ func load(thread *starlark.Thread, module string) (starlark.StringDict, error) {
return starlark.ExecFile(thread, filename, nil, nil)
}

func newHasFields(thread *starlark.Thread, _ *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
func newHasFields(thread *starlark.Thread, b *starlark.Builtin, args starlark.Tuple, kwargs []starlark.Tuple) (starlark.Value, error) {
if len(args)+len(kwargs) > 0 {
return nil, fmt.Errorf("%s: unexpected arguments", b.Name())
}
return &hasfields{attrs: make(map[string]starlark.Value)}, nil
}

Expand Down Expand Up @@ -222,6 +226,9 @@ func (hf *hasfields) SetField(name string, val starlark.Value) error {
if hf.frozen {
return fmt.Errorf("cannot set field on a frozen hasfields")
}
if strings.HasPrefix(name, "no") {
return starlark.ErrNoSuchField // for testing
}
hf.attrs[name] = val
return nil
}
Expand All @@ -231,6 +238,7 @@ func (hf *hasfields) AttrNames() []string {
for key := range hf.attrs {
names = append(names, key)
}
sort.Strings(names)
return names
}

Expand Down
95 changes: 95 additions & 0 deletions starlark/spell.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package starlark

// This file defines a simple spell checker for use in attribute errors
// ("no such field .foo; did you mean .food?")

import (
"strings"
"unicode"
)

// nearest returns the element of candidates
// nearest to x using the Levenshtein metric.
func nearest(x string, candidates []string) string {
// Ignore underscores and case when matching.
fold := func(s string) string {
return strings.Map(func(r rune) rune {
if r == '_' {
return -1
}
return unicode.ToLower(r)
}, s)
}

x = fold(x)

var best string
bestD := (len(x) + 1) / 2 // allow up to 50% typos
for _, c := range candidates {
d := levenshtein(x, fold(c), bestD)
if d < bestD {
bestD = d
best = c
}
}
return best
}

// levenshtein returns the non-negative Levenshtein edit distance
// between the byte strings x and y.
//
// If the computed distance exceeds max,
// the function may return early with an approximate value > max.
func levenshtein(x, y string, max int) int {
// This implementation is derived from one by Laurent Le Brun in
// Bazel that uses the single-row space efficiency trick
// described at bitbucket.org/clearer/iosifovich.

// Let x be the shorter string.
if len(x) > len(y) {
x, y = y, x
}

// Remove common prefix.
for i := 0; i < len(x); i++ {
if x[i] != y[i] {
x = x[i:]
y = y[i:]
break
}
}
if x == "" {
return len(y)
}

row := make([]int, len(y)+1)
for i := range row {
row[i] = i
}

for i := 1; i <= len(x); i++ {
row[0] = i
best := i
prev := i - 1
for j := 1; j <= len(y); j++ {
a := prev + b2i(x[i-1] != y[j-1]) // substitution
b := 1 + row[j-1] // deletion
c := 1 + row[j] // insertion
k := min(a, min(b, c))
prev, row[j] = row[j], k
best = min(best, k)
}
if best > max {
return best
}
}
return row[len(y)]
}

func min(x, y int) int {
if x < y {
return x
} else {
return y
}
}
15 changes: 15 additions & 0 deletions starlark/testdata/builtins.star
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,21 @@ assert.eq(str(getattr(myset, "union")), "<built-in method union of set value>")
assert.fails(lambda: getattr(myset, "onion"), "no .onion field or method")
assert.eq(getattr(myset, "onion", 42), 42)

# error messages should suggest spelling corrections
hf.one = 1
hf.two = 2
hf.three = 3
hf.forty_five = 45
assert.fails(lambda: hf.One, 'no .One field.*did you mean .one')
assert.fails(lambda: hf.oone, 'no .oone field.*did you mean .one')
assert.fails(lambda: hf.FortyFive, 'no .FortyFive field.*did you mean .forty_five')
assert.fails(lambda: hf.trhee, 'no .trhee field.*did you mean .three')
assert.fails(lambda: hf.thirty, 'no .thirty field or method$') # no suggestion

# spell check in setfield too
def setfield(): hf.noForty_Five = 46 # "no" prefix => SetField returns NoSuchField
assert.fails(setfield, 'no .noForty_Five field.*did you mean .forty_five')

# repr
assert.eq(repr(1), "1")
assert.eq(repr("x"), '"x"')
Expand Down
5 changes: 5 additions & 0 deletions starlark/testdata/string.star
Original file line number Diff line number Diff line change
Expand Up @@ -401,3 +401,8 @@ assert.eq("¿Por qué?".title(), "¿Por Qué?")
assert.eq("ljubović".title(), "Ljubović")
assert.true("Dženan Ljubović".istitle())
assert.true(not "DŽenan LJubović".istitle())

# method spell check
assert.fails(lambda: "".starts_with, "no .starts_with field.*did you mean .startswith")
assert.fails(lambda: "".StartsWith, "no .StartsWith field.*did you mean .startswith")
assert.fails(lambda: "".fin, "no .fin field.*.did you mean .find")
11 changes: 11 additions & 0 deletions starlark/value.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ package starlark // import "go.starlark.net/starlark"

import (
"bytes"
"errors"
"fmt"
"math"
"math/big"
Expand Down Expand Up @@ -319,11 +320,21 @@ var (
)

// A HasSetField value has fields that may be written by a dot expression (x.f = y).
//
// An implementation of SetField may return ErrNoSuchField,
// in which case the runtime will report a better error that
// includes the field name and warns of possible misspelling.
type HasSetField interface {
HasAttrs
SetField(name string, val Value) error
}

// ErrNoSuchField may be returned by an implementation of
// HasSetField.SetField to indicate that no such field exists.
// In that case the runtime will report a better error that
// includes the field name and warns of possible misspelling.
var ErrNoSuchField = errors.New("no such field")

// NoneType is the type of None. Its only legal value is None.
// (We represent it as a number, not struct{}, so that None may be constant.)
type NoneType byte
Expand Down

0 comments on commit 557c1f1

Please sign in to comment.