Skip to content

Commit

Permalink
Relax Transformer name rules
Browse files Browse the repository at this point in the history
Relax the Transformer name to be any Go identifier or qualified identifier.
Clarify in the documentation how this is used (e.g., in Diff) and
what names are valid.

Fixes #102
  • Loading branch information
dsnet committed Feb 17, 2019
1 parent 19e9c26 commit a3a8566
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 17 deletions.
12 changes: 10 additions & 2 deletions cmp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package cmp
import (
"fmt"
"reflect"
"regexp"
"strings"

"github.com/google/go-cmp/cmp/internal/function"
Expand Down Expand Up @@ -205,6 +206,11 @@ func (invalid) apply(s *state, _, _ reflect.Value) {
panic(fmt.Sprintf("cannot handle unexported field: %#v\n%s", s.curPath, help))
}

// identRx represents a valid identifier according to the Go specification.
const identRx = `[_\p{L}][_\p{L}\p{N}]*`

var identsRx = regexp.MustCompile(`^` + identRx + `(\.` + identRx + `)*$`)

// Transformer returns an Option that applies a transformation function that
// converts values of a certain type into that of another.
//
Expand All @@ -222,7 +228,9 @@ func (invalid) apply(s *state, _, _ reflect.Value) {
// to prevent the transformer from being recursively applied upon itself.
//
// The name is a user provided label that is used as the Transform.Name in the
// transformation PathStep. If empty, an arbitrary name is used.
// transformation PathStep (and eventually shown in the Diff output).
// The name must be a valid identifier or qualified identifier in Go syntax.
// If empty, an arbitrary name is used.
func Transformer(name string, f interface{}) Option {
v := reflect.ValueOf(f)
if !function.IsType(v.Type(), function.Transformer) || v.IsNil() {
Expand All @@ -231,7 +239,7 @@ func Transformer(name string, f interface{}) Option {
if name == "" {
name = "λ" // Lambda-symbol as place-holder for anonymous transformer
}
if !isValid(name) {
if !identsRx.MatchString(name) {
panic(fmt.Sprintf("invalid name: %q", name))
}
tr := &transformer{name: name, fnc: reflect.ValueOf(f)}
Expand Down
7 changes: 3 additions & 4 deletions cmp/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,10 +113,9 @@ func TestOptionPanic(t *testing.T) {
args: []interface{}{"/*", func(int) bool { return true }},
wantPanic: "invalid name",
}, {
label: "Transformer",
fnc: Transformer,
args: []interface{}{"_", func(int) bool { return true }},
wantPanic: "invalid name",
label: "Transformer",
fnc: Transformer,
args: []interface{}{"_", func(int) bool { return true }},
}, {
label: "FilterPath",
fnc: FilterPath,
Expand Down
11 changes: 0 additions & 11 deletions cmp/path.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,14 +296,3 @@ func isExported(id string) bool {
r, _ := utf8.DecodeRuneInString(id)
return unicode.IsUpper(r)
}

// isValid reports whether the identifier is valid.
// Empty and underscore-only strings are not valid.
func isValid(id string) bool {
ok := id != "" && id != "_"
for j, c := range id {
ok = ok && (j > 0 || !unicode.IsDigit(c))
ok = ok && (c == '_' || unicode.IsLetter(c) || unicode.IsDigit(c))
}
return ok
}

0 comments on commit a3a8566

Please sign in to comment.